thecynic / pylutron

MIT License
31 stars 42 forks source link

Add support for HVAC controls #88

Open wilburCforce opened 4 months ago

wilburCforce commented 4 months ago

The Lutron system supports HVAC controls and we should add these into pylutron for consumption by other systems like Home Assistant.

wilburCforce commented 4 months ago

I've already completed the coding for this to a large degree but I wanted to get some feedback on best approach. It was suggested to me that we use enums to store the data so as to abstract the Lutron specific values. I can see the value in that but it also would make the HVAC data different from the rest of the data. @cdheiser what are your thoughts on that? I'll update my repo accordingly and then submit a pull request for it where we can work out the final details as needed. I have the HA component working with the updates as well so these can release fairly close together.

cdheiser commented 4 months ago

@wilburCforce if you can point me to the work in progress, I can take a look and contemplate.

wilburCforce commented 4 months ago

@wilburCforce if you can point me to the work in progress, I can take a look and contemplate.

it's a bit of a mess at the moment but you'll get the idea. if you look at my master branch you'll see a version with an HVAC class around line 687 that has the enums starting to get worked in. This version is very alpha ish.... https://github.com/wilburCforce/pylutron/blob/master/pylutron/__init__.py

If you look at my HVACsupport branch you'll see a cleaner version that does not have the enums around line 693 https://github.com/wilburCforce/pylutron/blob/HVACsupport/pylutron/__init__.py

It still needs work and clean up but it's generally working. I ran the HVACsupport branch with my local HA for several months testing it out. There are a lot of details around temperature units to work out and I want to clean up the naming. So don't be too critical :-)

Really I'm just asking if you think we should use the enums since they aren't used anywhere else. On one hand, it will be more readable. On the other, the code bloats because we need to look up the values before reading or writing them. Essentially, the repeater will answer with say a 1 and we will convert that to "cool". Similarly, when an integration wants to set the value it will set "cool" and we will look that up and send a 1. (hypothetical example, not real values).

Let me know if you think it is worth it or would rather just keep the raw values. I'm leaning to raw values myself just to reduce complexity.

wilburCforce commented 4 months ago

Work to resolve this issue is now complete. I've entered PR https://github.com/thecynic/pylutron/pull/92