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

Raise default MaxRpmDiffForSettledFan #249

Merged
merged 2 commits into from
Aug 13, 2023

Conversation

nivekuil
Copy link
Contributor

On my system this was stabilizing around 12-13 so init was stuck, leading to some time spent looking under the hood. I think it should be harmless to raise this a bit and maybe save someone else the trouble.

On my system this was stabilizing around 12-13 so fan2go was never terminating, leading to some time spent looking under the hood. I think it should be harmless to raise this a bit and maybe save someone else some trouble.
@markusressel
Copy link
Owner

Sounds like a good idea, better to have a "not so good" init than a stuck one.

One request: Please also change the value in the fan2go.yaml example configuration.
Thx!

@markusressel markusressel merged commit 7c86ff0 into markusressel:master Aug 13, 2023
1 check passed
@nivekuil
Copy link
Contributor Author

nivekuil commented Aug 13, 2023

updated. btw, why default runFanInitializationInParallel to false? It looks like you explicitly defaulted it to true in one commit here https://github.com/markusressel/fan2go/pull/32/commits but then it got lost by the last patch in the series. I would have liked parallel by default, makes for a better out of the box UX

GitHub
contributes to #28 This PR adds

a config option runFanInitializationInParallel to prevent fans from beeing initialized all at the same time, and initialize them one after another instead
a "settle...

@markusressel
Copy link
Owner

Hmm, yes that may have been an oversight. I don't think it matters that much though. Not using it may help with not overheating system components, while using it saves time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants