search

plugwise / python-plugwise

Plugwise python module for Smiles (Anna, Adam, P1) and Stretch
https://pypi.org/project/plugwise
MIT License
10 stars 9 forks source link

[BUG] Python error in getting thermostat presets #87

Closed vincentwolsink closed 3 years ago

vincentwolsink commented 3 years ago

A python error occurs in getting thermostat presets when using Plugwise beta component.

Edit: This seems to be only happening when the "Opentherm thermostat" (main thermostat) is not placed in a zone.

Logger: homeassistant.config_entries
Source: custom_components/plugwise/gateway.py:144
Integration: Plugwise Beta (documentation)
First occurred: 12:09:23 (1 occurrences)
Last logged: 12:09:23

Error setting up entry Adam for plugwise
Traceback (most recent call last):
  File "/home/homeassistant/lib/python3.8/site-packages/homeassistant/config_entries.py", line 293, in async_setup
    result = await component.async_setup_entry(hass, self)  # type: ignore
  File "/home/homeassistant/.homeassistant/custom_components/plugwise/__init__.py", line 21, in async_setup_entry
    return await async_setup_entry_gw(hass, entry)
  File "/home/homeassistant/.homeassistant/custom_components/plugwise/gateway.py", line 144, in async_setup_entry_gw
    api.get_all_devices()
  File "/home/homeassistant/lib/python3.8/site-packages/plugwise/smile.py", line 288, in get_all_devices
    self.all_device_data()
  File "/home/homeassistant/lib/python3.8/site-packages/plugwise/smile.py", line 248, in all_device_data
    data = self.get_device_data(dev_id)
  File "/home/homeassistant/lib/python3.8/site-packages/plugwise/smile.py", line 391, in get_device_data
    device_data = self.device_data_climate(details, device_data)
  File "/home/homeassistant/lib/python3.8/site-packages/plugwise/smile.py", line 343, in device_data_climate
    device_data["presets"] = self.presets(details["location"])
  File "/home/homeassistant/lib/python3.8/site-packages/plugwise/helper.py", line 535, in presets
    for rule_id in rule_ids:
TypeError: 'NoneType' object is not iterable

Home Assistant Core:

Plugwise product:

bouwew commented 3 years ago

@vincentwolsink a few questions:

bouwew commented 3 years ago

After a detailed look at the xml-data on my Adam, and the data that we have received from other users, I'd say this is not something we want to fix because then we are going against the way that Plugwise has set everything up in their xml-data. Who knows what other problems this will cause.

So, in my mind, the solution for solving the bug you are reporting, is assigning a location to (all of) the (present) thermostat(s) that don't have one. This is also how Plugwise wants you to set things up. Do you agree?

vincentwolsink commented 3 years ago

@bouwew thanks for your reply.

This is indeed plugwise-beta 0.15.0. But I think the bug is in python-plugwise itself. I can send you the output of the domain_objects if you want.

