EvanMulawski / FanControl.CorsairLink

The unofficial CorsairLink plugin for Fan Control. Adds support for Corsair controllers, liquid coolers, and power supplies. An alternative to iCUE.
125 stars 10 forks source link

Stopping Threshold Ignored at Startup When using Offset #146

Closed Phelagor closed 5 months ago

Phelagor commented 5 months ago

Short description

It seems that the threshold for stopping a fan is ignored at App start and when using "refresh sensors detection".

Hardware

I'm using an iCue Commander Pro with multiple Fans. Three of them are PWM (4-Pin), the fourth is DC (3-Pin).

FanControl.CorsairLink Version

v1.5.1 v1.6.0-beta.1

Detailed Issue description

The following occurs (similar for all Fans on Commander Pro):

Settings: I got the Fan set to starting at 42% and stopping at 41%. I set an Offset of 40% and a minimum of 0%. The Fan is triggered by a graph curve that is at 0% until temps hit a certain point (60°C) and then goes up to 5% and so on.

Result: What I get is the fan set to 40% which also results in it spinning.

corsair_fan_issue

Expected: When starting the system the temps are way below the 60°C and the curve is sending 0%. So the fan would be set to 0%+40%=40%, since below stopping threshold (41%) this would result in 0%.

Quick-fix: When confirming the Offset in the App (Change value once) it does exactly what is expected: Below stopping threshold => 0% Fan speed despite an offset of 40%.

corsair_fan_issue-2

Additional Information

Using the Fan Headers of my Mainboard for my AIO I can see that the normal Fan Control Controllers do exactly what I expect. (Stop the fans when offset + set speed is below stopping threshold). The Fans of AIO are the same type as the PWMs on the Commander Pro, so there is no Issue with the fans.

Use Case

All those fans are Case fans. They are triggered by a Temp Curve (Graph) The Curve is sending 0% until the temp hits a certain point, so during idle the PC is quiet. The PWM Fans spin at the same speed between 1% and 24%, so there is no point to use this range. Just stop the fans when 23% or below (set the stop threshold to 23%). Since the same curve is used for multiple purposes it just starts at 5%, not specific to any fans (like the 24% threshold here). So I need the Offset to raise the set Speed (sent by the curve) to value above 24%.

The DC Fan is running at the same speed between 1% and 42%, so that's the same as for the PWM fans, just with a different Offset (+40%).

EvanMulawski commented 5 months ago

Hi @Phelagor, just to confirm, when you change the Offset to 39 or 41 and back to 40, the fan power is set to the expected value of 0%?

Phelagor commented 5 months ago

Yes. When changing the Offset to any other value and back to 40% the resulting speed is set to 0% (since value is below 41% which is stopping threshold). I mean when I change it to anything below the stopping threshold it gets set to 0% anyways. No need to change back to 40%.

It's easily reproduceable when you got the hardware. Just set a stopping threshold to a value. Set the Offset to anything below that value, no minimum value set and hit the "Refresh sensors detection". => Result: Fan speed is set to the offset value instead of 0%

EvanMulawski commented 5 months ago

@Rem0o Is this on your end or mine? An issue specific to the plugin backend?

I was able to reproduce this with my Commander PRO:

https://github.com/EvanMulawski/FanControl.CorsairLink/assets/1044791/c98b3015-7d45-4f30-b22d-405e5f11a95d

2024-02-09T00:13:37.6343724Z [DBG] 1000D (0D05002A93190A16): STATE
Requested power for channel 0: 0 %

(refresh)

2024-02-09T00:13:39.0380600Z [DBG] 1000D (0D05002A93190A16): STATE
Requested power for channel 0: 50 %  <-- call to Reset, plugin sets value to 50%
2024-02-09T00:13:41.5046693Z [DBG] 1000D (0D05002A93190A16): STATE
Requested power for channel 0: 34 %
Rem0o commented 5 months ago

I gave it a good look and tried to reproduce the issue first with a LHM sensor, then in a unit tests with only the control logic, and I can't get the control to set anything but 0 in that scenario, since the final line of code before setting the control sensor is: image

@EvanMulawski was your log for the requested % basically here? image

Or was it further down the line through the ChannelTrackingStore and all the logic you got before arriving at the actual HID call setting the power?

EvanMulawski commented 5 months ago

@Rem0o Added logging to Set and retested.

public void Set(float val)
{
    _logger.Debug("SENSOR_SET", $"{Name}: Value={val}");
    var intVal = (int)val;
    _value = intVal;
    _device.SetChannelPower(_sensor.Channel, intVal);
}

With the config in the video, this is the first log after plugin initialization at startup:

SENSOR_SET: 1000D (0D05002A93190A16) Fan #1: Value=34

After changing Offset to 5%:

SENSOR_SET: 1000D (0D05002A93190A16) Fan #1: Value=0

After refresh sensor detection:

SENSOR_SET: 1000D (0D05002A93190A16) Fan #1: Value=35
Rem0o commented 5 months ago

@EvanMulawski What default value is applied when a control is disabled? Is it the case that the default value set during the refresh is actually lower than the start/stop% ?

EDIT: I think I found why I couldn't do it: image

This state never happens with LHM and with this I can actually do the behavior. Will fix.

Phelagor commented 5 months ago

Looks like you found the reason. So it's in fan control not in this plugin? Will there be some kind of nightly build / Beta? Some Version to test if the issue is fixed?

Rem0o commented 5 months ago

@Phelagor I was able to identify the issue in the main fancontrol program on my dev machine without the plugin. It's a state-specific logic issue when the sensor value is set to null after a refresh. The reason it only happens with this plugin is that no other plugin AFAIK sets the sensor value to null like here, so I haven't taken into account this state. In any case FanControl should support that case and it will be fixed in V181.

Rem0o commented 5 months ago

V181

Phelagor commented 5 months ago

V181

Just updated and it works as expected. Stopping Threshold after restart / refresh is now considered as expected. The problem described in this issues seems to be fixed with V181. Thanks a lot for the great work to both of you 👍