Add support for get temperature on m1 #792
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for working on this. It indeed looks very promising. |
Mark3K
force-pushed
the
m1-components-support
branch
from
b127791 to
f7a9eb0
Compare
|
I am learning rust and happen to have M1 Mac, so it's an opportunity for me to practice and I am gland I can help |
|
Don't forget to take a look at the CI too. |
Mark3K
force-pushed
the
m1-components-support
branch
2 times, most recently
from
246c542 to
8c7e09d
Compare
Mark3K
changed the title
src/apple/macos/component/arm.rs
Outdated
Comment on lines
51
to
54
| if client.is_none() { | ||
| return; | ||
| } | ||
| let client = client.unwrap(); |
There was a problem hiding this comment.
Suggested change
| if client.is_none() { | |
| return; | |
| } | |
| let client = client.unwrap(); | |
| let client = match client { | |
| Some(c) => c, | |
| None => return, | |
| }; |
src/apple/macos/component/arm.rs
Outdated
| pub(crate) fn refresh(&mut self) { | ||
| self.inner.clear(); | ||
|
|
||
| let client = || -> Option<CFReleaser<_>> { |
There was a problem hiding this comment.
I'm not sure to understand why you needed to make this into a closure. Why not a simple block instead?
src/apple/macos/component/arm.rs
Outdated
Comment on lines
69
to
72
| if key_ref.is_none() { | ||
| return; | ||
| } | ||
| let key_ref = key_ref.unwrap(); |
There was a problem hiding this comment.
Suggested change
| if key_ref.is_none() { | |
| return; | |
| } | |
| let key_ref = key_ref.unwrap(); | |
| let key_ref = match key_ref { | |
| Some(k) => k, | |
| None => return, | |
| }; |
There was a problem hiding this comment.
Please note that you could match directly on CFReleaser::new(...).
src/apple/macos/component/arm.rs
Outdated
| let client = client.unwrap(); | ||
|
|
||
| unsafe { | ||
| let services = IOHIDEventSystemClientCopyServices(client.inner()); |
There was a problem hiding this comment.
services needs to be freed using CFRelease. And before every return too. The best thing you can do is wrapping it into a struct which implements Drop (which calls CFRelease automatically).
There was a problem hiding this comment.
Ah you already added CFReleaser so you already have the needed struct. :)
src/apple/macos/component/arm.rs
Outdated
Comment on lines
77
to
81
| let service = CFReleaser::new(CFArrayGetValueAtIndex(services, i) as *const _); | ||
| if service.is_none() { | ||
| continue; | ||
| } | ||
| let service = service.unwrap(); |
There was a problem hiding this comment.
Suggested change
| let service = CFReleaser::new(CFArrayGetValueAtIndex(services, i) as *const _); | |
| if service.is_none() { | |
| continue; | |
| } | |
| let service = service.unwrap(); | |
| let service = match CFReleaser::new(CFArrayGetValueAtIndex(services, i) as *const _) { | |
| Some(s) => s, | |
| None => continue, | |
| }; |
src/apple/macos/component/arm.rs
Outdated
Comment on lines
83
to
90
| let name = CFReleaser::new(IOHIDServiceClientCopyProperty( | ||
| service.inner(), | ||
| key_ref.inner(), | ||
| )); | ||
| if name.is_none() { | ||
| continue; | ||
| } | ||
| let name = name.unwrap(); |
There was a problem hiding this comment.
Suggested change
| let name = CFReleaser::new(IOHIDServiceClientCopyProperty( | |
| service.inner(), | |
| key_ref.inner(), | |
| )); | |
| if name.is_none() { | |
| continue; | |
| } | |
| let name = name.unwrap(); | |
| let name = match CFReleaser::new(IOHIDServiceClientCopyProperty( | |
| service.inner(), | |
| key_ref.inner(), | |
| )) { | |
| Some(n) => n, | |
| None => continue, | |
| }; |
src/apple/macos/component/arm.rs
Outdated
Comment on lines
158
to
167
| let event = CFReleaser::new(IOHIDServiceClientCopyEvent( | ||
| self.service.inner() as *const _, | ||
| kIOHIDEventTypeTemperature, | ||
| 0, | ||
| 0, | ||
| )); | ||
| if event.is_none() { | ||
| return; | ||
| } | ||
| let event = event.unwrap(); |
src/apple/macos/component/arm.rs
Outdated
| self.temperature = IOHIDEventGetFloatValue( | ||
| event.inner(), | ||
| IOHIDEventFieldBase(kIOHIDEventTypeTemperature), | ||
| ) as f32; |
There was a problem hiding this comment.
Suggested change
| ) as f32; | |
| ) as _; |
src/apple/macos/ffi.rs
Outdated
| pub fn IOHIDEventGetFloatValue(event: IOHIDEventRef, field: i64) -> f64; | ||
| } | ||
|
|
||
| // pub type CFMutableArrayRef = *mut __CFArray; |
There was a problem hiding this comment.
Please remove the unused code.
Mark3K
force-pushed
the
m1-components-support
branch
from
4c1a5b1 to
15c964d
Compare
|
Thanks for the time and advices and the code indeed looks more cleaner :) |
Mark3K
force-pushed
the
m1-components-support
branch
from
7ad87ba to
044871f
Compare
GuillaumeGomez
approved these changes
Aug 20, 2022
|
Looks all good to me now, thanks! I'll ask on twitter if someone with a M1 mac can double-check and then I merge. EDIT: tweet is here |
|
On M2 it reports incorrect temperatures: |
|
Thanks @evanrichter ! So it seems that the list of components is retrieved as it should but not their information. Then ping @Mark3K. 😉 |
|
yeah I know the call for volunteers was for M1, but if it worked on M2 that would be pretty good evidence it also works on M1 |
|
I expect that if it doesn't work on M2, it doesn't work on M1. So thanks a lot for testing it! |
|
M1 result here. (with commit 044871f) Click to expand |
|
The problem is when refresh component list, its not refresh the new component if refreshes.components_list() {
self.refresh_components_list();
} else if refreshes.components() {
self.refresh_components();
}so I fixed it by refresh the components immediately after create it $ sysctl -a | grep brand_string
machdep.cpu.brand_string: Apple M1 Pro
$ cargo run --example simple
Compiling sysinfo v0.25.1
Finished dev [unoptimized + debuginfo] target(s) in 0.81s
Running `target/debug/examples/simple`
Getting processes' information...
Done.
To get the commands' list, enter 'help'.
> temperature
PMU tdie10: 40.7704°C (max: 0°C)
PMU tdie9: 40.859207°C (max: 0°C)
PMU TP2s: 40.415207°C (max: 0°C)
PMU tdev1: 38.69821°C (max: 0°C)
PMU TP3g: 40.948013°C (max: 0°C)
PMU tdev4: 36.009018°C (max: 0°C)
PMU TP2s: 40.68161°C (max: 0°C)
PMU tdev6: 38.099106°C (max: 0°C)
PMU tdie3: 40.68161°C (max: 0°C)
PMU tdev7: 39.14865°C (max: 0°C)
PMU tdev1: 38.7838°C (max: 0°C)
PMU TP3g: 40.504013°C (max: 0°C)
PMU tdie1: 40.415207°C (max: 0°C)
PMU tdie5: 40.948013°C (max: 0°C)
PMU tdie2: 40.7704°C (max: 0°C)
PMU tdie1: 41.036804°C (max: 0°C)
gas gauge battery: 34°C (max: 0°C)
PMU tdie6: 40.7704°C (max: 0°C)
PMU tdie2: 40.859207°C (max: 0°C)
gas gauge battery: 34.799988°C (max: 0°C)
PMU tdie5: 40.504013°C (max: 0°C)
PMU tdie8: 40.7704°C (max: 0°C)
PMU tdie2: 40.859207°C (max: 0°C)
PMU tdie8: 41.2144°C (max: 0°C)
PMU TP0s: 40.68161°C (max: 0°C)
PMU TP1g: 41.036804°C (max: 0°C)
PMU tdev2: 39.88739°C (max: 0°C)
PMU TP0s: 40.859207°C (max: 0°C)
PMU tdev4: 35.824326°C (max: 0°C)
PMU tdev5: 34.20804°C (max: 0°C)
PMU TP1g: 41.12561°C (max: 0°C)
PMU tdev7: 39.66667°C (max: 0°C)
PMU tdie4: 40.859207°C (max: 0°C)
PMU tdev8: 39.55406°C (max: 0°C)
PMU tdev2: 39.833344°C (max: 0°C)
PMU tdie2: 40.3264°C (max: 0°C)
gas gauge battery: 34.799988°C (max: 0°C)
NAND CH0 temp: 36°C (max: 0°C)
PMU tdie0: 40.859207°C (max: 0°C)
gas gauge battery: 34.799988°C (max: 0°C)
PMU tdie3: 41.12561°C (max: 0°C)
PMU tdie6: 40.7704°C (max: 0°C)
PMU tdie9: 41.036804°C (max: 0°C)
PMU TP1s: 41.2144°C (max: 0°C)
PMU tcal: 51.850006°C (max: 0°C)
PMU tdie10: 40.504013°C (max: 0°C)
PMU TP2g: 41.2144°C (max: 0°C)
PMU tdev3: 39.88739°C (max: 0°C)
PMU TP1s: 41.036804°C (max: 0°C)
PMU tdev5: 35.360367°C (max: 0°C)
PMU tdev6: 38.51352°C (max: 0°C)
PMU tcal: 51.850006°C (max: 0°C)
PMU TP2g: 40.859207°C (max: 0°C)
PMU tdev8: 39.55406°C (max: 0°C)
PMU tdie1: 40.504013°C (max: 0°C)
PMU tdev3: 39.9955°C (max: 0°C)
PMU tdie0: 41.036804°C (max: 0°C)
gas gauge battery: 34°C (max: 0°C)
PMU tdie1: 41.12561°C (max: 0°C)
gas gauge battery: 34.699997°C (max: 0°C)
PMU tdie4: 40.948013°C (max: 0°C)
PMU tdie7: 40.68161°C (max: 0°C)
PMU tdie7: 40.859207°C (max: 0°C)
> |
Mark3K
force-pushed
the
m1-components-support
branch
from
c69275d to
f20b920
Compare
|
@evanrichter @weihanglo it will be great helpful if you guys give a another test |
|
Now looks nicer. Sorry I didn't look into it more, and thanks for your quick fix, @Mark3K ! click to expand |
|
Thanks everyone for your help on this! Time to merge it then. 😃 |
|
emmm,Which label represents the temperature of CPU1? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, I trying to help with this issue #456, and it almost work, but I am a newbie to rust and not familiar with MacOS development.
when I run the code on my m1 mac, I got 63 sensors' value, but some of are duplicated, like there are 4 sensors with name
PMU tdie1, and I do not known if it is right, so I need some help with this.