Added support to washer-dryers to override some of the settings defined when a course is selected. Partial implementation of feature request #596 #716
Conversation
|
@ollo69 I see some checks need to be run. If you send me a pointer to the documentation I will try to run them. Regards John |
| self._course_overrides: dict | None = {} | ||
| self._course_overrides_lists: dict | None = { | ||
| 'temp': ['TEMP_COLD', 'TEMP_20', 'TEMP_30', 'TEMP_40', 'TEMP_60', 'TEMP_95'], | ||
| 'spin': ['NO_SPIN', 'SPIN_400', 'SPIN_800', 'SPIN_1000', 'SPIN_Max'], | ||
| 'rinse': ['RINSE_NORMAL', 'RINSE_PLUS']} | ||
| # Need to initialise this list so that the UI can setup the select drop down. | ||
| # A better solution would be to generate this information from the data returned ThinQ API. |
There was a problem hiding this comment.
This values can be different depending on device model and should be retrieved from model info. This could work for your device but not for other. I need to be sure that only supported modes can be used.
There was a problem hiding this comment.
I agree. I have not looked at custom components or programmed in Python until I created this PR, so picking apart the model info was beyond me for this first step. I also did not know how to avoid a race condition between the HA framework setting up selectors and retrieving the data from model info. This does not appear to be a problem for the course names, so it must be doable. I can look further at this subject to my comments below.
There was a problem hiding this comment.
This values can be different depending on device model and should be retrieved from model info.
Now retrieved from model info.
| @property | ||
| def select_temp_enabled(self) -> bool: | ||
| """Return if select temp is enabled.""" | ||
| return self._select_enabled('temp') | ||
|
|
||
| async def select_start_temp(self, temp_name: str) -> None: | ||
| """Select a secific water temperature for remote start.""" | ||
| self._select_start_option('temp', 'Water Temp', temp_name) | ||
|
|
||
| @property | ||
| def select_rinse_enabled(self) -> bool: | ||
| """Return if select rinse is enabled.""" | ||
| return self._select_enabled('rinse') | ||
|
|
||
| async def select_start_rinse(self, rinse_name: str) -> None: | ||
| """Select a secific rinse option for remote start.""" | ||
| self._select_start_option('rinse', 'Rinse Option', rinse_name) | ||
|
|
||
| @property | ||
| def select_spin_enabled(self) -> bool: | ||
| """Return if select spin is enabled.""" | ||
| return self._select_enabled('spin') | ||
|
|
||
| async def select_start_spin(self, spin_name: str) -> None: | ||
| """Select a secific spin for remote start.""" | ||
| self._select_start_option('spin', 'Spin Speed', spin_name) | ||
|
|
There was a problem hiding this comment.
This will make all the logic very complex, because there are different combination that can vary depending on selected course. I would prefer to just add some parameter to the service call and allow to change this value calling the HA service with prober parameters, that will be ignored if not valid.
There was a problem hiding this comment.
@ollo69 ,
This will make all the logic very complex, because there are different combination that can vary depending on selected course
You are correct the ability to override a temp, spin or rinse depends on the WM model and the course selected. The PR handles this by extracting the necessary data for course info and storing it in _course_overrides_lists. The code should not required updating for different or future VM models unless a new setting is introduced (e.g. amount washing liquid to use) and we want to provide the user with a GUI selector to set it. Background: In the GUI and API the temp, spin and rinse select elements are not enabled until a course is selected that permits them to be overridden. For example, if the course Cotton is selected temp, spin and rinse are enabled, while for the course Allergy Care only spin and rinse are enabled. For each enabled override, a list of allowed values is extracted from from the course description. When a user attempts to set a value it is checked against this list. If the value is not in the list it is ignored and the user is informed of the permitted values by raising ServiceValidationError.
I would prefer to just add some parameter to the service call and allow to change this value calling the HA service with prober parameters, that will be ignored if not valid
When I started this work you had not publish the change to remote_start that allowed a course name string to be passed as a parameter. I have since studied your change and learnt how an integration can define a service that can be used by an automation or script.
I feel it is very easy to add another parameter to remote_start that accepts a JSON dictionary of overrides. The method can check _course_overrides_lists to confirm if the setting (e.g. spin speed) can be overridden and that the value is correct. An example call would be remote_start('Eco 40-60', '{"temp": "TEMP_60", "spin": "SPIN_400"}')
If you are happy with the above approach I am happy to modify the PR. I feel it will be a simple change. If you are thinking of something different please let me know.
I feel there is one problem we should agree on. For course names users can discover the valid course names that can be passed to remote_start using the course selection entity on the integration GUI. There may be another method that I have not found. How will a user, for a given course, discover setting that can be overridden and their permitted values that can be passed to remote_start. The selectors for temp, spin and rinse in the integration GUI will provide this information to the user. If you don't wish to have these, how do you think we can expose the information in _course_overrides_lists to the user?
In summary:
- I like the idea of adding overrides to
remote_start. It gives a cleaner HA script. - I believe the code will handle all current and future course settings and WM models.
- Without a discovery mechanism for the overrides that is simple for the user I don't feel we have usable solution. I am therefore reluctant to remove the temp, spin and rinse selectors if we don't provide an alternative. Nevertheless, this is your call.
Regards
John
There was a problem hiding this comment.
@ollo69 I had all the logic in place to add overrides to remote_start so I did it and update the PR. Most of the needed code was to help the user understand what to do when the remote_start service call fails by displaying meaning full messages.
I have not removed the selectors yet. I am waiting your feed back, see above.
| ThinQSelectEntityDescription( | ||
| key="temp_selection", | ||
| name="Set Water Temp", | ||
| icon="mdi:tune-vertical-variant", | ||
| options_fn=lambda x: x.device.temps_list, | ||
| select_option_fn=lambda x, option: x.device.select_start_temp(option), | ||
| available_fn=lambda x: x.device.select_temp_enabled, | ||
| value_fn=lambda x: x.device.selected_temp, | ||
| ), | ||
| ThinQSelectEntityDescription( | ||
| key="rinse_selection", | ||
| name="Set Rinse Option", | ||
| icon="mdi:tune-vertical-variant", | ||
| options_fn=lambda x: x.device.rinses_list, | ||
| select_option_fn=lambda x, option: x.device.select_start_rinse(option), | ||
| available_fn=lambda x: x.device.select_rinse_enabled, | ||
| value_fn=lambda x: x.device.selected_rinse, | ||
| ), | ||
| ThinQSelectEntityDescription( | ||
| key="spin_selection", | ||
| name="Set Spin Speed", | ||
| icon="mdi:tune-vertical-variant", | ||
| options_fn=lambda x: x.device.spins_list, | ||
| select_option_fn=lambda x, option: x.device.select_start_spin(option), | ||
| available_fn=lambda x: x.device.select_spin_enabled, | ||
| value_fn=lambda x: x.device.selected_spin, | ||
| ), |
There was a problem hiding this comment.
As in my previous comment, I prefer to not use a select but just provide additional parameter to be used in the remote_start service.
There was a problem hiding this comment.
Please see my comments above.
|
@ollo69 I have updated this PR to address the issues you raised. I have not removed the selectors yet. I am waiting for your feedback to my question about a discovery mechanism. |
This PR provides entities for overriding the temperature, spin and rinse options set-up when a washer course is selected. It checks that override values are permitted for the selected course and provide an error message if an invalid value is selected.
I have developed this because I regularly use the same course with a different water temperature to the default. I have tested this code using HA script and lovelace using a machine in the UK. It should also work for a machine in a different region, but I cannot test this.