Only setting one of `index` or `rpmChannel` is not enough #230

piyueh · 2023-05-19T17:10:56Z

Describe the bug

Only setting either one of index or rpmChannel gives this error: must have one of index or rpmChannel, must be >= 1.

To Reproduce

$ fan2go detect (I'm only posting relevant sections here):

Click me to expand

> dell_smm-isa-0000
  Fans     Index  Channel  Label        RPM  PWM  Auto 
  -----------------------------------------------------
           1      1        hwmon4/fan1  0    0    false
           2      2        hwmon4/fan2  0    0    false
  Sensors  Index  Label                       Value
  -------------------------------------------------
           1      hwmon4/temp1 (temp1_input)  44000
           2      hwmon4/temp2 (temp2_input)  37000
           3      hwmon4/temp3 (temp3_input)  38000
           4      hwmon4/temp4 (temp4_input)  32000
           5      hwmon4/temp5 (temp5_input)  38000
           6      hwmon4/temp6 (temp6_input)  38000
           7      hwmon4/temp7 (temp7_input)  37000

> coretemp-isa-0000
  Sensors  Index  Label                       Value
  -------------------------------------------------
           1      Package id 0 (temp1_input)  53000
           2      Core 0 (temp2_input)        51000
           3      Core 1 (temp3_input)        51000
           4      Core 2 (temp4_input)        52000
           5      Core 3 (temp5_input)        50000
           6      Core 4 (temp6_input)        52000
           7      Core 5 (temp7_input)        48000

Case 1: setting only rpmChannel (and also pwmChannel)
a. $ cat /etc/fan2go/fan2go.yaml:

Click me to expand

fans:
  - id: fan-1
    hwmon:
      platform: dell_smm-isa-0000
      rpmChannel: 1
      pwmChannel: 1
    neverStop: true
    curve: fan-curve-1
sensors:
  - id: cpu-package
    hwmon:
      platform: coretemp-isa-0000
      index: 1
curves:
  - id: fan-curve-1
    linear:
      sensor: cpu-package
      min: 50
      max: 90

b. # fan2go fan init -i fan-1, I got

Click me to expand

INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
INFO  Using persistence at: /etc/fan2go/fan2go.db
INFO  Deleting existing data for fan 'fan-1'...
INFO  Computing pwm map...
ERROR   Unable to set Fan Mode of 'fan-1' to "1": PWM mode stuck to -1
ERROR   Unable to set Fan Mode of 'fan-1' to "0": write
    /sys/devices/platform/dell_smm_hwmon/hwmon/hwmon5/pwm1_enable:
    invalid argument
INFO  Measuring RPM curve...
ERROR   Unable to set Fan Mode of 'fan-1' to "1": PWM mode stuck to -1
ERROR   Unable to set Fan Mode of 'fan-1' to "0": write
    /sys/devices/platform/dell_smm_hwmon/hwmon/hwmon5/pwm1_enable:
    invalid argument
WARNING  Could not enable manual fan mode on fan-1, trying to
    continue anyway...
SUCCESS  Done!
INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
WARNING  Cannot send notification, unable to detect user of current display session
FATAL   fan fan-1: must have one of index or rpmChannel, must be >= 1
panic:

goroutine 1 [running]:
github.com/pterm/pterm.checkFatal(...)
      github.com/pterm/[email protected]/prefix_printer.go:351
github.com/pterm/pterm.(*PrefixPrinter).Printfln(0x55e9b8d99dc0, {0xc00050fb40, 0x3d}, {0x0, 0x0, 0x0})
      github.com/pterm/[email protected]/prefix_printer.go:293 +0x1ed
github.com/markusressel/fan2go/internal/ui.Fatal({0xc00050fb40, 0x3d}, {0x0, 0x0, 0x0})
      github.com/markusressel/fan2go/internal/ui/logging.go:52 +0x8b
github.com/markusressel/fan2go/cmd/fan.glob..func1(0x55e9b8d99e80?, {0x55e9b872711f?, 0x5?, 0x0?})
      github.com/markusressel/fan2go/cmd/fan/curve.go:27 +0xe5
github.com/markusressel/fan2go/cmd/fan.glob..func2(0x55e9b8d9e860?, {0xc00029b420?, 0x2?, 0x2?})
      github.com/markusressel/fan2go/cmd/fan/init.go:55 +0x2f2
github.com/spf13/cobra.(*Command).execute(0x55e9b8d9e860, {0xc00029b400, 0x2, 0x2})
      github.com/spf13/[email protected]/command.go:940 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0x55e9b8d9d160)
      github.com/spf13/[email protected]/command.go:1068 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
      github.com/spf13/[email protected]/command.go:992
github.com/markusressel/fan2go/cmd.Execute()
      github.com/markusressel/fan2go/cmd/root.go:88 +0xf9
main.main()
      ./main.go:25 +0x17

Case 2: setting only index
a. $ cat /etc/fan2go/fan2go.yaml:

Click me to expand

fans:
  - id: fan-1
    hwmon:
      platform: dell_smm-isa-0000
      index: 1
    neverStop: true
    curve: fan-curve-1
sensors:
  - id: cpu-package
    hwmon:
      platform: coretemp-isa-0000
      index: 1
curves:
  - id: fan-curve-1
    linear:
      sensor: cpu-package
      min: 50
      max: 90

b. # fan2go fan init -i fan-1, I got

Click me to expand

INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
INFO  Using persistence at: /etc/fan2go/fan2go.db
INFO  Deleting existing data for fan 'fan-1'...
INFO  Computing pwm map...
ERROR   Unable to set Fan Mode of 'fan-1' to "1": PWM mode stuck to -1
ERROR   Unable to set Fan Mode of 'fan-1' to "0": write
    /sys/devices/platform/dell_smm_hwmon/hwmon/hwmon5/pwm1_enable:
    invalid argument
INFO  Measuring RPM curve...
ERROR   Unable to set Fan Mode of 'fan-1' to "1": PWM mode stuck to -1
ERROR   Unable to set Fan Mode of 'fan-1' to "0": write
    /sys/devices/platform/dell_smm_hwmon/hwmon/hwmon5/pwm1_enable:
    invalid argument
WARNING  Could not enable manual fan mode on fan-1, trying to
    continue anyway...
SUCCESS  Done!
INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
WARNING  Cannot send notification, unable to detect user of current display session
FATAL   fan fan-1: must have one of index or rpmChannel, must be >= 1
panic:

goroutine 1 [running]:
github.com/pterm/pterm.checkFatal(...)
      github.com/pterm/[email protected]/prefix_printer.go:351
github.com/pterm/pterm.(*PrefixPrinter).Printfln(0x5636519a8dc0, {0xc0004a9800, 0x3d}, {0x0, 0x0, 0x0})
      github.com/pterm/[email protected]/prefix_printer.go:293 +0x1ed
github.com/markusressel/fan2go/internal/ui.Fatal({0xc0004a9800, 0x3d}, {0x0, 0x0, 0x0})
      github.com/markusressel/fan2go/internal/ui/logging.go:52 +0x8b
github.com/markusressel/fan2go/cmd/fan.glob..func1(0x5636519a8e80?, {0x56365133611f?, 0x5?, 0x0?})
      github.com/markusressel/fan2go/cmd/fan/curve.go:27 +0xe5
github.com/markusressel/fan2go/cmd/fan.glob..func2(0x5636519ad860?, {0xc000136260?, 0x2?, 0x2?})
      github.com/markusressel/fan2go/cmd/fan/init.go:55 +0x2f2
github.com/spf13/cobra.(*Command).execute(0x5636519ad860, {0xc000136240, 0x2, 0x2})
      github.com/spf13/[email protected]/command.go:940 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0x5636519ac160)
      github.com/spf13/[email protected]/command.go:1068 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
      github.com/spf13/[email protected]/command.go:992
github.com/markusressel/fan2go/cmd.Execute()
      github.com/markusressel/fan2go/cmd/root.go:88 +0xf9
main.main()
      ./main.go:25 +0x17

Expected behavior

No error messages.

Desktop (please complete the following information):

Distro: Arch Linux
uname -a: 6.3.2-arch1-1
sensors -v: sensors version 3.6.0+git with libsensors version 3.6.0+git
fan2go version: commit c20619e

The text was updated successfully, but these errors were encountered:

markusressel · 2023-05-19T18:26:18Z

The reason you are seeing both SUCCESS as well as a FATAL message is that after the init command finishes the curve command is automatically run. This is done to print the fan curve that has been measured during the init command.

Can you try running the fan2go fan curve command individually to see if the same error occurs?

piyueh · 2023-05-19T19:22:08Z

Can you try running the fan2go fan curve command individually to see if the same error occurs?

No errors when doing fan2go fan curve.

markusressel · 2023-05-19T19:57:16Z

Hmm that's odd. The init command should raise the same error as the curve command if the configuration is invalid.

Do you still get the error even after fixing the permissions?

Do you get an error when running fan2go config validate?

piyueh · 2023-05-19T20:12:19Z

Do you still get the error even after fixing the permissions?

