Remove need for separate EgardiaServer setup #11344
Conversation
| if self._egardiaserver is not None: | ||
| _LOGGER.debug("Starting EgardiaServer and registering callback") | ||
| # Register callback for alarm status changes through EgardiaServer | ||
| self._egardiaserver.register_callback(self.handle_system_status_event) |
There was a problem hiding this comment.
line too long (82 > 79 characters)
| "starting EgardiaServer") | ||
| except IOError as ioe: | ||
| return False | ||
|
|
| bound = hass.data[DATA_EGARDIASERVER].bind() | ||
| if not bound: | ||
| raise IOError("Binding error occurred while " + | ||
| "starting EgardiaServer") |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| _LOGGER.info("Setting up EgardiaServer") | ||
| try: | ||
| if DATA_EGARDIASERVER not in hass.data: | ||
| hass.data[DATA_EGARDIASERVER] = egardiaserver.EgardiaServer('', rs_port) |
There was a problem hiding this comment.
line too long (88 > 79 characters)
| except requests.exceptions.RequestException: | ||
| raise exc.PlatformNotReady() | ||
| except egardiadevice.UnauthorizedError: | ||
| _LOGGER.error("Unable to authorize. Wrong password or username") | ||
| return False | ||
|
|
||
|
|
||
| if rs_enabled: |
| except requests.exceptions.RequestException: | ||
| raise exc.PlatformNotReady() | ||
| except egardiadevice.UnauthorizedError: | ||
| _LOGGER.error("Unable to authorize. Wrong password or username") | ||
| return False | ||
|
|
||
|
|
| if self._egardiaserver is not None: | ||
| _LOGGER.debug("Starting EgardiaServer and registering callback") | ||
| # Register callback for alarm status changes through EgardiaServer | ||
| self._egardiaserver.register_callback(self.handle_status_event) |
There was a problem hiding this comment.
You don't need to do this inside the entity. You have access to the callback method of the entity as soon as it has been instantiated. Please move the registration and start of the server to outside the entity in setup platform. Then you don't need to pass in hass to the entity either.
| """Initialize object.""" | ||
| self._name = name | ||
| self._egardiasystem = egardiasystem | ||
| self._status = STATE_UNKNOWN | ||
| self._rs_enabled = rs_enabled |
| @@ -119,23 +140,18 @@ def state(self): | |||
| @property | |||
| def should_poll(self): | |||
| """Poll if no report server is enabled.""" | |||
| if not self._rs_enabled: | |||
| "starting EgardiaServer") | ||
| except IOError: | ||
| return False | ||
|
|
||
| add_devices([EgardiaAlarm( |
There was a problem hiding this comment.
Separate the entity instantiation from the entity addition.
|
See my comments in the old PR about home assistant event listener for stop. |
| hass.data[D_EGARDIASRV].stop() | ||
|
|
||
| # listen to home assistant stop event | ||
| hass.bus.listen('homeassistant_stop', handle_stop_event) |
There was a problem hiding this comment.
Use hass.bus.listen_once and import the home assistant stop event constant from const.py and use that here.
| _LOGGER.info("Setting up EgardiaServer") | ||
| try: | ||
| if D_EGARDIASRV not in hass.data: | ||
| hass.data[D_EGARDIASRV] = egardiaserver.EgardiaServer('', |
There was a problem hiding this comment.
Cache the server instance in a local variable so you don't need to look that up in hass.data every time.
hass.data[D_EGARDIASRV] = server = egardiaserver.EgardiaServer(...)| hass.data[D_EGARDIASRV].register_callback( | ||
| eg_dev.handle_status_event) | ||
| hass.data[D_EGARDIASRV].start() | ||
| server.register_callback(eg_dev.handle_status_event) |
There was a problem hiding this comment.
Registering a callback should be done for each setup_platform run, so move that out of the check for existing server in hass.data . It shouldn't matter much if you put it after server.start().
|
|
||
| try: | ||
| egardiasystem = egardiadevice.EgardiaDevice( | ||
| host, port, username, password, '') | ||
| host, port, username, password, '', version) | ||
| except requests.exceptions.RequestException: | ||
| raise exc.PlatformNotReady() | ||
| except egardiadevice.UnauthorizedError: | ||
| _LOGGER.error("Unable to authorize. Wrong password or username") | ||
| return False |
| hass.data[D_EGARDIASRV] = server | ||
| server.start() | ||
| except IOError: | ||
| return False |
|
Thanks for your help Martin! |
Description:
We removed the need for the user to run a separate Egardiaserver component. This is a breaking change, but makes the usage of the component much easier (and it can now also run on HASS.io).
This is a replacement for #11322
Related issue (if applicable): fixes #
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4282
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass