oxidecomputer / oxidecomputer/hubris

thermal should be a bit more careful about NaN values

Open
#2,613 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

thermal 🚫🔥🚫
Dominant language
Rust
Stars
3.6k
Forks
239
Avg merge
1d 12h
Merged PRs (30d)
23

Description

There are a number of places in the thermal loop where we probably ought to check that f32 values are not NaNs. In particular, when we read temperatures from sensors, here:
https://github.com/oxidecomputer/hubris/blob/3a8bce06611371da9962e240ff8938ec837952c7/task/thermal/src/control.rs#L1260-L1264
...and from dynamic inputs, here:
https://github.com/oxidecomputer/hubris/blob/3a8bce06611371da9962e240ff8938ec837952c7/task/thermal/src/control.rs#L1291-L1293

we should probably check if they are NaN, in case a sensor either returns a NaN directly or a sensor's driver produced one while converting from another numeric format. When we then compare these temperatures to the configured power-down and critical thresholds, they will compare false, despite meaning "we don't know the temperature". In such cases, we should probably be using the worst-case temperature estimate for that component, instead.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in task/thermal/src/control.rs at the sensor-temperature handling around lines 1260-1264 and dynamic-input handling around lines 1291-1293. Trace how these values are compared with power-down and critical thresholds, then verify that NaN inputs use the component's worst-case temperature estimate. Confirm the thermal loop no longer treats an unknown temperature as below either threshold.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
embedded-iot
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.