Skip to content

Fix a few issues with CoreTemperature and add HSI#124

Merged
tuna-f1sh merged 3 commits into
Tigge:masterfrom
rasher:core-temp-fix
Aug 23, 2025
Merged

Fix a few issues with CoreTemperature and add HSI#124
tuna-f1sh merged 3 commits into
Tigge:masterfrom
rasher:core-temp-fix

Conversation

@rasher

@rasher rasher commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Found and fixed a few issues with CoreTemperature:

  • The sensor wasn't included in the device lists, meaning it didn't show up in the command line tool options at least - possibly other issues as well
  • Added support for Heat Strain Index value which was added somewhat recently (https://github.com/CoreBodyTemp/CoreBodyTemp/blob/main/CORE%20Connectivity%20Implementation%20Notes.pdf)
  • Set data value to None upon receiving invalid values according to spec
  • Fixed a typo CoteTemperatureData -> CoreTemperatureData
  • Storing/updating event-count correctly, although it's not used for anything

I'm not 100% sure about the handling of invalid values. Setting it to None seems appropriate to me, but I'm not sure if it might have any unintended consequences down the line?

rasher added 2 commits August 21, 2025 18:40
- Read Heat Strain Index from Core Temperature data (if available).
- Store "reserved" in a field. Contents are not clear.
- Fix typo in data class name (Cote -> Core)
- Set value to None for invalid values.
@rasher

rasher commented Aug 22, 2025

Copy link
Copy Markdown
Contributor Author

Failed check must be unrelated, a re-run from someone with permissions should fix it.

@tuna-f1sh

Copy link
Copy Markdown
Collaborator

I'm not 100% sure about the handling of invalid values. Setting it to None seems appropriate to me, but I'm not sure if it might have any unintended consequences down the line?

Yes undecided here too. Technically None isn't nice since the types aren't Optional[float] anyone using type checking is going to get a warning (not related to this PR but quality should actually default to CoreTempDataQuality.Unused). Perhaps propagating the profile invalid value itself without scaling: 0xFF, 0x800, etc? Someone using the Core Temperature device should know this is an invalid value from the sensor profile.

@tuna-f1sh

Copy link
Copy Markdown
Collaborator

They should also initialise to either of these values too, rather than 0.0.

Consumers may compare to core_temp.*_INVALID constants.
@rasher

rasher commented Aug 22, 2025

Copy link
Copy Markdown
Contributor Author

Yeah I think that makes sense. I've made constants of them so users can use those. Although that means they should be floats in order for the typing to work out, which feels a bit off.

I tense up when floats come up, should work out though since they're actually integers.

>>> float(0x8000) == 0x8000
True
>>> float(0x800) == 0x800
True
>>> float(0xFF) == 0xFF
True

It's a little wonky looking - I'm not sure if there's a more elegant solution short of doing a bunch of explicit conversions.

@tuna-f1sh

Copy link
Copy Markdown
Collaborator

Looks good. Thanks.

@tuna-f1sh tuna-f1sh merged commit 0497f30 into Tigge:master Aug 23, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants