Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thermal Zones #347

Merged
merged 10 commits into from
Jun 21, 2022
Merged

Thermal Zones #347

merged 10 commits into from
Jun 21, 2022

Conversation

Camerooooon
Copy link
Collaborator

Closes #341

@@ -290,6 +290,28 @@ pub fn list_cpu_governors() -> Vec<String> {
list_cpus().into_iter().map(|x| x.gov).collect()
}

pub fn read_int(path: &str) -> Result<i32, Error> {
Copy link
Owner

Choose a reason for hiding this comment

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

Good call moving these

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I noticed that they would be duplicate with the thermal zone reader. We also need to refactor the cpus code it's a little wacky I feel like. That's an issue for another time.

}
}

pub fn read_thermal_zones() -> Vec<ThermalZone> {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice!

pub fn read_thermal_zones() -> Vec<ThermalZone> {
let mut zones = Vec::<ThermalZone>::new();

for a in read_dir(THERMAL_ZONE_DIR).expect("Could not read thermal directory") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

prob wanna check the type first, since most thermal zones are just constants (there is no thermal zones for each individual cores, instead it is shown as one x86_pkg_temp)

Copy link
Collaborator

Choose a reason for hiding this comment

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

also fun fact, there are 19 thermal zones on my hexacore laptop, 11 thermal zones on my octacore laptop, and 2 on my dual-core laptop

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

prob wanna check the type first, since most thermal zones are just constants (there is no thermal zones for each individual cores, instead it is shown as one x86_pkg_temp)

I'm planning on converting type to an enum with a catch all type "other" for all undocumented thermal zones. That way we can filter out all virtual thermal zones

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm planning on converting type to an enum with a catch all type "other" for all undocumented thermal zones. That way we can filter out all virtual thermal zones

This doesn't seem to be necessary.

@@ -0,0 +1,40 @@
use super::Error;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we could prob also add fan speed and cooling here later since they are in the same directory

Copy link
Owner

Choose a reason for hiding this comment

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

Yes!! Fan speed!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The cooling devices didn't show fan speed on my laptop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, maybe its device or manufacture specific then

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm, maybe its device or manufacture specific then

Could be bios security. I'll check when I get a chance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So I couldn't find fan speed in the cooling device directory so I created its own issue since it might be more involved. @JakeRoggenbuck @Shuzhengz

Copy link
Owner

Choose a reason for hiding this comment

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

Okay, sounds good

src/cpu.rs Outdated Show resolved Hide resolved
@Camerooooon Camerooooon marked this pull request as ready for review June 21, 2022 05:24
@JakeRoggenbuck
Copy link
Owner

Tested, everything works great!

@JakeRoggenbuck JakeRoggenbuck merged commit 62f864c into JakeRoggenbuck:main Jun 21, 2022
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.

Take temperature from thermal zones
3 participants