Add support to read demand control data #268
Conversation
|
I know you are very busy but maybe you find a little bit of time to check the first two PRs (this one and the corresponding one in iobroker.daikin). I just would like to know if I need to improve something or if they are already in a mergeable quality. These two PRs cover the "read" part for demand control. The other two (#269 and Apollon77/ioBroker.daikin#185) cover the "write" part and are implemented on top. The code runs since since six weeks successfully in my iobroker environment with two Stylish devices. Using this enhancement I was able to switch off the daikin-cloud adapter. |
|
Hi @Matze2, sorry for the very late review. One question: Do all devices have this option? Do you know what devices that do not have it return? ok i checked mine ... I get Seems like it always gets returned but "type=0" means not supported? In fact you now added it in a way that - in case of an error being returned - the sensor infos (which all have) would not be returned. Can you please adjust the order of the calls (put demand control latest and make sure it does not fail the whole chain? So maybe ignore errors there or such?) |
|
Hi @Apollon77 , I think I understood. |
|
Perfect, no hurries, I'm also on vacation next week :-) I try to be faster afterwards with reviews :-) |
To not risk that existing data cannot be retrieved anymore
Matze2
force-pushed
the
expose-demand-control-vars
branch
from
8b7ddf0
to
45f37d9
Compare
... and do not call it again. This means, first time the update will not work, an error is logged in the iobroker protocol. On the next update demand control polling is skipped.
Matze2
force-pushed
the
expose-demand-control-vars
branch
from
992dba1
to
d195795
Compare
|
Hi @Apollon77 , now that the heating season starts, I have better possibilities to test my changes. I worked on all your feedback, let me explain briefly what I've done.
Could be. But there is no documentation about that "type" field. At least I extended the code to expose also the "type" field. See commit 79afa3c. I can use it to disallow write changes on demand control if type != 1.
This one was trivial I just exchanged the order and sensor info is called first again. See commit 45f37d9.
For this I first added a new boolean where I remember if "get_demand_control" URL threw an error. In this case I will skip the call fur future update calls. As a result the first update would raise an error (which means no value is updated) but with the next update the demand control part is skipped and everything works like before. See commit 6fccacf. If we don't want this one-time-error, then we need to ignore errors for demand control completely. With this change the adapter should work as before even if the AC does not have demand control support and returns with PARAM_NG or similar errors. See commit d195795. I tested both choices by using an invalid URL "get_demand_control2". The drawback with the last commit is that even if something else goes wrong, there is no way to escalate that. Currently the callback can only support response or error. But it is safer to keep the old behavior and no sudden new error messages even if only once at adapter start. I would appreciate if you have time to review. Let me know if you need more explanations. Thanks in advance for your time. Just to note that I made also some corresponding commits in iobroker. daikin project. |
|
@Matze2 for the type thing: Did yiu tried what happens if you set type to 0 or such ? |
|
Okl this "read" is ok now ... I will merge ... we only need to see if tests run or needs to be enhanced too |
|
Thanks for merging. 👍 Yes this is why I split read and write in different PRs. After that I'll rebase the "write" PRs so they are easier to review. |
|
Lets do the lib first please ... then adapter is easier |
Closes #267
For now only the minimum needed for later control is exposed:
Not exposed:
Note: DemandControl class is currently trivial, but will be extended later for write support.