search

maximvelichko / pyvera

A python library to control devices via the Vera hub
GNU General Public License v2.0
26 stars 30 forks source link

Support for more than 1 controller #117

Closed vangorra closed 4 years ago

vangorra commented 4 years ago

Making code run in less of a singleton way by allowing a subscription to use an instance of a controller. Fixing tests as they were broken.

pavoni commented 4 years ago

This looks very sensible to me - any other comments before I merge?

GaryOkie commented 4 years ago

Will there be a corresponding update to the docs to clarify how subscribing to multiple controllers works?

And thanks @vangorra for working on this code. Sure good to see it get some attention!

BTW - has anyone been following discussions at the Vera developers forum about a complete rewrite of the Vera controller Linux firmware? It's long overdue, but having backward compatibility with plugins and API is not planned for the first release, if ever. Feedback there is welcome, especially as it relates to HA integration and the API.

vangorra commented 4 years ago

While I'd prefer to eliminate the singleton altogether, I realize that will break other dependent projects, alas it must stay. Hence my compromise PR.

I don't think the docs need an update. This change adds expected behavior to the class structure. It's the get_cotroller() call in SubscriptionRegistry that's unusual and seeds confusion when trying to use a different controller.

pavoni commented 4 years ago

Now released as 0.3.5. Thanks for the contribution.

I've just rebuilt my dev machine - and can't currently test integration with Home Assistant - so can someone do this before putting together a PR for HA?

vangorra commented 4 years ago

I've confirmed 0.3.5 is working with home assistant.

pavoni commented 4 years ago

Fantastic - thanks very much @vangorra Are you happy to do the HA PR or would you like me to?

vangorra commented 4 years ago

I'll do the PR. I want to remove HA's use of get_controller anyway. That will get us closer to supporting more than one controller in HA.

pavoni commented 4 years ago

Perfect - thanks!