-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
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
Remote Home-Assistant: Linking multiple instances via Websocket API #13876
Conversation
which connects to a remote Home-Assistant instance via the Websocket API
According to the changelog, they should be compatible ( https://github.com/aaugustin/websockets/blob/master/docs/changelog.rst ) Compression is now enabled by default
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'] |
There was a problem hiding this comment.
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': |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary return
Really hype to get this into hass! |
@cmsimike maybe you can help him with writing some tests? |
Wouldn't mind that at all :) - but please tell me before you do, would otherwise start soonish with them |
@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. |
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 | ||
|
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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, { |
There was a problem hiding this comment.
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 | ||
# } | ||
# ] | ||
#} |
There was a problem hiding this comment.
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 = { |
There was a problem hiding this comment.
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') |
There was a problem hiding this comment.
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, | ||
# } | ||
# } | ||
#) |
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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 '# '
|
||
CONFIG_SCHEMA = vol.Schema({ | ||
DOMAIN: vol.Schema({ | ||
vol.Required(CONF_INSTANCES): [INSTANCES_SCHEMA], |
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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' % ( |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
f068a5a
to
0ca296e
Compare
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 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.
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:
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. |
No, it was so close... |
@balloob made some good points though ;) |
@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. |
Will the be any work done to this great plugin? I had some issues when it comes to stability. |
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
, andsecure
configuration parameters. An API password can also be set viaapi_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
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):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices: