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 |
custom_components/smartthinq_sensors/wideq/devices/washerDryer.py
Outdated
Show resolved
Hide resolved
| @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. |
|
Hi @ollo69 |
|
@ollo69 I have updated this PR to address the issues you have raised. Also, I have added service validation checks and messages that will tell a service user the course options available and permitted values for their machine in the local language. I have not removed the selectors yet. If the selectors are the reason you will not take this PR then I will remove them. |
|
Unfortunately this year I'm very busy with my job and my time to work on this integration is very limited. |
|
@ollo69, as requested I have re-based this PR. |
|
Hi, I have merged this locally as I wanted to try it out, but I get the following error: My washer supports setting the temperature and course and such through the thinQ App and I could provide some mitmproxy JSON output if necessary for my model. Sadly I am not invested enough inside the wideq code to fix this right now. |
Hi @ollo69, |
|
Hi @lancasterJ , I have downloaded the latest version of your feature branch and loaded it into HA. My issue with select persisted, and that stopped the washer from populating info, so I had to comment out the I also added the debug log to rinse and spin, in addition to course and temp. With this, I was able to get the debug logs, though of course the selects now don't work because I never return actual information to it. This is the full debug log from my washer: Note that it wasn't turned on at this point, so that's probably why all options are shown and not specific ones for a course. I believe it could be a timing issue because the list populated a bit slowly, way slower than the courses list, and my error log from before looks like the select is trying to call in the list before it's created. I believe it could be a timing issue because the list populated a bit slowly, way slower than the courses list, and my error log from before looks like the select is trying to call in the list before it's created. |
…tion with the setup model_info
|
Hi @ollo69 I cannot find away to sync machine info loading with setting up the selectors, so I have removed the selectors. This means that the only way to override a default course setting is to use the SmartThinQ LGE Sensors: Remote Start action in a script. Invalid parameters will be logged. I recommend using Actions tab on the Developer Tools page for testing. Invalid values for the Programme and Option Overrides will be reported on this screen. This give the user a way to discovery valid course and override combinations. If this version works I need to do some code cleaning before you merge it. The same race condition must exist with the course_list function. Luckily, users are not seeing it. Regards |
|
Hi @lancasterJ, I'm not the dev but still tried out the new changes as I reported the issues with select and such in the first place. I used the following: Eco 40-60 with JSON {"temp": "TEMP_60", "spin": "SPIN_1400", "rinse": "RINSE_NORMAL"} and I was then able to remote start the washer with my desired program and parameters. Thank you for implementing this! @ollo69 I think once code-cleanup is done this would be a great addition to the integration and with a bit of proper documentation it could benefit lots of users. I would've still preferred the selects as they're more user-friendly but I think this will suffice for now. |
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.