vmatare / thinkfan

The minimalist fan control program
GNU General Public License v3.0
552 stars 62 forks source link

Add support for searching hwmon sensors by name #69

Closed akheron closed 5 years ago

akheron commented 5 years ago

Add a new config directive thermal_zone name which searches /sys/devices/virtual/thermal for a file called name. It its content matches the name, use the temp1_input next to it as the temperature input.

Fixes #43

vmatare commented 5 years ago

Oh thanks, I like the idea, but in the current form it's not as useful as it could be. I think all hwmon sensors have this name file, so I'd like to see it generalized to arbitrary sysfs paths. That is, don't add a new HwmonSensor class, but extend the existing one so it is able to look up sensors by name. Haven't looked closely yet, but the code looks well-structured enough to achieve that easily.

After that I'd also like to see the functionality integrated in the YAML parser, as well, but I can also do that if you don't want to read into the YAML API ;-)

akheron commented 5 years ago

I'm not sure what you mean about extending the HwmonSensor class. Do you mean that it would be given a parent directory, and then it would search the sensor under that directory by name? Then the config syntax would be something like:

hwmon /sys/devices/thermal/:sensor_name (correction_value)
akheron commented 5 years ago

BTW, I need this change because it seems that my CPU temperature hwmon sensor's sysfs path changes between boots and even suspend-to-ram/resume rounds :grimacing:

akheron commented 5 years ago

It also seems that some hwmon sensors can have multiple tempN_inputs, and there should be a way to somehow choose between them if the hwmon sensor is matched by name.

vmatare commented 5 years ago

Yeah, so what I meant was to integrate the name-search functionality into the existing HwmonSensorDriver class. You have some logic for handling multiple tempN_input files in yamlconfig.cpp. Check out thinkfan.conf.yaml. There you have an example for how different sub-sensors can be selected by index.

vmatare commented 5 years ago

I think you should look at the find_hwmons(..) function in yamlconfig.cpp. My hunch right now is that the path you find with find_temp_input_by_name(...) could be an input to that function.

akheron commented 5 years ago

I force-pushed a change which discards the driver changes and does it all in the yaml side. It first recursively searches for a hwmon that has the correct name, and then feeds that hwmon to find_hwmons(), as you suggested.

vmatare commented 5 years ago

Yes, I like that much better. I also wanted to deprecate the old config syntax, which is another reason why new features such as this should go into the YAML config.