Add support for HVAC units#92
Conversation
|
@cdheiser I believe this is ready now. I've done extensive testing with the HA integration that is just about done as well. |
|
@cdheiser or @thecynic I think this is probably worthy of bumping the release to 0.3.0 since this adds a major component. |
| """Handles an event update for this object, e.g. temp level change.""" | ||
| _LOGGER.debug("HVAC handle_update %d -- %s" % (self._integration_id, args)) | ||
|
|
||
| def _u_current_temp_f(temp): |
There was a problem hiding this comment.
Something about this makes me uneasy enough that I'm contemplating alternative arrangements. I just don't have an excellent idea yet.
The one thing rattling around my head is what should the object model of an HVAC be?
Is it one object with a bunch of properties? Or is the fan an object, and the operating mode an object, etc...
It feels like there's a lot of repeated boilerplate below that could be factored out and make it easier to read and parse. Like an HVACTemperatire class that can deal with the current temperature and setpoints (you end up with one instance for Farenheight and one for Celcius), and an HVACModes class, etc...
There was a problem hiding this comment.
I'm not opposed to the idea at all but that might also introduce more complexity and not save much actual code in the long run. Additionally, when consuming this in HA there are crossovers between modes, fans, and presets. Having multiple objects makes it more complicated to consume and increases the code size there.
| HVAC.Event.ECO_MODE: (_u_eco_mode, 1), | ||
| } | ||
| if event in handler_functions: | ||
| handler, num_args = handler_functions[event] |
There was a problem hiding this comment.
Why not just pass all remaining arguments?
There was a problem hiding this comment.
I'm a little new to python so I'm not sure how to do that. If you could suggest an approach so I could learn that would be great.
There was a problem hiding this comment.
You'd replace this all with:
handler(*args[1:])
temp template version change - to be reverted
|
@cdheiser Is there anything here really worth delaying this? |
|
@cdheiser This is the 1 year anniversary of waiting for your review. |
2 participants
This PR aims to add support for basic HVAC controls.
enums are used to store friendly text instead of integers so that applications utilizing this code do not need to have a dictionary to know what is happening. Similarly, those applications will write the values as the strings.
due to the number of attributes related to this object, the handle_update function has several helper functions and an attempt was made to streamline that code.