search

TamtamHero / fw-fanctrl

A simple systemd service to better control Framework Laptop's fan(s)
BSD 3-Clause "New" or "Revised" License
178 stars 33 forks source link

Adding support for framework-laptop-kmod as an alternative to ectool #47

Open NAKlama opened 2 months ago

NAKlama commented 2 months ago

Since there is a kernel module (https://github.com/DHowett/framework-laptop-kmod ) allowing fan control over sysfs, I added the ability to use that instead of ectool. It would allow those unhappy with the supplied binary to use the kernel module instead.

NAKlama commented 2 months ago

I've also added using the GPU temperature as well (as an option). Since the Framework 16 uses the same fan to cool the CPU and GPU, a high GPU temperature with a low CPU temperature would not cause the fans to speed up, this way it does.

leopoldhub commented 2 months ago

Hello, thank you for your contribution, I will check your various PRs in detail in about 3 hours.

At first glance, this one looks pretty good but I think all these conditions add a lot of complexity (and more in the future if other ways to get temperature/control the fans are implemented as well). Would you mind making FanController abstract and creating inherited subclasses for ectool and kmod instead?

This should reduce the complexity and make the code a bit easier to read.

NAKlama commented 2 months ago

I can do that.

NAKlama commented 2 months ago

How is this?

NAKlama commented 2 months ago

Did a quick test using both ectool and framework-laptop-kmod. Nothing thorough, but had it running and changed strategies for a couple of minutes each.

leopoldhub commented 2 months ago

Hi, thanks for your hard work! I have left some comments on your modifications. Please let me know if you have any questions

leopoldhub commented 1 week ago

Hi @NAKlama,

Are you still working on it? If not, I will close this PR in about a week.

Have a nice day