GitHub/pyhOn
3
0
Fork 0
Code Issues 3 Pull Requests 1 Packages Projects Releases 91 Wiki Activity

Issue with sync_command #16

Merged
VadymMelnychuk merged 20 commits from main into main 2023-10-12 16:43:41 +02:00
Conversation 0 Commits 20 Files Changed 3 +14 -1
1 changed files with 14 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit bab8bce804 - Show all commits

15
pyhon/appliance.py
View File

@ -277,7 +277,13 @@ class HonAppliance:
_LOGGER.info("Can't set %s - %s", key, error) _LOGGER.info("Can't set %s - %s", key, error)
continue continue
def sync_command(self, main: str, target: Optional[List[str] | str] = None) -> None: def sync_command(
self,
main: str,
target: Optional[List[str] | str] = None,
mandatory_only: bool = False,
target_parameters: Optional[List[str]] = None,
) -> None:
base: Optional[HonCommand] = self.commands.get(main) base: Optional[HonCommand] = self.commands.get(main)
if not base: if not base:
return return
@ -288,6 +294,13 @@ class HonAppliance:
for name, target_param in data.parameters.items(): for name, target_param in data.parameters.items():
if not (base_param := base.parameters.get(name)): if not (base_param := base.parameters.get(name)):
continue continue
if mandatory_only and not target_param.mandatory:
continue
if target_parameters and name not in target_parameters:
continue
self.sync_parameter(base_param, target_param) self.sync_parameter(base_param, target_param)
def sync_parameter(self, main: Parameter, target: Parameter) -> None: def sync_parameter(self, main: Parameter, target: Parameter) -> None:
Andre0512 commented 2023-07-27 18:53:16 +02:00 (Migrated from github.com)
Review
Copy Link

This is important for other functions and will introduce problems for other appliances if we remove it. Please use startProgram.program to switch the mode instead of settings.machMode.

This is important for other functions and will introduce problems for other appliances if we remove it. Please use `startProgram.program` to switch the mode instead of `settings.machMode`.
VadymMelnychuk commented 2023-07-27 21:40:47 +02:00 (Migrated from github.com)
Review
Copy Link

Wait. Looks like my explanation is not so good. So. I use command ‘startProgram’ for turn on water heater. I have no any other ways to turn on appliance. Next. startProgram has 3 params: onOffStatus (mandatory, fixed) machMode (optional, fixed) tempSel (optional, fixed)

However, real settings.machMode is range, and has 1,2 and 3 modes (eco, max, bps). And it’s correct type of this parameter. I don’t know why manufacturer implement parameter ‘startProgram.machMode’ as fixed. But, we have what we have.

As result. I call start program - >sync command replace range of ‘settings.machMode’ with only one value. It’s incorrect. More over. I didn’t touch sync if base and target parameter are range. I removed sync code only for case if base parameter is fixed and target is range. That’s it. Let me know of you need more examples.

Wait. Looks like my explanation is not so good. So. I use command 'startProgram' for turn on water heater. I have no any other ways to turn on appliance. Next. startProgram has 3 params: onOffStatus (mandatory, fixed) machMode (optional, fixed) tempSel (optional, fixed) However, real settings.machMode is range, and has 1,2 and 3 modes (eco, max, bps). And it's correct type of this parameter. I don't know why manufacturer implement parameter 'startProgram.machMode' as fixed. But, we have what we have. As result. I call start program - >sync command replace range of 'settings.machMode' with only one value. It's incorrect. More over. I didn't touch sync if base and target parameter are range. I removed sync code only for case if base parameter is fixed and target is range. That's it. Let me know of you need more examples.
VadymMelnychuk commented 2023-08-02 20:31:57 +02:00 (Migrated from github.com)
Review
Copy Link

@Andre0512 Any comments or suggestions from your side?

@Andre0512 Any comments or suggestions from your side?
VadymMelnychuk commented 2023-10-04 10:57:30 +02:00 (Migrated from github.com)
Review
Copy Link

@Andre0512 hey man will we continue on this request? Tell me, should I wait or not

@Andre0512 hey man will we continue on this request? Tell me, should I wait or not
Andre0512 commented 2023-10-12 01:18:09 +02:00 (Migrated from github.com)
Review
Copy Link

Hi, sorry you had to wait so long! So I can`t merge this unfortunately if it removes line 301-303, this will break functionalities of some appliances…

If you change startProgram.program to ‘eco’ then machMode is 1, if you change startProgram.program to ‘max’ then machMode is 2 and so on. So you don’t need to change machMode itself at any time, you can work with changing startProgram.program and it’s ok that machMode is fixed, it will be updated with the changing program. This is how I implemented it for other appliances. The user can’t change machMode itself, you always need to change startProgram.program Is it understandable what I mean and can you try it? 🙂

Hi, sorry you had to wait so long! So I can`t merge this unfortunately if it removes line 301-303, this will break functionalities of some appliances... If you change `startProgram.program` to 'eco' then `machMode` is 1, if you change `startProgram.program` to 'max' then `machMode` is 2 and so on. So you don't need to change `machMode` itself at any time, you can work with changing `startProgram.program` and it's ok that `machMode` is fixed, it will be updated with the changing program. This is how I implemented it for other appliances. The user can't change `machMode` itself, you always need to change `startProgram.program` Is it understandable what I mean and can you try it? :slightly_smiling_face:
VadymMelnychuk commented 2023-10-12 15:37:51 +02:00 (Migrated from github.com)
Review
Copy Link

@Andre0512 Thanks. I understood the concept. It was not clear for me at the start. So, I rewrite my fork of hOn according to your explanation. All work. I revert back sync_command Should be fine now.

@Andre0512 Thanks. I understood the concept. It was not clear for me at the start. So, I rewrite my fork of hOn according to your explanation. All work. I revert back `sync_command` Should be fine now.