Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle preset finding in XML better #14

Closed
CoMPaTech opened this issue Sep 9, 2019 · 77 comments
Closed

Handle preset finding in XML better #14

CoMPaTech opened this issue Sep 9, 2019 · 77 comments

Comments

@CoMPaTech
Copy link
Collaborator

Modify (and test!) if searching for "a rule that has a template_id tag containing zone_setpoint_and_state_based_on_preset and has active set to true" instead of what is currently implemented in haanna.py (lines 62/63):

            rule_id = self.get_rule_id_by_name(root,
                                               'Thermostat presets')

As reported in home-assistant/core#26520

CoMPaTech added a commit that referenced this issue Sep 9, 2019
@bouwew
Copy link
Contributor

bouwew commented Sep 10, 2019

@CoMPaTech I think we should investigate a bit more before we push this update.
I'm thinking the reported problem is not normal, maybe it is due to a misconfiguration, maybe it can be fixed by a factory reset of the Anna in question?

@CoMPaTech
Copy link
Collaborator Author

Not sure, but finding it by tag makes more sence to me (in hindsight) as 'what if' they decide to rename it. A name as such is somethings thats changeable/language, that's why I think it makes sense to use the tagfinder instead of name. IIRC thats also the original route @laetificat took.

@bouwew
Copy link
Contributor

bouwew commented Sep 12, 2019

I looked at your code-change and noticed the fall-back :)
I still think the Anna in question should be rebooted, or something similar, to see whether that remedies the issue.
What is the firmware version on your Anna? Mine is 3.1.6. Is yours really 3.1.8?

@bouwew
Copy link
Contributor

bouwew commented Sep 30, 2019

Let's close this issue, it's fixed I think?

@bouwew
Copy link
Contributor

bouwew commented Oct 14, 2019

I was thinking when looking at this issue again, the problem is, when there is no name for the schedule(s), as in w2ckers-case, other stuff will not work. We might have fixed this particular error but there will be other errors.

@bouwew
Copy link
Contributor

bouwew commented Oct 22, 2019

@CoMPaTech
I think this Issue can be closed. I mean, w2cker tested my new code. The weekschedule that he has created is properly detected. And there were no errors in his log. I'd say your fixes are working!

@fwestenberg
Copy link

Totday I updated HA to version 0.102.0. I get the following error now in the log: "File "/usr/local/lib/python3.7/site-packages/haanna/haanna.py", line 197, in get_rule_id_by_template_tag
rule.find("template").attrib["tag"]
KeyError: 'tag'". Anna integration not working anymore. :-(

@bouwew
Copy link
Contributor

bouwew commented Nov 21, 2019

Please let us know the firmware-version of your Anna, and please post the contents of
http://[you smile ip]/core/domain_objects on pastebin.com.
This will help us to find what is causing the issue.

@fwestenberg
Copy link

fwestenberg commented Nov 21, 2019

Domain Objects here: https://pastebin.com/pduQDyz6

Firmware version is 1.8.20.

@CoMPaTech
Copy link
Collaborator Author

I think you have to add the legacy statements then ... see https://www.home-assistant.io/blog/2019/11/20/release-102/#read-more on breaking changes. See https://www.home-assistant.io/integrations/plugwise/ on setting legacy_anna

@fwestenberg
Copy link

After changing this, I got a new error:

File "/usr/local/lib/python3.7/site-packages/haanna/haanna.py", line 373, in get_schedule_temperature
return float(measurement)
TypeError: float() argument must be a string or a number, not 'NoneType'

Any change I can update to a higher version?

@bouwew
Copy link
Contributor

bouwew commented Nov 22, 2019

I looked at the various codes, I can't find anything obviously wrong at the moment.
But, I added detection for the reported fault. This specific fault should be gone.
Can you please test the updated haanna-code for me? Let's see where that leads us.

Replace in the file /homeassistant/components/plugwise/manifest.json on your system:
"requirements": ["haanna==0.13.5"]
by
"requirements": ["https://github.com/bouwew/haanna/archive/master.zip#haanna==0.13.6"]
and let me know the result.

@fwestenberg
Copy link

fwestenberg commented Nov 22, 2019

I created it as custom component (I'm on Hass.io) but with this change it's working again. But... setpoint and presets have disappeared.

@bouwew
Copy link
Contributor

bouwew commented Nov 22, 2019

Can you reproduce the fault in the custom component with haanna 0.13.5?

I'm at a bit of a loss, it's working fine in my system (3.1.7) and it's working on someone else's legacy Anna. I've compared your XML-data to this persons XML-data, basically they are the same. So I don't understand why it's not working for you. The value is present in your XML but somehow the Haanna-code doesn't find it, very strange.

@fwestenberg
Copy link

What do you mean with "reproduce the fault in the custom component with haanna 0.13.5"?
And to make it even better: I have two Anna's (also one upstairs). Both ain't working well.

@bouwew
Copy link
Contributor

bouwew commented Nov 22, 2019

You have created a Plugwise custom component, correct? And you've made the change in the file manifest.json as I asked? Now I'm asking you to revert the change you made to this file and try again.
Does this result in the same error?

You write you have two Anna's, do you have these both "connected" to Home Assistant? How? Via 2 separate climate instances in HA? You must have 2 Smiles then?

@bouwew
Copy link
Contributor

bouwew commented Nov 22, 2019

Can you show me how your Anna climate device(s) looks like in Developer Tools --> States?

@fwestenberg
Copy link

I have this:
image

@bouwew
Copy link
Contributor

bouwew commented Nov 22, 2019

I think I have found what is causing the error. Not sure how to solve it yet.
And I see you have more issues: state and hvac_modes also don't work.

I will not be available for the coming 2-3 weeks, I'm moving to a different city, 2 hrs drive away.
I will let @CoMPaTechknow what I have found, maybe he has time to fix the reported error.

@bouwew
Copy link
Contributor

bouwew commented Nov 23, 2019

I could not let this go yet :)

I have made a haanna-simulator in which I imported the XML-data that you put on Pastebin.
In the simulator, the schedule_temperature is found correctly, there is no error.

So, I think there is something wrong with your HA-system, maybe the haanna-version on your system is outdated, or something went wrong with the 0.102 update.

After updating your HA-system, you've made Plugwise into a custom component, you wrote. How have you done that? Which files were put where? And please show the contents of the file manifest.json of your custom component.

@bouwew
Copy link
Contributor

bouwew commented Nov 24, 2019

@fwestenberg, another user reported the same problem as you have found.
According to his information the fault happens when there is a week-schedule present but it's not activated. The fault disappears when the week-schedule is activated.
Is your situation the same? Do you have a week-schedule present but it's not activated?

@fwestenberg
Copy link

My week-schedule is empty. I deleted it because I don't need it. I will create one and see if this solves the problem.

@bouwew
Copy link
Contributor

bouwew commented Nov 24, 2019

Can you first create another XML-dump? That might help us to find a solution.

@fwestenberg
Copy link

IMG_20191124_085011
IMG_20191124_084953
IMG_20191124_084934

@bouwew
Copy link
Contributor

bouwew commented Nov 24, 2019

It looks like it's all working as it should?

@fwestenberg
Copy link

But where is the setpoint adjustment?

@fwestenberg
Copy link

IMG_20191124_090816

But seems you are right. This card works well, also setpoint adjustment.

@bouwew
Copy link
Contributor

bouwew commented Jan 9, 2020

You can create your own pull-request towards the translation :)

Or use the Simple Thermostat card, you can configure the names of the presets to anything you like.

@bouwew
Copy link
Contributor

bouwew commented Jan 9, 2020

Can you please capture one last XML?

http://[your smile ip]/core/direct_objects

Another user has reported a similar issue and he found there's another XML-path where the actual indicator I'm looking for, is hidden. Hopefully the XML-data from your system can confirm this.

@fwestenberg
Copy link

Here it is: https://pastebin.com/6DaEQz9N

@bouwew
Copy link
Contributor

bouwew commented Jan 9, 2020

Thanks! Was this capture taken with the heating in on/active state?

@fwestenberg
Copy link

The setpoint was 18 degrees while the actual temperature was about 19. So I say inactive. Even when I raise the setpoint:

