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

Remote Home-Assistant: Linking multiple instances via Websocket API #13876

Conversation

lukas-hetzenecker
Copy link
Contributor

@lukas-hetzenecker lukas-hetzenecker commented Apr 14, 2018

Description:

This platform links multiple home-assistant instances together, replacing the deprecated "Multiple Instances".
The master instance connects to the Websocket APIs of the slaves, the connection options are specified via the host, port, and secure configuration parameters. An API password can also be set via api_password.

After the connection is completed, the remote states get populated into the master instance.
The entity ids can optionally be prefixed via the entity_prefix parameter.

The component keeps track which objects originate from which instance. Whenever a service is called on an object, the call gets forwarded to the particular slave instance.

When the connection to the remote instance is lost, all previously published states are removed again from the local state registry.

A possible use case for this is to be able to use different z-wave networks, on different z-wave sticks (with the second one possible running on another computer in a different location). This is a highly requested feature, as can be seen in the following forum postings:

Currently this use case could be solved by using mqtt_eventstream, but this comes with its own set of problems, e.g.: https://community.home-assistant.io/t/master-slave-with-mqtt-eventstream-retain-all-published-devices/17012

Another option would be to use, mqtt_statestream, but this makes dealing with service calls a real hassle (see https://community.home-assistant.io/t/best-way-to-connect-zwave-switches-on-second-ha-instance-to-main-ha-instance/40577 )

My alternative proposes a way, which

  • does not need any extra configuration stating which remote objects are imported
  • makes all remote objects available locally
  • retains all attributes of the remote objects
  • removes remote objects again after a connection loss to the remote instance
  • allows interacting with remote objects via service calls
  • allows customization of those remote objects in the master config in the customize block

.. and therefore makes linking multiple instances really simple.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5170

Example entry for configuration.yaml (if applicable):

remote_homeassistant:
  slaves:
  - host: localhost
    port: 8124
  - host: localhost
    port: 8125
    api_password: test
    entity_prefix: "slave02_"

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

According to the changelog, they should be compatible
( https://github.com/aaugustin/websockets/blob/master/docs/changelog.rst )
Compression is now enabled by default
@cmsimike
Copy link
Contributor

This is huge for me! Thank you!!

from homeassistant.config import DATA_CUSTOMIZE
import homeassistant.helpers.config_validation as cv

REQUIREMENTS = ['websockets==4.0.1']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use aiohttp to make the websocket connection as it's already part of the core.

"""Publish remove event on local instance."""
if message['type'] == 'result':
pass
elif message['type'] == 'event':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use guard clauses to make the code more readable.

elif message['type'] != 'event':
    return

if message

SLAVES_SCHEMA = vol.Schema({
vol.Required(CONF_HOST): cv.string,
vol.Optional(CONF_PORT, default=8123): cv.port,
vol.Optional(CONF_SECURE, default=False): cv.boolean,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make more sense to have the user just define a url like https://blabla.duckdns.org:1220

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wss://blabla.duckdns.org:1220 .. hopefully users won't confuse ws/wss with http/https?

@@ -208,6 +208,9 @@ omit =
homeassistant/components/raspihats.py
homeassistant/components/*/raspihats.py

homeassistant/components/remote_homeassistant.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope.

This needs like a 1000 tests added.


_LOGGER = logging.getLogger(__name__)

CONF_SLAVES = 'slaves'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides slaves being a politically loaded word, it's also the wrong description. We are not delegating work to them, we're integrating them into our instance. I think that we should just call the entry instances

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally agree, fixed

except websockets.exceptions.ConnectionClosed:
_LOGGER.error('remote websocket connection closed')
await self._disconnected()
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary return

@cmsimike
Copy link
Contributor

cmsimike commented May 2, 2018

Really hype to get this into hass!

@balloob
Copy link
Member

balloob commented May 2, 2018

@cmsimike maybe you can help him with writing some tests?

@lukas-hetzenecker
Copy link
Contributor Author

Wouldn't mind that at all :) - but please tell me before you do, would otherwise start soonish with them

@cmsimike
Copy link
Contributor

cmsimike commented May 2, 2018

@balloob thanks for letting me know that's all that's required! i have added it to my todo list. not sure when i can get to it.

@lukas-hetzenecker definitely, but as above, might be a bit before i can get to it.

@lukas-hetzenecker
Copy link
Contributor Author

FYI, just started with the test cases.. (preparation for my PyDays talk can wait.. ;) )

…onents"

This reverts commit 6c8e23d1b2001e027fd03593d5010f8d7b479610.

#self.hass_main.block_till_done()
pass

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line at end of file

#setup_component(self.hass_main, remote_homeassistant.DOMAIN, {
# remote_homeassistant.DOMAIN: config})

#self.hass_main.block_till_done()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

# }
# ]
#}
#setup_component(self.hass_main, remote_homeassistant.DOMAIN, {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

# #remote_homeassistant.CONF_PORT: instance01_port
# }
# ]
#}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

#self.hass_instance01.block_till_done()


#config = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many blank lines (2)
block comment should start with '# '

# }
#)

#setup_component(self.hass_instance01, 'websocket_api')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

# http.CONF_SERVER_PORT: instance01_port,
# }
# }
#)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '


#self.hass_main.block_till_done()

#setup_component(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '


self.hass_main = get_test_home_assistant()

#self.hass_main.block_till_done()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

def setup_method(self):
"""Setup things to be run when tests are started."""
self.hass_instance01 = get_test_home_assistant()
#self.hass_instance02 = get_test_home_assistant()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

block comment should start with '# '

@MartinHjelmare MartinHjelmare changed the title Remote Home-Assistance: Linking multiple instances via Websocket API Remote Home-Assistant: Linking multiple instances via Websocket API May 19, 2018

CONFIG_SCHEMA = vol.Schema({
DOMAIN: vol.Schema({
vol.Required(CONF_INSTANCES): [INSTANCES_SCHEMA],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vol.Required(CONF_INSTANCES): vol.All(cv.ensure_list, [INSTANCES_SCHEMA]),

"""Set up the remote_homeassistant component."""
conf = config.get(DOMAIN)

for instance in conf.get(CONF_INSTANCES):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use dict.get(key) for required config items, use dict[key].

vol.Optional(CONF_SECURE, default=False): cv.boolean,
vol.Optional(CONF_API_PASSWORD): cv.string,
vol.Optional(CONF_SUBSCRIBE_EVENTS,
default=DEFAULT_SUBSCRIBED_EVENTS): cv.ensure_list,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vol.All(cv.ensure_list, [cv.string])

def __init__(self, hass, conf):
"""Initialize the connection."""
self._hass = hass
self._host = conf.get(CONF_HOST)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self._host = conf[CONF_HOST]


async def async_connect(self):
"""Connect to remote home-assistant websocket..."""
url = '%s://%s:%s/api/websocket' % (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use new style string formatting: https://pyformat.info/.

Maybe break out url string into constant?

if self._entity_prefix:
domain, object_id = split_entity_id(entity_id)
object_id = self._entity_prefix + object_id
entity_id = domain + '.' + object_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use new style string formatting.

asyncio.ensure_future(self._recv())

def _next_id(self):
_id = self.__id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docstring.

return _id

async def _call(self, callback, message_type, **extra_args):
_id = self._next_id()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add docstring.

def _remove_prefix(entity_id):
domain, object_id = split_entity_id(entity_id)
object_id = object_id.replace(self._entity_prefix, '', 1)
return domain + '.' + object_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use new style string formatting.

instance01_port = 8124
instance02_port = 8125

class TestRemoteHomeassistant(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you go with pytest tests that are not class based. That will be cleaner and you probably want to use the async home assistant api, so then write tests that are coroutines.

from homeassistant.helpers.typing import HomeAssistantType, ConfigType
from homeassistant.const import (CONF_HOST, CONF_PORT, EVENT_CALL_SERVICE,
EVENT_HOMEASSISTANT_STOP,
EVENT_STATE_CHANGED, EVENT_SERVICE_REGISTERED, CONF_URL)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (89 > 79 characters)

import homeassistant.components.websocket_api as api
from homeassistant.core import EventOrigin, split_entity_id
from homeassistant.helpers.typing import HomeAssistantType, ConfigType
from homeassistant.const import (CONF_HOST, CONF_PORT, EVENT_CALL_SERVICE,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'homeassistant.const.CONF_URL' imported but unused

from homeassistant.helpers.typing import HomeAssistantType, ConfigType
from homeassistant.const import (CONF_HOST, CONF_PORT, EVENT_CALL_SERVICE,
EVENT_HOMEASSISTANT_STOP,
EVENT_STATE_CHANGED, EVENT_SERVICE_REGISTERED, CONF_URL)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (89 > 79 characters)

import homeassistant.components.websocket_api as api
from homeassistant.core import EventOrigin, split_entity_id
from homeassistant.helpers.typing import HomeAssistantType, ConfigType
from homeassistant.const import (CONF_HOST, CONF_PORT, EVENT_CALL_SERVICE,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'homeassistant.const.CONF_URL' imported but unused

@balloob
Copy link
Member

balloob commented May 19, 2018

So I've just added a stub test to get 2 hass instances work together to help you move along. Because you're going around our async infrastructure, I could not get it working without an asyncio.sleep. You will have to fix this before this can get merged.

However, and this brings me to the next part: the code is messy, too many features for MVP, too configurable and the data flows in the wrong direction.

Functionality like this should preferably proposed in our architecture repo to allow people to discuss such major features.

Here is an MVP that I would accept. Client is the one that has "remote_homeassistant" configured. Master is another HASS instance with the websocket API running.

  • Name should be updated. Maybe "replicate" ? Remote is confusing with the remote component.
  • Clients push their states to the "master" (current code pulls from master to client)
  • Clients can only publish to 1 master
  • For MVP, clients will only publish their states to the master. Services and other events are out of scope for MVP.
  • When a client disconnected, all states are removed from the master.

The reason for keeping the config decentralized, not centralized is that it will only require the master instance to have a fixed IP. All other clients can just spin up, publish states while they are online and drop off, and it will be cleaned up.

The other nice thing about this approach is that it can be broken up in multiple PRs:

  1. Add a new "set state" command to the websocket API that will set the state but also remove them when the connection drops.
  2. Add a new component that makes a connection and pushes all states at time of setup and then listens for state changes and pushes them out.

I am going to close this PR now as this functionality should be added in 2 PRs. If you don't agree with my proposal, feel free to open an issue in the architecture repo so we can discuss this.

@balloob balloob closed this May 19, 2018
@godinperson
Copy link

No, it was so close...

@lukas-hetzenecker
Copy link
Contributor Author

@balloob made some good points though ;)
But, for the time being, you can use it by putting it into your custom_components

@trebolcinco
Copy link

@lukas-hetzenecker was super simple to setup as a custom component, thanks for doing this. Have a large house spread across a couple buildings and this solves my zwave distance issues.

@steinis123
Copy link

Will the be any work done to this great plugin? I had some issues when it comes to stability.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Aug 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants