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

Added thermal zone information and documentation #359

Merged
merged 3 commits into from Jul 15, 2019

Conversation

StewartThomson
Copy link
Contributor

@StewartThomson StewartThomson commented Jul 12, 2019

For issue #73 This PR adds a go file for ThermalZoneInformation that was autogenerated by New-Collector.ps1

Copy link
Collaborator

@carlpett carlpett left a comment

Choose a reason for hiding this comment

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

Hey @StewartThomson,
Thanks for giving this a go! I have some things that I'd like cleaned up, but mostly looks good 👍

collector/thermalzone.go Outdated Show resolved Hide resolved
collector/thermalzone.go Show resolved Hide resolved
collector/thermalzone.go Outdated Show resolved Hide resolved
collector/thermalzone.go Show resolved Hide resolved
collector/thermalzone.go Outdated Show resolved Hide resolved
docs/collector.thermalzone.md Outdated Show resolved Hide resolved
@carlpett
Copy link
Collaborator

Question though - does it seem to give proper data on your machine? I get seemingly constant temperatures of 301 Kelvin, even after running stress tests for a good while.

Removed references to tcp in collector.thermalzone.md
@StewartThomson
Copy link
Contributor Author

StewartThomson commented Jul 13, 2019

Thanks for the review! I do get appropriate data. It matches up with what is displayed in Novabench, and temps go up appropriately when benchmarking. It may have something to do with the fact that the code isn't looping over all of the zones. Also, I wasn't sure if you guys modified the generated go files or not, but I'll make the changes described above on Monday. Have a good weekend!

@carlpett
Copy link
Collaborator

Excellent, thanks!

…perature

- Converted decikelvin to Celsius
- Added a loop to get the values from each zone
- Added documentation for percent passive limit and throttle reasons
@StewartThomson
Copy link
Contributor Author

StewartThomson commented Jul 15, 2019

Resolved the above comments

Also added a name field to identify each thermalzone in the loop

Copy link
Collaborator

@carlpett carlpett left a comment

Choose a reason for hiding this comment

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

This is excellent, great work!
🎉

@carlpett carlpett merged commit 6107a59 into prometheus-community:master Jul 15, 2019

Name | Description | Type | Labels
-----|-------------|------|-------
`wmi_thermalzone_percent_passive_limit` | % Passive Limit is the current limit this thermal zone is placing on the devices it controls. A limit of 100% indicates the devices are unconstrained. A limit of 0% indicates the devices are fully constrained. | gauge | None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra credit for great documentation 👍

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.

None yet

2 participants