Skip to content

fix: report an absent sensor as -1 rather than 0 - #2127

Open
GribanovIvan wants to merge 2 commits into
flightlessmango:masterfrom
GribanovIvan:fix/absent-sensor-default
Open

GribanovIvan wants to merge 2 commits into
flightlessmango:masterfrom
GribanovIvan:fix/absent-sensor-default

Conversation

@GribanovIvan

Copy link
Copy Markdown

junction_temp and memory_temp are declared with a -1 default and the HUD only draws them when the value is greater than -1, but the constructor initialises both to 0 and so defeats both. Any GPU without those sensors therefore renders a steady "0 C" instead of hiding the row - an APU exposing only an edge sensor shows 0 C junction forever.

Initialise them to -1 so the declared default and the draw condition agree.

junction_temp and memory_temp are declared with a -1 default and the HUD only
draws them when the value is greater than -1, but the constructor initialises
both to 0 and so defeats both. Any GPU without those sensors therefore renders
a steady "0 C" instead of hiding the row - an APU exposing only an edge sensor
shows 0 C junction forever.

Initialise them to -1 so the declared default and the draw condition agree.
Copilot AI lite review requested due to automatic review settings August 28, 2026 19:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There is a maintainability issue due to duplicated default initialization for junction_temp/memory_temp (in-class defaults plus ctor init), which can drift again.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes GPU temperature reporting in MangoHud’s gpu_metrics defaults so absent junction/memory sensors don’t render as 0 C in the HUD, aligning constructor initialization with the sentinel semantics used by the draw conditions.

Changes:

  • Initialize junction_temp and memory_temp to -1 in gpu_metrics() so “sensor absent” stays hidden in the HUD.
  • Bring constructor initialization in line with the struct’s declared default values and HUD visibility checks.
File summaries
File Description
src/gpu_metrics_util.h Sets default junction/memory temps to -1 to correctly hide missing-sensor rows in the HUD.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/gpu_metrics_util.h Outdated
@flightlessmango

Copy link
Copy Markdown
Owner

The cleaner solution might be to remove the constructor entirely and set defaults

Addresses review feedback on the previous commit. Setting junction_temp and
memory_temp to -1 in the constructor fixed the symptom but kept two places
declaring the same defaults, which is what let them drift apart to begin with.

Give every member an in-class default instead and remove the constructor, so
there is one source of truth. The resulting values are identical; only the
duplication goes away.
@flightlessmango

Copy link
Copy Markdown
Owner

this can be one commit

@GribanovIvan

Copy link
Copy Markdown
Author

If squash merging is enabled for the repo, Squash and merge folds them into one at merge time: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests
Or do you actually want me to force-push the branch as a single commit?

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.

3 participants