I haven't actually fixed the permission error. I think the permission issue (#229) is something beyond my current Linux knowledge. Like, I don't know why sudo is not working while an actual root works. Also, even now pwm1_enable already has the permission -rw-r--r--, but something as simple as cat pwm1_enable as a root still gives the Operation not supported error. It probably has something to do with file attributes or access control lists. I'm still figuring it out. But anyway, I agree #229 seems to be the problem of my filesystem and has nothing to do with fan2go.

Once I figure the permission issue out, I'll update here if this issue is also gone or not.

Do you get an error when running fan2go config validate?

Nope. No error from fan2go config validate.

mks-h · 2023-09-10T19:20:19Z

I also face similar issue when running sudo fan2go fan init -i fan_cpu:

Log

mks@vostro:~$ sudo fan2go fan init -i fan_cpu
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
 INFO  Using persistence at: /etc/fan2go/fan2go.db
 INFO  Deleting existing data for fan 'fan_cpu'...
 INFO  Computing pwm map...
 INFO  Measuring RPM curve...
 SUCCESS  Done!
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
 WARNING  Cannot send notification, missing env variable 'DISPLAY'!
  FATAL   fan fan_cpu: must have one of index or rpmChannel, must be >= 1
panic: 

goroutine 1 [running]:
github.com/pterm/pterm.checkFatal(...)
	/home/mks/go/pkg/mod/github.com/pterm/[email protected]/prefix_printer.go:351
github.com/pterm/pterm.(*PrefixPrinter).Printfln(0x10e8020, {0xc0004d3440, 0x3f}, {0x0, 0x0, 0x0})
	/home/mks/go/pkg/mod/github.com/pterm/[email protected]/prefix_printer.go:293 +0x1ed
github.com/markusressel/fan2go/internal/ui.Fatal({0xc0004d3440, 0x3f}, {0x0, 0x0, 0x0})
	/home/mks/Projects/fan2go/internal/ui/logging.go:52 +0x8b
github.com/markusressel/fan2go/cmd/fan.glob..func1(0x10e80e0?, {0xbba265?, 0x5?, 0x0?})
	/home/mks/Projects/fan2go/cmd/fan/curve.go:27 +0xe5
github.com/markusressel/fan2go/cmd/fan.glob..func2(0x10ecb40?, {0xc000078280?, 0x2?, 0x2?})
	/home/mks/Projects/fan2go/cmd/fan/init.go:55 +0x2f2
github.com/spf13/cobra.(*Command).execute(0x10ecb40, {0xc000078260, 0x2, 0x2})
	/home/mks/go/pkg/mod/github.com/spf13/[email protected]/command.go:940 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0x10eb440)
	/home/mks/go/pkg/mod/github.com/spf13/[email protected]/command.go:1068 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
	/home/mks/go/pkg/mod/github.com/spf13/[email protected]/command.go:992
github.com/markusressel/fan2go/cmd.Execute()
	/home/mks/Projects/fan2go/cmd/root.go:88 +0xf9
main.main()
	/home/mks/Projects/fan2go/main.go:25 +0x17

My config

fans:
  - id: fan_cpu
    hwmon:
      platform: dell_smm-isa-0000
      rpmChannel: 1
      pwnChannel: 1
    neverStop: true
    curve: curve_average
sensors:
  - id: sensor_cpu1
    hwmon:
      platform: dell_smm-isa-0000
      index: 1
  - id: sensor_cpu2
    hwmon:
      platform: dell_smm-isa-0000
      index: 2
  - id: sensor_other
    hwmon:
      platform: dell_smm-isa-0000
      index: 3
curves:
  - id: curve_cpu1
    linear:
      sensor: sensor_cpu1
      min: 30
      max: 60
  - id: curve_cpu2
    linear:
      sensor: sensor_cpu2
      min: 30
      max: 60
  - id: curve_other
    linear:
      sensor: sensor_other
      min: 30
      max: 60
  - id: curve_average
    function:
      type: average
      curves:
        - curve_cpu1
        - curve_cpu2
        - curve_other

And I also don't have any issues (and any curves) when running the sudo fan2go fan curve -i command.

mks@vostro:~$ sudo fan2go fan curve -i curve_average
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
mks@vostro:~$ sudo fan2go fan curve -i curve_cpu1
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
mks@vostro:~$ sudo fan2go fan curve -i curve_cpu2
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml
mks@vostro:~$ sudo fan2go fan curve -i curve_other
 INFO  Using configuration file at: /etc/fan2go/fan2go.yaml

P.S. Config validates alright

markusressel · 2023-09-10T19:23:35Z

@mks-h There is a typo in your config:

pwnChannel
pwmChannel

