Use multiple reads when creating curves and exclude "bad readings"

[potens1]

Hi,

first of all, thank you for the time you spent writing this software. I'm currently evaluating replacing fancontrol with this

Is your feature request related to a problem? Please describe.
When the software creates its DB of curves, it ramps the fan, but sometimes, there are bad readings that should be excluded.

Examples:

First creation :

IN_Front
               
  Start PWM  23
  Max PWM    25

 48214 ┤         ╭╮
 45000 ┤         ││
 41785 ┤         ││
 38571 ┤         ││
 35357 ┤         ││
 32143 ┤        ╭╯│
 28928 ┤        │ │
 25714 ┤        │ │
 22500 ┤        │ │
 19286 ┤        │ │
 16071 ┤        │ │
 12857 ┤        │ │
  9643 ┤        │ │
  6429 ┤        │ │
  3214 ┤        │ │
     0 ┼────────╯ ╰────────────────────────────────────────────────────────────────────────────────────────
                                                     RPM / PWM


OUT_Top
               
  Start PWM  0 
  Max PWM    58

 4917 ┤                      ╭╮
 4589 ┤                      ││
 4261 ┤                      ││
 3933 ┤                      ││
 3606 ┤                      ││
 3278 ┤                      ││
 2950 ┤                      ││
 2622 ┤                      ││
 2294 ┤                      ││
 1967 ┤                      ││
 1639 ┤                      ││
 1311 ┤                      ││
  983 ┤                      ││
  656 ┤                      ││                                           ╭───────────────────────────────
  328 ┤                      ││               ╭───────────────────────────╯
    0 ┼──────────────────────╯╰───────────────╯
                                                    RPM / PWM

Second DB creation (after delete)

IN_Front
               
  Start PWM  23
  Max PWM    25

 29471 ┤        ╭╮
 27506 ┤        ││
 25541 ┤        ││
 23577 ┤        ││
 21612 ┤        ││
 19647 ┤        ││
 17682 ┤        ││
 15718 ┤        ││
 13753 ┤        ││
 11788 ┤        │╰╮
  9824 ┤        │ │
  7859 ┤        │ │
  5894 ┤        │ │
  3929 ┤        │ │
  1965 ┤        │ │
     0 ┼────────╯ ╰────────────────────────────────────────────────────────────────────────────────────────
                                                     RPM / PWM


OUT_Top
                
  Start PWM  0  
  Max PWM    249

 682 ┤                                                                                           ╭───────
 636 ┤                                                                                    ╭──────╯
 591 ┤                                                                             ╭──────╯
 545 ┤                                                                       ╭─────╯
 500 ┤                                                                 ╭─────╯
 454 ┤                                                           ╭─────╯
 409 ┤                                                      ╭────╯
 364 ┤                                                 ╭────╯
 318 ┤                                             ╭───╯
 273 ┤                                          ╭──╯
 227 ┤                                         ╭╯
 182 ┤                                        ╭╯
 136 ┤                                        │
  91 ┤                                       ╭╯
  45 ┤                                       │
   0 ┼───────────────────────────────────────╯
                                                   RPM / PWM

Describe the solution you'd like
I think it could be a solution to read multiple times the speed, with some delay to ensure the fan had time to ramp up, and exclude the min and max

Describe alternatives you've considered
quite the same as above, but doing a comparison with the previous reading, if it was something else than 0, check the diff. but since every fan is different, it's difficult (impossible?) to find the right threshold to apply, even if, in my case, we are talking about difference in the 10's of 1000s RPM difference...

Additional context
I don't know if it make sense, but maybe add an option to rebuild the curve of a specific fan ?
I don't know either if those parameters are applied to the build curve process (or any other from the conf, but the definition of the fan):

rpmPollingRate: 1s
rpmRollingWindowSize: 10

but it sounds like this one

    startPwm: 102

is not.

I hope all of this makes sense and is not due to me not understanding the usage.

[markusressel]

Hey @potens1 🤓

We had a similar issue in #28, however your curve looks different, because it does start at 0, but then seems to jump up, while still spinning down?!

I think it could be a solution to read multiple times the speed, with some delay to ensure the fan had time to ramp up, and exclude the min and max

In the beginning we had multiple measurements, but this just takes forever to finish, so I dumped it.
I am also thinking that it might be an option to skip a few values in between and just interpolate them linearly.

For the first measurement, there actually already is logic in place to wait for the fan to settle to a given speed.
It may not yet work as intended. After the inital measurement is done, each PWM value is measured with a 2 second delay, as you can see here:

fan2go/internal/controller/controller.go

Line 215 in cab80d2

time.Sleep(2 * time.Second)

I wanted to avoid waiting for the settling for the same reason as mentioned above: it takes forever.

I don't know if it make sense, but maybe add an option to rebuild the curve of a specific fan ?

