Added Window-Heater Handling for Skoda Enyaq #62
Conversation
Added special handling for Skoda Enyaq to read the window heating status * added attributes to window_heater for front and rear window * read current status from airConditioning/windowsHeatingStatuses instead of airConditioningSettings/windowsHeatingEnabled Note: airConditioningSettings/windowsHeatingEnabled has been used before. as it it not clear, if this is needed by some car, it was kept and priorizied below the new code
Added special handling for Skoda Enyaq to read the window heating status * added attributes to window_heater for front and rear window * read current status from airConditioning/windowsHeatingStatuses instead of airConditioningSettings/windowsHeatingEnabled Note: airConditioningSettings/windowsHeatingEnabled has been used before. as it it not clear, if this is needed by some car, it was kept and priorizied below the new code
There was a problem hiding this comment.
The changes to dashboard.py is good for including the attributes of the window heater. It will only benefit the Enyaq though since the attributes included only will get populated for the Enyaq. Keep in mind that the Enyaq is hosted on it's own API (so far) and all other connected Skoda cars are hosted on another API with different endpoints and data structures.
The change would remove the timestamp attribute of when start/stop climatisation was last triggered and the result without adding anything new.
A better way to add the front and rear window heating attributes for the Enyaq would be to create a check if the attributes are supported and then include them as well.
I take that back, I forgot that the legacy API also have states for both front and rear windows. The last_result and last_timestamp should still be kept though. |
|
Overall a good addition. I will review the code some more to try and have the same functionality but with fewer rows of code. |
|
shall i add the last_result and last_timestamp again and re-pull or do you merge the code? |
I'll readd when I merge 👍🏻 |
|
oh, sorry, i just saw that my tests for git (commit within eclipse, add and delete files) seem to be forwarded to the whole pull-request i made. |
Minor changes, variable names and added potential error handling
| @@ -597,8 +597,8 @@ def assumed_state(self): | |||
| @property | |||
| def attributes(self): | |||
| return { | |||
| 'last_result': self.vehicle.climater_action_status, | |||
| 'last_timestamp': self.vehicle.climater_action_timestamp | |||
| "Front":self.vehicle.window_heater_attributes.get("Front", ''), | |||
There was a problem hiding this comment.
A better way is to keep the "last_result" and "last_timestamp" and then create a check for if "attributes" are supported, and if so include them as well.
skodaconnect/vehicle.py
Outdated
| @@ -2143,7 +2143,18 @@ def window_heater(self): | |||
| status_rear = self.attrs.get('climater', {}).get('status', {}).get('windowHeatingStatusData', {}).get('windowHeatingStateRear', {}).get('content', '') | |||
| if status_rear == 'on': | |||
| return True | |||
| elif self.attrs.get('airConditioningSettings', {}): | |||
| elif self.attrs.get('airConditioning', {}).get('windowsHeatingStatuses', False): | |||
There was a problem hiding this comment.
The "windowHeatingStatuses" should be redundant since this check is only to enter this code block if the vehicle is hosted on the new API (Enyaq)
There was a problem hiding this comment.
This whole def code block could be shorter if the specific "gets" are kept short for the different API flavors. Then a more generic check can be done for the status_front and status_rear to return True if heating is enabled for either front or rear and both API's.
Added special handling for Skoda Enyaq to read the window heating status
Note: airConditioningSettings/windowsHeatingEnabled has been used before. as it it not clear, if this is needed by some car, it was kept and priorizied below the new code