laetificat / haanna

Plugwise Anna API to use in conjunction with Home Assistant.
MIT License
5 stars 5 forks source link

Fix multiple thermostats #28

Closed fwestenberg closed 4 years ago

fwestenberg commented 5 years ago

When multiple thermostats are configured, the static variables holds the data of only one thermostat (endpoint, user and password). So both thermostats will eventually ping the same endpoint and consist of the same data.

fwestenberg commented 4 years ago

Hi, can you please accept this PR? It's running stable here since I created this one. Thanks!

fwestenberg commented 4 years ago

@laetificat Please check out this PR. Thanks.

CoMPaTech commented 4 years ago

Sorry, didn't time to look at this yet. @bouwew does this look good to you as well? Just so we're not interfering with the overhaul you were planning?

bouwew commented 4 years ago

Yes, it looks good. I'm running the updated code in my system since yesterday, the latest fix since this morning. Need still to perform some tests. But I understand why this change is needed. So OK from me. Also, no conflict with my planned changes, github should be able to manage any conflicts.

bouwew commented 4 years ago

Update: this update is not fully working. HA shows an error when I have the weekly schematic disabled and try to enable it again (select Auto).

fwestenberg commented 4 years ago

I changed the method get_rule_id_by_name from static. Now it's working. Only thing left is the existing bug when no schema is present.

bouwew commented 4 years ago

I tried various things including rolling back to the reference, my anna-ha github. But now I still keep getting local variable 'template_id' referenced before assignment. It was either this message or ...get_rule_id_by_name, takes 2 positional arguments but 3 were given, or maybe ...takes 3 positional arguments but 2 were given. I tried various things but always one of these 2 errors would show.

It tried what you are proposing but then the first error pops up. To get this error, first disable the schema then try to enable it by selecting Operation --> Auto. Also, my Anna is a recent version, 3.1.10.

bouwew commented 4 years ago

I think my haanna version is not updating as I expect it to be, via manifest.json.

bouwew commented 4 years ago

@fwestenberg I have included several of your suggested changes into my haanna-version 0.14.2. I also noticed that you removed most of the @staticmethod headers, is there an advantage in doing that?

Also, I would like to suggest to close this pull-request as I've included the important changes in my haanna-update.