Skip to content

Unified component temperature api with GPU support - #1706

Draft
CodedNil wants to merge 5 commits into
GuillaumeGomez:mainfrom
CodedNil:gpu_temp
Draft

Unified component temperature api with GPU support#1706
CodedNil wants to merge 5 commits into
GuillaumeGomez:mainfrom
CodedNil:gpu_temp

Conversation

@CodedNil

Copy link
Copy Markdown

Add support for reading the GPU temperature for linux macos and windows.
For nvidia it uses the nvml library, for amd it reads the files directly.

I don't have the means to test macos or windows support (I can test windows soon), it is verified against their documentation and I made sure it compiles. If anyone else can test those it would be great, else I can remove that code and return None as an unsupported platform.

Comment thread src/common/gpu.rs
@GuillaumeGomez

GuillaumeGomez commented Jul 24, 2026

Copy link
Copy Markdown
Owner

I'm gonna send a PR to fix the CI.

Also: this PR introduces a change in paradigm: the temperature used to be only part of Component and now we're duplicating it into GPU. Not too sure what to think about it... It's more logical, but it would also mean that we likely should do the same for CPU, disks and motherboard.

@CodedNil

CodedNil commented Jul 24, 2026

Copy link
Copy Markdown
Author

Also: this PR introduces a change in paradigm: the temperature used to be only part of Component and now we're duplicating it into GPU. Not too sure what to think about it... It's more logical, but it would also mean that we likely should do the same for CPU, disks and motherboard.

I'd personally think it would be great if the same was done for CPU, disks, motherboards as afaik this is the current best method for getting cpu temps (this is what I do in my app).

cpu.temperature = components
    .iter()
    .find(|component| {
        ["k10temp", "coretemp", "zenpower", "cpu"]
            .iter()
            .any(|name| component.label().contains(name))
    })
    .and_then(sysinfo::Component::temperature)
    .unwrap_or_default();

Which feels more brittle then it needs to be when you have to list out potential component names rather than making sure sysinfo gives you it with more guarantees with a global_cpu_temperature() function.
I could make it push the temperature to components (I think amd gpus already are in there?) but afaik nvidia gpus aren't even registered as components currently there.

@GuillaumeGomez

Copy link
Copy Markdown
Owner

No, I like the way this is going, just that I (we?) will need to do it for all items. And also maybe remove Component.

@CodedNil
CodedNil force-pushed the gpu_temp branch 2 times, most recently from 11cc193 to c250405 Compare July 24, 2026 15:36
@CodedNil

Copy link
Copy Markdown
Author

I can make cpu/disk/motherboard etc expose temperature fields in this PR as another commit (renamed to Unified component temperature api with gpu support) or would you like in a separate PR?
Seems quite simple to do from looking at the code.

@GuillaumeGomez

Copy link
Copy Markdown
Owner

By that you mean having a temperature method on Disk, CPU and Motherboard right? If you can retrieve the information for each of them, then we can remove Component (and Components). And yeah, as part of this PR in another commit sounds good to me. If you can make one commit per "class" (ie disk, CPU, motherboard), even better.

@GuillaumeGomez

Copy link
Copy Markdown
Owner

Please rebase on #1707 when you push the next commit(s) so CI can pass as well.

@CodedNil
CodedNil force-pushed the gpu_temp branch 2 times, most recently from 29fab8f to 767a29b Compare July 24, 2026 17:52
@CodedNil

Copy link
Copy Markdown
Author

Rebased and confirmed it works for nvidia, needed a separate library path lookup for my system (nixos) with isn't breaking.

@CodedNil

CodedNil commented Jul 24, 2026

Copy link
Copy Markdown
Author

New commit adds support for cpu temperature directly on the CPU object are you happy with that? It includes the max and critical threshold, so I think fully covers what the component provided.

@CodedNil
CodedNil force-pushed the gpu_temp branch 3 times, most recently from b3ad2e9 to ea9987d Compare July 24, 2026 20:27
@CodedNil CodedNil changed the title Add AMD and Nvidia GPU temperature reading Unified component temperature api with GPU support Jul 25, 2026
@CodedNil

CodedNil commented Jul 29, 2026

Copy link
Copy Markdown
Author

I think this is as feature complete as I can make it now. I thought a lot on the ergonomics of the those functions trying to match your style and think this is implemented in the simplest way now.
It only implements cpu, disks and motherboard temperature on Linux, I'm not versed whatsoever in how that works on windows/bsd/apple and don't have those platforms to test, and it's hard to interpret from existing component.rs code how we'd read for those there if its even provided at all.
Perhaps it would be wise to revert the implementations of gpu temperature sensing also for macos/windows, or I could make best guess attempts to implement at least cpu temperature on those platforms too if you are happy with that?
I've made sure to update the simple example to print these fields to test.

@GuillaumeGomez

Copy link
Copy Markdown
Owner

Thanks! Gonna try to come back to this "soon" (might be up to a few weeks ^^').

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