Although I am not sure if thats the actual issue 🤔

mks-h · 2023-09-10T19:27:02Z

There is a typo in your config:

I guess this deserves another issue that config validation doesn't really validate :)

Fixed the typo, but the problem stays.

markusressel · 2023-09-10T19:31:52Z

Well, the config is still valid even with this typo, since pwmChannel can use a fallback if not specified. Currently it is not an error if there are additional/unknown properties in the config. Maybe this should be changed, but that would require some sort of config file schema, which currently doesn't exist, and I am not sure if there are good tools to do this in golang.

mks-h · 2023-09-11T02:22:24Z

The issue is that the GetFans function sets Input to a sequential number while discovering all fans (len(result) + 1 where result []fans.HwMonFan{}). It does so right after the first Validate call when executing sudo fan2go fan init -i fan_cpu. So it escapes the validation at first. Then, when it proceeds to run curveCmd, the Validate is run again, and it catches the new value.

I'm not sure how to solve the issue, though — so I'm leaving it up to you.

markusressel · 2023-09-17T23:03:23Z

Thx for investigating, however, I am not sure I can follow your logic 🤔
The first thing the fan init does is run getFan(fanId), which in turn validates the configuration as it is before proceeding to run hwmom.GetChips(), which I think is the method you are referring to?

See:

https://github.com/markusressel/fan2go/blob/04ce45b7a6efe8b1ee87ae61aa032c6bb7328da0/cmd/fan/init.go#L21C5-L21C5

fan2go/cmd/fan/fan.go

Line 36 in 04ce45b

err := configuration.Validate(configPath)

fan2go/cmd/fan/fan.go

Line 41 in 04ce45b

controllers := hwmon.GetChips()

mks-h · 2023-09-19T01:27:32Z

Yes. And then GetChips runs GetFans a little down the file, which set's up the default values for each fan:

https://github.com/markusressel/fan2go/blob/master/internal/hwmon/hwmon.go#L201

…figuration-twice-breaks-validation reset Configuration data before unmarshalling the config

markusressel · 2023-09-27T20:18:00Z

@mks-h @piyueh Please try again with latest dev commit.

piyueh added the bug Something isn't working label May 19, 2023

This comment was marked as off-topic.

Sign in to view

This was referenced Sep 27, 2023

Confusing error message. Need help setting up. #258

Closed

reset Configuration data before unmarshalling the config #259

Merged

markusressel closed this as completed in #259 Sep 27, 2023

markusressel added a commit that referenced this issue Sep 27, 2023

Merge pull request #259 from markusressel/bugfix/#230-loading-the-con…

1b392d9

…figuration-twice-breaks-validation reset Configuration data before unmarshalling the config

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only setting one of `index` or `rpmChannel` is not enough #230

Only setting one of `index` or `rpmChannel` is not enough #230

piyueh commented May 19, 2023

markusressel commented May 19, 2023 •

edited

Loading

piyueh commented May 19, 2023 •

edited

Loading

markusressel commented May 19, 2023

piyueh commented May 19, 2023

This comment was marked as off-topic.

mks-h commented Sep 10, 2023 •

edited

Loading

markusressel commented Sep 10, 2023 •

edited

Loading

mks-h commented Sep 10, 2023

markusressel commented Sep 10, 2023

mks-h commented Sep 11, 2023

markusressel commented Sep 17, 2023 •

edited

Loading

mks-h commented Sep 19, 2023

markusressel commented Sep 27, 2023

Only setting one of index or rpmChannel is not enough #230

Only setting one of index or rpmChannel is not enough #230

Comments

piyueh commented May 19, 2023

markusressel commented May 19, 2023 • edited Loading

piyueh commented May 19, 2023 • edited Loading

markusressel commented May 19, 2023

piyueh commented May 19, 2023

This comment was marked as off-topic.

mks-h commented Sep 10, 2023 • edited Loading

markusressel commented Sep 10, 2023 • edited Loading

mks-h commented Sep 10, 2023

markusressel commented Sep 10, 2023

mks-h commented Sep 11, 2023

markusressel commented Sep 17, 2023 • edited Loading

mks-h commented Sep 19, 2023

markusressel commented Sep 27, 2023

Only setting one of `index` or `rpmChannel` is not enough #230

Only setting one of `index` or `rpmChannel` is not enough #230

markusressel commented May 19, 2023 •

edited

Loading

piyueh commented May 19, 2023 •

edited

Loading

mks-h commented Sep 10, 2023 •

edited

Loading

markusressel commented Sep 10, 2023 •

edited

Loading

markusressel commented Sep 17, 2023 •

edited

Loading