Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maxPwmChangePerCycle buggy, keeps fan off #326

Open
jwefers opened this issue Nov 20, 2024 · 3 comments
Open

maxPwmChangePerCycle buggy, keeps fan off #326

jwefers opened this issue Nov 20, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@jwefers
Copy link
Contributor

jwefers commented Nov 20, 2024

Thanks for picking up my PR and making the direct algo happen. I think i dis

Describe the bug
when using controlAlgorithm.direct.maxPwmChangePerCycle with any value, it wants to set the correct pwm value, but then sets the fan speed to zero instead.

As last time, i'll try to properly debug this when i find time, but right now, Stalker 2 was more important.

To Reproduce
Steps to reproduce the behavior:

config:

fans:
  - id: myfan
    hwmon:
      platform: nct6792-isa-0290
      rpmChannel: 3
      pwmChannel: 3
    neverStop: true
    curve: topfans_curve
    controlAlgorithm:
        direct:
            maxPwmChangePerCycle: 10
    pwmMap:
      0: 0
      255: 255

I assume curve and sensor do not matter, use anything.

What happens is, Debug shows correct desired PWM, but actually set PWM is zero and fan stays off.
When just using direct without maxPwmChangePerCycle, it works as expected.

Expected behavior
As temp in curves are changing, target PWM is slowly approached

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • Distro: Arch
  • uname -a: Linux xxx 6.11.9-arch1-1 1 SMP PREEMPT_DYNAMIC Sun, 17 Nov 2024 16:06:17 +0000 x86_64 GNU/Linux
  • sensors -v: sensors version 3.6.0+git with libsensors version 3.6.0+git
  • fan2go version: 0.9.0
@jwefers jwefers added the bug Something isn't working label Nov 20, 2024
@jwefers jwefers changed the title maxPwmChangePerCycle buggy, turns fans off maxPwmChangePerCycle buggy, keeps fan off Nov 20, 2024
@markusressel
Copy link
Owner

Hi @jwefers , thx for reporting.

I am not sure what was going on with this code, I might have let copilot go a bit far without thinking about it 😄
Please have a look at the PR I have created, it simplifies the logic greatly and should fix the issue. I am unsure about the "timedelta" component though, so input from a user that actually has this usecase would be great.

#327 (comment)

Thx!

markusressel added a commit that referenced this issue Nov 28, 2024
…Cycle-keeps-fan-off

reworked DirectControlLoop to fix custom values for "maxPwmChangePerCycle"
@markusressel
Copy link
Owner

As mentioned in #327 I have merged the pending PR.
@jwefers Please let me know if the implementation makes sense to you.

@jwefers
Copy link
Contributor Author

jwefers commented Nov 30, 2024

i'll get to it eventually, its not urgent - i'll get around to investigate/test the fix. Thanks for staying on it

ccat3z added a commit to ccat3z/xyz.ccat3z.nix that referenced this issue Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants