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 temperatures sensors for macOS and Linux #334

Merged
merged 2 commits into from
Apr 10, 2017

Conversation

egourlao
Copy link
Contributor

This pull request, following #329, introduces in the host package :

  • A new structure, TemperatureStat, that stores a temperature key and its sensor value. This is to ensure a standard portable structure for temperature sensors data for the package:
type TemperatureStat struct {
	SensorKey   string  `json:"sensorKey"`
	Temperature float64 `json:"sensorTemperature"`
}
  • A new function, SensorsTemperatures(), available for the moment on macOS and Linux builds. This function returns a list of TemperatureStat objects - each of these objects represent the value of a temperature sensor.

    • On Linux, the data is obtained the same way as psutil, through the hwmon interface. It is syntaxically correct and follows the psutil's procedure, but hasn't been tested properly yet - if a Linux-using contributor could test this is on their machine, I would be grateful for it.

    • On macOS, the data is obtained through the System Management Controller, with a C library acting as a middleman (host/include/smc.{c,h}). This C library is GNU-licensed, and the license has been kept as-is. Thus, the function is only available for Cgo-enabled builds. Match between a sensor key and what it represents is available in the host/include/smc.h file. The function returns 0 for a sensor value if there was an error on the SMC side or that the sensor is not available, which is the normal behaviour of the SMC function - but if we only want values that could have been accessed, we could ignored sensor values that are equal to 0.

This is my first contribution to the project, so if any contributor has any comment on this, please feel free to speak up!

Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
Could you change to use HostSys and add some tests to host/host_test.go ?


func SensorsTemperatures() ([]TemperatureStat, error) {
var temperatures []TemperatureStat
files, err := filepath.Glob("/sys/class/hwmon/hwmon*/temp*_*")
Copy link
Owner

Choose a reason for hiding this comment

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

Could you use common.HostSys to change /sys path via environment variable?

@shirou
Copy link
Owner

shirou commented Mar 20, 2017

Thank you! It works like a charm at least on my Darwin environment. awesome!

Added TemperatureStat string test
@egourlao
Copy link
Contributor Author

Sure! I didn't add any unit tests for SensorsTemperatures() yet - only for the struct -, since otherwise the tests would fail on the platforms where temperatures are not yet available, but tell me if you want me to add some!

@shirou
Copy link
Owner

shirou commented Apr 10, 2017

I'm very sorry. I have not noticed your update. It seems good for me. Thank you for your contribution!

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