The reason I am not assigning the thermostat to any zone is because I want to remove the external physical thermostat and use my tablet with Home Assistant as thermostat in the specific zone. According to Plugwise support this should be possible. In the Plugwise app it works perfectly fine. (I do need support for using Jip as a temperature source for the climate device in HA though, which doesn't seem to be present at the moment, but that is something else).

I did fix it locally by just adding an if statement to check if rule_ids is populated before going into the for loop. But I don't know if this will have any unwanted side-effects.

bouwew commented 3 years ago

Ok clear I think. You want the python-plugwise code to handle the case no rules_ids found in a way so that no error is thrown, correct? This means you don't have presets defined for that "special" thermostats, correct?

About adding support for Jip, if you have one, should be a piece of cake. To be able to do this, and also to understand better how your system with the "special" thermostat looks like, please send me the XML-data from your system: please read https://github.com/plugwise/python-plugwise/tree/main/userdata#sharing and link the requested xml-data onto a post.

vincentwolsink commented 3 years ago

Ok clear I think. You want the python-plugwise code to handle the case no rules_ids found in a way so that no error is thrown, correct? This means you don't have presets defined for that "special" thermostats, correct?

Yes indeed. Thanks for your help. See the attached files. core.appliances.xml.txt core.direct_objects.xml.txt core.domain_objects.xml.txt core.locations.xml.txt core.modules.xml.txt

bouwew commented 3 years ago

Great, thanks! I'll have a look.

bouwew commented 3 years ago

Question: in Woonkamer I see a Tom and a Jip. But these are both "slaves" to the HA thermostat. How did you set this up in the Plugwise App? How is the HA thermostat called?

Also, I guess the Toon is the thermostat that is not assigned a location?

vincentwolsink commented 3 years ago

The external thermostat is called "Toon". It used to be in Woonkamer, currently I removed it from the zone, so Jip is the temperature sensor for Woonkamer. I'm still in contact with Plugwise support if this is the actual way to go. I would expect the "Toon" device to disappear when disconnecting the (wired) external thermostat, but this doesn't happen.

bouwew commented 3 years ago

Did you try to remove the Toon? When you click on the Toon there should be a button called Verwijderen. image

bouwew commented 3 years ago

And when I read it correctly on the Plugwise website, the Jip becomes the Master-thermostat so to speak. So you should be able to remove the Toon and have the Jip and the Tom in the Woonkamer, the Jip should be the master. In the Plugwise App, under Bediening you should see a thermostat with the name Woonkamer.

As I read it here https://www.plugwise.com/product/jip/ a Jip has the same status as a Lisa.

bouwew commented 3 years ago

@vincentwolsink Please test plugwise-beta version 0.16.0b0, it contains support for the Jip as a zone thermostat. Please let me know if it works. When it does, I'll do a formal release to v0.16.0.

vincentwolsink commented 3 years ago

I tried removing the "Toon" thermostat before, but it still got detected by HA, so I don't think it actually works. I just deleted it again, see attached domain_objects.

core.domain_objects.xml.txt

I will test 0.16.0b0 asap. Thanks!

vincentwolsink commented 3 years ago

I tried removing the "Toon" thermostat before, but it still got detected by HA, so I don't think it actually works. I just deleted it again, see attached domain_objects.

Just confirmed it. Even though the Toon thermostat is completely gone in the Plugwise app, it is still detected by Plugwise beta and causes the python error. So this might be a "bug" on the Plugwise side.

bouwew commented 3 years ago

Yes, I see what you mean. I'll think about what I can do to remove/ignore the Toon.

vincentwolsink commented 3 years ago

I just tested 0.16.0b0 and it works like a charm! The Jip device now has a climate entity. And even the error for Toon is gone! The device is still there in HA, but it doesn't throw a python error anymore. Perfect.

Very happy with this, thanks a lot. Where is your "buy me a coffee" button? 😄

bouwew commented 3 years ago

That's good news! Yes, I've also fixed the python error for good measure.

But the Toon still showing up, that bothers me somewhat. Something to fix in the future... Please leave this bug-report open, I'll keep you posted...

bouwew commented 3 years ago

I think I've solved it, I'll release a test-version soon.

bouwew commented 3 years ago

@vincentwolsink Please test plugwise-beta v0.16.0b1 The Toon should no longer show, or it should become unavailable (which means it's no longer there but HA Core kept the reference, you can manually remove the entity).

vincentwolsink commented 3 years ago

Tested v0.16.0b1, works. Toon no longer shows up.

bouwew commented 3 years ago

Great, thanks for checking! I'll start the release-process. Might take a while, vacation time...

bouwew commented 3 years ago

Bugs/missing functionality solved/implemented in #88

bouwew commented 3 years ago

@vincentwolsink You should also have a Jip humidity sensor. Is it present and does it have a correct value? In the 0-100% range?

vincentwolsink commented 3 years ago

Yes, looks good.

screenshot
bouwew commented 3 years ago

Thanks for confirming that works too. Please close this issue when you think the reported bugs are solved.

vincentwolsink commented 3 years ago

Issue solved. Thanks a lot for the help!