vmatare / thinkfan

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

sensor indices don't work for thinkpad_acpi sensors #84

Closed mowgli closed 4 years ago

mowgli commented 4 years ago

With the old config format it is possible to specify one sensor to be ignored by using a '.' instead of the numeric value.

I found no comparable way to do the same in yaml config file.

There is the 'indices' keyword in sensor definition but it seems to me broken. I cannot specify it as [0, 1, 2, 3, 4, 6, 8, 9] to just care about that sensor indexes (ignoring 5, 7 and all after 9) as just the length of that array is taken into consideration to specify how many sensors to check. The actual values are ignored.

vmatare commented 4 years ago

I'm not sure I understand the problem. I have the following in my config:

sensors:
  - hwmon: /sys/class/hwmon
    name: nct6795
    indices:
    - 1 # System temperature
    - 3 # Power Converter temperature

And thinkfan correctly uses only sensors 1 and 3 from that particular hwmon:

# strace -e trace=file -p `pgrep thinkfan`
strace: Process 15671 attached
openat(AT_FDCWD, "/sys/class/hwmon/hwmon2/temp1_input", O_RDONLY) = 5
openat(AT_FDCWD, "/sys/class/hwmon/hwmon2/temp3_input", O_RDONLY) = 5

So the values are definitely taken into account.

Could you test with the current master again and give a more detailed account of what is going wrong (if anything?)

mowgli commented 4 years ago

Hi Victor,

Am Mi den 8. Apr 2020 um 21:56 schrieb Victor Mataré:

I'm not sure I understand the problem. I have the following in my config:

sensors:
  - hwmon: /sys/class/hwmon
    name: nct6795
    indices:
    - 1 # System temperature
    - 3 # Power Converter temperature

And thinkfan correctly uses only sensors 1 and 3 from that particular hwmon:

strace -e trace=file -p 15671
strace: Process 15671 attached
openat(AT_FDCWD, "/sys/class/hwmon/hwmon2/temp1_input", O_RDONLY) = 5
openat(AT_FDCWD, "/sys/class/hwmon/hwmon2/temp3_input", O_RDONLY) = 5

So the values are definitely taken into account.

That might cout for hwmon, what I do not use. I have a thinkpad with the following:

sensors:
  - tpacpi: /proc/acpi/ibm/thermal
    #indices: [0, 1, 2, 3, 4, 6, 8, 9] # Anzahl gibt maximale Anzahl, egal, welche Nummern
    #correction: ...
  - atasmart: /dev/sda

fans:
  - tpacpi: /proc/acpi/ibm/fan

#    + CPU
#    |  + HDAPS (?)
#    |  |  + PCMCIA
#    |  |  |  + GPU
#    |  |  |  |  + Battery (vorne)
#    |  |  |  |  |     + Battery (hinten)
#    |  |  |  |  |     |     + Nordbridge/DRAM
#    |  |  |  |  |     |     |  + Southbridge/WLAN
#    1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16
#   45 52 52 40 33 -- 32 -- 42 38 -- -- -- -- -- --
#   +6 +1 +1  0 +2    +1    +1  0
#   +8  0  0 -2 +1    +1   +21 +15                 +1
#   +7  0  0  0 +1     0    +2  0
#   +7  0  0  0 +1    +1    +2 +2                  +1
levels:
  - speed: level 0
    upper_limit: [55, 99, 99, 50, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99]

  - speed: level 1
    lower_limit: [46, 98, 98, 46, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98]
    upper_limit: [60, 99, 99, 60, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99]

  - speed: level 2
    lower_limit: [56, 98, 98, 56, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98]
    upper_limit: [65, 99, 99, 65, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99]

  - speed: level 3
    lower_limit: [61, 98, 98, 61, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98]
    upper_limit: [70, 99, 99, 70, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99]

  - speed: level 7
    lower_limit: [66, 98, 98, 66, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98]
    upper_limit: [75, 99, 99, 75, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99]

  - speed: level disengaged
    lower_limit: [71, 98, 98, 71, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98, 98]

Independent of what I write in indices, I always have to specify all sensors in the limits below (all that with 98/99 values).

I found out by tests that I only need to consider CPU and GPU. All other values are not relevant for fan control. (Currently I even left the last value, the disk out, but that ist still subject to tests.)

Regards Klaus -- Klaus Ethgen http://www.ethgen.ch/ pub 4096R/4E20AF1C 2011-05-16 Klaus Ethgen Klaus@Ethgen.ch Fingerprint: 85D4 CA42 952C 949B 1753 62B3 79D0 B06F 4E20 AF1C

vmatare commented 4 years ago

Good catch, it was actually a bug in the thinkpad_acpi-specific sensor driver. Should be fixed in the updated master now. Thanks for the help in narrowing this down!

vmatare commented 4 years ago

@menobista Please don't spam existing issues with unrelated comments. Open a new issue if you're unsure. I'm about to delete your comment, but from what I can tell, it looks like you have disabled USE_YAML when compiling thinkfan. Please read README.md

vmatare commented 4 years ago

Assuming this is fixed.