introduce "bypassPwmMap" option to not calculate a pwm map for a fan

[jwefers]

Some fan controllers (such as my very new nct6687) take a good second between setting an rpm value and it reflecting back on read.

This either breaks the fan init sequence (see #358) or, if the delay is increased (in this case around 1s), makes it insufferably long.

This PR introduces a config option for a fan to bypass the pwm map calculation. During fan init, if this flag is set, a default 0=0..255=255 map is used as a configOverride.

Contributes to #272

[jwefers]

I wanted to add a test for this, but:

You have Config as a field of all Fan-implementing actual Fans. The code, as before, relies on a Config field being there to check if bypassPwmMap is set.

This is not the case for MockFan, and computePwmMap rightfully says "type is unknown" for a MockFan.

I propose that in addition to Config being a field in each struct, a GetConfig() func should be added to the Fan interface. This would remove some code duplication. I have not checked how much effort it would be for MockFan to carry a mocked config. Any advice?

[markusressel]

I wanted to add a test for this, but:

You have Config as a field of all Fan-implementing actual Fans. The code, as before, relies on a Config field being there to check if bypassPwmMap is set.

This is not the case for MockFan, and computePwmMap rightfully says "type is unknown" for a MockFan.

I propose that in addition to Config being a field in each struct, a GetConfig() func should be added to the Fan interface. This would remove some code duplication. I have not checked how much effort it would be for MockFan to carry a mocked config. Any advice?

I am not too happy about exposing the generic struct of the Config object, but we also already have the same in the Sensor implementation, so I guess it would be fine to add a GetConfig() method to the interface of a fan as well.

[markusressel]

I wonder how much effort it would be to - instead of adding an option to bypass the default logic - expand the configuration of the pwmMap paramter to allow subconfigurations, like f.ex. :

fan:
  ...
  pwmMap:
    linear:
      0: 0
      255: 255

fan:
  ...
  # would be the default
  pwmMap: autodetect

fan:
  ...
  pwmMap:
    values:
      0: 0
      1: 1
       ...

I am not too fond of configuration options that "disable other options" that may be specified at the same time, as it can be confusing what is actually applied.

[jwefers]

i think i prefer your idea with linear interpolation of a given pwmMap

[jwefers]

let me try tomorrow, but i think the GetConfig method is worthwile in some places, where you essentially have a switch over all actual fan types and then identical code in them.

Need to take care of other duties now

[markusressel]

I have reworked how the pwmMap is computed in #372 . I hope the added documentation makes it easier to understand what its purpose is. The proposal to skip the automatic detection and provide more options for specifying it manually are unaffected by this and are still a good idea.

[DanielGibson]

fan:
  ...
  pwmMap:
    values:
      0: 0
      1: 1
       ...

If this means that a user who just wants to have 1:1 mapping must type in a pwmMap with 255 entries, then that sounds way too tedious to be usable.

But of course if you're gonna allow pwmMap: autodetect you could also allow pwmMap: identity (or "1to1" or whatever)

[markusressel]

@DanielGibson yes, see: https://github.com/markusressel/fan2go/pull/363#issuecomment-2852014619

[DanielGibson]

@DanielGibson yes, see: #363 (comment)

Ah, you mean that linear: means that the values in between will just be linearly interpolated, ok, sounds reasonable I guess (though if you'll have autodetect detect keyword you could also add one for identity to make this common case even simpler)

[DanielGibson]

In #390 I have a similar solution (among several other related things), for now as a commandline option (fan2go fan -i myFan init --skip-auto-pwm-map or the short form -s), in the "fan2go fan init: new --skip-auto-pwm-map (-s) option" commit.

It prevents the automatic creation of the SetPwmToGetPwmMap in computeSetPwmToGetPwmMapAutomatically() (or rather just uses a 1:1 map there), which I think is the actually problematic part. However the normal PwmMap is still created based on the users configuration, if any (in computePwmMap() -> computePwmMapAutomatically())