Support Smart-Home-Interface Part 1: Field definitions #199
Conversation
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Guzz-T
force-pushed
the
issue/190/definitions
branch
from
a35ce11 to
f8a11e3
Compare
|
The previously added class still requires revision. Therefore, I have removed everything except the field definitions and the new data types. Please merge if everything is fine. |
Guzz-T
changed the title
luxtronik/datatypes.py
Outdated
| 0: "Normal", # No correction | ||
| 1: "Increased", # Increase the temperature by the values within the SHI-settings | ||
| 2: "Increased", # Increase the temperature by the values within the SHI-settings | ||
| 3: "Decreased", # Decrease the temperature by the values within the SHI-settings |
There was a problem hiding this comment.
Just out of curiousity: Is there any reference for the exact meaning of those values? I assume it is correct (due to the fact that you're making use of this already), but I'm somewhat surprised that there are two values for Increased.
There was a problem hiding this comment.
Unfortunately, there is no documentation for the fields. At least with the holdings, you have the display on the controller itself. I don't know why there are (apparently) two equivalent values here. However, the behavior was the same twice on my heat pump.
luxtronik/datatypes.py
Outdated
| 0: "No-Limit", | ||
| 1: "Soft-Limit", | ||
| 2: "Hard-Limit", |
There was a problem hiding this comment.
Nit-picking: Where are the dashes (-) coming from as separator? Are whitespaces allowed here?
For instance in ModeState you're using spaces.
There was a problem hiding this comment.
You're right, it's not entirely consistent here. The heat pump also uses names with spaces.
| "type": LevelMode, | ||
| "writeable": True, | ||
| "since": "3.92.0", | ||
| "description": "Increase or decrease the temperature by the SHI-offsets-setting", |
There was a problem hiding this comment.
SHI is being used here without any introduction. Probably it would be helpful to have a short documentation about SHI in the top of the class, so anyone new to this project knows that this refers to "Smart Home Interface".
There was a problem hiding this comment.
I really like the clean approach and the new class / holding definition. Especially the idea to store the firmware version since when a particular field is available.
I commented on some smaller details (mostly nit-picking), don't see any overall problems at all.
|
Thanks for the review, I've incorporated the points. |
First pull request to gradually integrate the smart home interface. This pull request includes:
Part1 for #190