IMG_20200109_163017
IMG_20200109_163036

@fwestenberg
Copy link

Here's a paste with thermostat active:

https://pastebin.com/dG1pVLWL

@bouwew
Copy link
Contributor

bouwew commented Jan 9, 2020

Thanks. But no avail, the boiler_state still shows off. Strange, scratching my head...

@fwestenberg
Copy link

Stop scratching. It's in the direct_objects:

https://pastebin.com/6xzpXj79

@bouwew
Copy link
Contributor

bouwew commented Jan 9, 2020

Yes, that is what I was expecting to see. Thanks!
On the other hand, a bit strange too. I was expecting the key central_heating_state to be there too.
The Pluswise Smile software works in mysterious ways...
Anyway with this key I should be able to make it work.

@bouwew
Copy link
Contributor

bouwew commented Jan 11, 2020

@fwestenberg
Please try my latest code at https://github.com/bouwew/anna-ha
Be sure to include the manifest.json file as this points to my updated haanna-version 0.14.0.
This version contains most of your fixes as well.

Also, I had to reintroduce some old haanna-code that I had removed before. Without this code it is not possible to reactivate a previously used weekschedule on the latest Anna. This might reintroduce a bug for the legacy Anna. Let me know if there is an error in the log related to the "lambda" function.

@fwestenberg
Copy link

Thanks for the update, I'm testing it now. I removed the @staticmethod headers because I did not see any advantage in the statics here. It's probably a leftover of the code before, when the whole Anna component was sort of static and incompatible with multiple thermostats. I think it's not wrong to remove the statics, but also not wrong to keep it.
For as far as I can see the component is working as it should. I tested both thermostats:

  • activating presets
  • manual temperature setting
  • heating active/inactive
  • schedule
  • actual temperature

@bouwew bouwew mentioned this issue Jan 16, 2020
@bouwew
Copy link
Contributor

bouwew commented Jan 16, 2020

Thanks for reporting back!
During the coming days I will push all the recent improvements into the HA next branch.

@fwestenberg
Copy link

@bouwew shouldn't this have been fixed? Seems like I was still on the plugwise_dev integration, but after changing to the original plugwise integration this shows up:

plugwise: Error on device update!
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 304, in _async_add_entity
await entity.async_device_update(warning=False)
File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 461, in async_device_update
await self.hass.async_add_executor_job(self.update)
File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
result = self.fn(*self.args, **self.kwargs)
File "/usr/src/homeassistant/homeassistant/components/plugwise/climate.py", line 276, in update
self._domain_objects
File "/usr/local/lib/python3.7/site-packages/haanna/haanna.py", line 164, in get_last_active_schema_name
rule_id = self.get_rule_id_by_template_tag(root, locator)
File "/usr/local/lib/python3.7/site-packages/haanna/haanna.py", line 187, in get_rule_id_by_template_tag
if rule.find("template").attrib["tag"] == rule_name:
KeyError: 'tag'

@bouwew
Copy link
Contributor

bouwew commented Feb 12, 2020

@fwestenberg
Can you please change in the plugwise manifest.json file the line:"requirements": [...], the contents between [ and ] to:

"https://github.com/bouwew/haanna-1/archive/master.zip#haanna==0.14.2"

Then save the file and restart HA. Let me know the result.

Update: this fixed a similar issue for two other legacy Anna users.

@WilbertVerhoeff
Copy link

@bouwew
I don't know if i'm allowed to command on this. So please kindly let me know.

I'm a legacy Anna user also. The change in the manifest.json works for me. However the illuminance is not in the attributes anymore. I don't know if the boiler state should be in it or the hvac -action should be update when de boiler is heating or not.

please let me know if i can help.

@bouwew
Copy link
Contributor

bouwew commented Feb 15, 2020

@WilbertVerhoeff

Sure, feedback is always welcome :)
Illuminance is not there because HA official did not allow it as an attribute. I have to add it as a sensor, which will probably be done sometime in the not too near future.
I added boiler_state because for some installations the hvac-action was always idle, heating was not shown. Adding this made these systems show the heating status correctly.

