SasaKaranovic / OpenFanController

Open-source open-hardware PC fan controller for everyone!
https://sasakaranovic.com/projects/openfan-controller/
GNU General Public License v3.0
125 stars 12 forks source link

Fan group #13

Closed Th3M1k3y closed 2 months ago

Th3M1k3y commented 4 months ago

Being able to put fans into groups could be useful.

When a fan group is added, the group is added to the list of fans, and the fans in the group is hidden from the dropdown. Selecting the group and manually applying a fan setting would then set all the fans in the group to the selected manual setting.

Controlling fans through the API could be done something like

/api/v0/fan/{group}/rpm?value={rpm}

SasaKaranovic commented 4 months ago

Hey there! Thanks for the suggestion!

What would be the benefit of using fan groups instead of creating fan profiles? Or maybe you have a very specific use-case in mind that this feature would be helpful for and can't use profiles for it?

In my use-cases, either the fans are being controlled automatically by a third party app (ie FanControl on Windows) or each individual fan is controlled programmatically (through scripting/automation or on Unraid via OpenFAN Unraid Service)

Also consider that adding groups feature to the API would potentially make things very opaque. Consider the example you suggested /api/v0/fan/{group}/rpm?value={rpm}. It is not immediately obvious which fans are going to be set to this RPM value. Can a single fan be a part of two groups? If yes, what happens if there is a conflict (both groups get request for different RPM). Or if you remove a fan from a group, how doe the application on the other end (ie third party app) get notified of this and how should it handle this?

Honestly... I like and dislike this idea at the same time. Maybe because I don't fully understand how this would work or maybe because I'm a bit worried about introducing "unnecessary" complexity or confusing end-users.

But I would love to hear more on this. So please whenever you get a chance share your thoughts and other people please feel free to chime in if you have anything to add.

Th3M1k3y commented 4 months ago

Hey there! Thanks for the suggestion!

What would be the benefit of using fan groups instead of creating fan profiles? Or maybe you have a very specific use-case in mind that this feature would be helpful for and can't use profiles for it?

The group of fans would not replace the profiles, it would just group some or all fans together so all are controlled at the same time.

Also consider that adding groups feature to the API would potentially make things very opaque. Consider the example you suggested /api/v0/fan/{group}/rpm?value={rpm}. It is not immediately obvious which fans are going to be set to this RPM value. Can a single fan be a part of two groups? If yes, what happens if there is a conflict (both groups get request for different RPM). Or if you remove a fan from a group, how doe the application on the other end (ie third party app) get notified of this and how should it handle this?

Fans would only be able to be in one group, which is why fans in a group is removed from the dropdown list.

My idea was based on the case I got, where I got 3 fans in the front. Instead of making 3 calls to set the fan speeds, it could be set by just setting the group instead.

I didn't consider how an third party app would/should handle it. Showing which fan's was set could be an array in the returned JSON, and then leave it up to the app creator to decide if they want to handle changes.

Honestly... I like and dislike this idea at the same time. Maybe because I don't fully understand how this would work or maybe because I'm a bit worried about introducing "unnecessary" complexity or confusing end-users.

Maybe this isn't the best approach either, but maybe it can plant a seed for a better idea. :)

The thinking behind my idea, was like instead of connecting the fan's together with a cable, it could be done virtually and that way keep the individual rpm monitoring.

SasaKaranovic commented 4 months ago

The group of fans would not replace the profiles, it would just group some or all fans together so all are controlled at the same time.

My idea was based on the case I got, where I got 3 fans in the front. Instead of making 3 calls to set the fan speeds, it could be set by just setting the group instead.

Maybe a better and less convoluted solution would be to add another API path that allows you to set multiple fans at once? Something like /api/v0/fan/multiple/set?value=0:1200;1:1000;2:1500. This call would then result in Fan 0 -> 1200 RPM, Fan 1 -> 1000 RPM and Fan 2 -> 1500 RPM.

I didn't consider how an third party app would/should handle it. Showing which fan's was set could be an array in the returned JSON, and then leave it up to the app creator to decide if they want to handle changes.

I would like to avoid putting any additional burden on the external application(s). Especially since in some cases they can be very simple scripts that just make the API based on some event. For this reason it might make more sense (and it's probably less convoluted and more transparent) to keep this grouping information in the external application. Mainly because if the external application already knows that there is a need to control fans 0, 3, 5 as single unit, making multiple API calls (or one if the above API change is implemented) makes more sense than making changes in two places (one in the OpenFAN to configure groups and then another in the external app to control those groups).

Out of curiosity how are you using your OpenFAN and how are you automating your fan control? I am assuming you don't change fan speed multiple times a second (which you really should not be doing anyway), so making multiple API calls vs single API call for fan group should have barely any performance or resource benefit.

Th3M1k3y commented 4 months ago

Maybe a better and less convoluted solution would be to add another API path that allows you to set multiple fans at once? Something like /api/v0/fan/multiple/set?value=0:1200;1:1000;2:1500. This call would then result in Fan 0 -> 1200 RPM, Fan 1 -> 1000 RPM and Fan 2 -> 1500 RPM.

That would work too

Out of curiosity how are you using your OpenFAN and how are you automating your fan control? I am assuming you don't change fan speed multiple times a second (which you really should not be doing anyway), so making multiple API calls vs single API call for fan group should have barely any performance or resource benefit.

Right now I am using https://github.com/SasaKaranovic/OpenFanUnraidService in my server, but I am also going to make something of my own at some point. I am not concerned about performance, I just like the workflow where I build up what needs to happen, then send it all over in one go.

SasaKaranovic commented 3 months ago

I've added it on the to-do list but it's very low priority because you can get same result by making multiple requests. Obviously we can argue that making one request is more "elegant" but these requests should not be made manually anyway.

github-actions[bot] commented 3 months ago

This issue is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 3 days.

github-actions[bot] commented 2 months ago

This issue was closed because it has been stalled for 5 days with no activity.