I think its a good thing to have, since it might also be necessary when switching out fans.
So something like fan2go fan --id IN_Front curve measure, would do the trick.

I don't know either if those parameters are applied to the build curve process

No, (currently) they are not. The only configuration option for the initialization sequence is whether to initialize all fans in parallel or not.

Its not a fix for the measurement algorithm, but you can use the startPwm configuration option to force fan2go to use this value as the lowest possible PWM value where the fan is still able to start rotating. In combination with neverStop: true, the fan will never use a PWM value below your custom startPwm. Finding this value is a little tricky because the graphs don't show it (there is an open issue about this on the library I use for the graphs: See here), but with a little bit of trial and error, or following the output of fan2go detect while it is running the initialzation sequence should help.

[potens1]

Hi !
Thank you for your swift answer.

We had a similar issue in #28, however your curve looks different, because it does start at 0, but then seems to jump up, while still spinning down?!

Not exactly, when the jump is read, in fact the fan is still stopped and not moving, I guess there is either an electronic noise, either the circuit counting the RPM is doing an overflow or something or the firmware is bugged from the hell which can be perfectly possible.

So something like fan2go fan --id IN_Front curve measure, would do the trick.

Yes ! That would be perfectly sensible

about the:

In combination with neverStop: true, the fan will never use a PWM value below your custom startPwm

I'm not sure if you say it should be in use when doing the detect, or if you mean in day to day usage.
Because, in the detect, I got this (and all the previous and the followings)

Measuring RPM of IN_Front at PWM 36: 0
DEBUG   Measured RPM of 659 at PWM 36 for fan IN_Front

with this in the conf

  - id: IN_Front
    hwmon:
      platform: it8688-*
      index: 3
    neverStop: true
    curve: gpu_curve
    startPwm: 50

Also, I wonder if I did not find a bug (could open a bug report if you think it is) but, in this case, when I ask the curve, the output gives me this:

IN_Front
               
  Start PWM  50
  Max PWM    25

With a max smaller than the start. if that's just cosmetic, that's okay, but if the boltdb do not have the values after 25, that's not good (I've been trying to open it for about two hours, but not knowing go does not help a lot)

So, long story short, your proposition of the new fan action would be great, and for the multiple value, it would have been nice to make it optional (so being able to choose between the first way the soft worked, or the fast one). The problem with the interpolation would be, i.e. in my case that if the wrong value is taken as a ref for interpolation, it will absolutely make it unusable (so, it does not solve my use case).

Again, thanks a lot for your work on this (I mean the software, not the specific ticket).

[markusressel]

Not exactly, when the jump is read, in fact the fan is still stopped and not moving, I guess there is either an electronic noise, either the circuit counting the RPM is doing an overflow or something or the firmware is bugged from the hell which can be perfectly possible.

Huh, well fan2go will probably never be able to deal with invalid data points like that. However, adding more options to manually override the fan curve is definitely on the TODO list.

I'm not sure if you say it should be in use when doing the detect, or if you mean in day to day usage.

In day to day usage. Currently, the fan curve is basically only used to automatically detect the min and max PWM speed. If you override those values, the initialization becomes pointless. Thats also a thing I want to improve in the future.

Also, I wonder if I did not find a bug (could open a bug report if you think it is) but, in this case, when I ask the curve, the output gives me this:

IN_Front
               
  Start PWM  50
  Max PWM    25

With a max smaller than the start. if that's just cosmetic, that's okay, but if the boltdb do not have the values after 25, that's not good (I've been trying to open it for about two hours, but not knowing go does not help a lot)

I would expect the fan2go daemon to crash with an error if the max is less than the min, because it doesnt make sense. I would have to look into it to give more details. I am currently on vacation though ☺️

So, long story short, your proposition of the new fan action would be great, and for the multiple value, it would have been nice to make it optional (so being able to choose between the first way the soft worked, or the fast one). The problem with the interpolation would be, i.e. in my case that if the wrong value is taken as a ref for interpolation, it will absolutely make it unusable (so, it does not solve my use case).

Again, thanks a lot for your work on this (I mean the software, not the specific ticket).

The DB always has values for the full 0-255 range. However, for you as a user it doesn't really matter, because the db is only used for extrapolating max and min PWM anyway, the rest is linear interpolation. The intention was to use the db as a reverse lookup for RPM speeds at some point, but this is still a TODO, because handling fans without RPM Sensor and other edge cases make this non-trivial.

[Nyxiad]

It sounds like Kalman filters could be very useful here. I've used them in robotics when I have several measurements with varying degrees of accuracy and precision that I want to use to estimate the true state of the robot. This could be applied here (with one or multiple measurements) to filter out bad readings and/or combine multiple readings from different sources. I've never programmed using Go though, so it may be a while before I get around to contributing this feature.
If you're curious and want to learn more, I would check out kalmanfilter.net