@CoMPaTech
Copy link
Collaborator Author

I'm sort of tinkering with sensors (again), but it takes some time to get it right :(

@bouwew
Copy link
Contributor

bouwew commented Feb 15, 2020

@WilbertVerhoeff
If you really want the illumination-value, you would need to install the Plugwise component as a custom_component, and I could tell you which lines to add to the file climate.py to get the illumination-value back.

@WilbertVerhoeff
Copy link

@bouwew
I have the custom component running with the haanna 14.2 (library?). If you want to, that would be really nice!

I want to thank you anyway for all the work you put in this integration. The Plugwise integration is THE reason i started with HomeAssistant.

@bouwew
Copy link
Contributor

bouwew commented Feb 16, 2020

@WilbertVerhoeff
Ok, no problem. The basis of the custom_component must be from the laetificat/anna-ha repository.
Then, you need to add the following lines in climate.py:

Under the def __init__(self, api, name, min_temp, max_temp): part this line:

self._illuminance = None

Under def device_state_attributes(self): these 2 lines:

         if self._illuminance:
            attributes["illuminance"] = self._illuminance

And finally under def update(self): add this line:

self._illuminance = self._api.get_illuminance(self._domain_objects)

The placement of these lines doesn't really matter, as long as they are placed under the correct header.

For editing the file, I would suggest the Visual Studio Code add-on.

@bouwew
Copy link
Contributor

bouwew commented Mar 16, 2020

@WilbertVerhoeff zou jij ons kunnen helpen?
We hebben xml-data van de legacy anna nodig om te kunnen komen tot een Home Assistant component die werkt voor alle Plugwise apparaten (Anna, Adam, Smile P1, etc.)
We zijn al een behoorlijk eind op weg maar deze data missen we nog.

@WilbertVerhoeff
Copy link

Yes, I want to help! How do you want the data? Can I copy paste it some where? Or can I open a port so you can watch when you want?

@bouwew
Copy link
Contributor

bouwew commented Mar 16, 2020

Great! Please go here: https://github.com/plugwise/Plugwise-Smile/tree/master/tests
And scroll a bit down for the exact URLs.
You will get the best result by browsing to the page, then right-click on the text and choosing Save as...
Then save them all as xml-file.

Also, please browse to http://{ip_of_your_smile}//system/status/xml and save the data there as well if there is any data.

Please email all the files to bouwe "@" westerdijk "." info (spaties en " weghalen).

Alvast bedankt!

@bouwew
Copy link
Contributor

bouwew commented Mar 30, 2020

@fwestenberg We are working on a common platform for all Plugwise devices. There will be support for the Plugwise Anna's, Smile P1's and the Adam, and Plugs via the Adam. We have tested with the Adam, Plugs and the Anna with firmware 3.1.11. Now we are looking for testers that have a legacy Anna. Are you willing to help?

@fwestenberg
Copy link

Sure! Just let me know what to do!

@bouwew
Copy link
Contributor

bouwew commented Mar 30, 2020

Very good, thanks!.
First, disable the present component, best to do this by removing the plugwise-lines in configuration.yaml and restarting HA.
Next, go here: https://github.com/plugwise/plugwise-beta/ and read the information shown in README.md.
Any questions regarding this, let me know here.

After having installed the new component and having configured it, please check the result: are all the various entities there? If anything went wrong, check the Log and send us the error-info. Please provide as detailed feedback as possible.

@bouwew
Copy link
Contributor

bouwew commented Mar 30, 2020

@CoMPaTech
Copy link
Collaborator Author

@fwestenberg @WilbertVerhoeff we've just released 0.2.0 of our custom_component (with an upstream HA-PR in progress). As per the release of HA-core 0.109 this wednesday (or if you already run HA-core beta 109) you can start using plugwise-beta.

Closing this issue. If you have any further comments or issues feel free to update

The (community post)[https://community.home-assistant.io/t/plugwise-smile-custom-component-beta/183560/3?u=compatech]
The (module)[https://github.com/plugwise/Plugwise-Smile] replacing this repository
The (custom_component)[https://github.com/plugwise/plugwise-beta]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants