Skip to content

Commit

Permalink
fix(collector): return previous states when the device is disconnected (
Browse files Browse the repository at this point in the history
  • Loading branch information
palazzem authored Mar 1, 2024
1 parent eaa145f commit 72ed533
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 6 deletions.
12 changes: 10 additions & 2 deletions custom_components/econnect_metronet/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import Any, Dict, Optional

import async_timeout
from elmo.api.exceptions import InvalidToken
from elmo.api.exceptions import DeviceDisconnectedError, InvalidToken
from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
from homeassistant.core import HomeAssistant
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator
Expand Down Expand Up @@ -53,7 +53,7 @@ async def _async_update_data(self) -> Optional[Dict[str, Any]]:
return await self.hass.async_add_executor_job(self._device.update)

async with async_timeout.timeout(POLLING_TIMEOUT):
if not self.last_update_success:
if not self.last_update_success or not self._device.connected:
# Force an update if at least one failed. This is required to prevent
# a misalignment between the `AlarmDevice` and backend IDs, needed to implement
# the long-polling strategy. If IDs are misaligned, then no updates happen and
Expand Down Expand Up @@ -82,3 +82,11 @@ async def _async_update_data(self) -> Optional[Dict[str, Any]]:
await self.hass.async_add_executor_job(self._device.connect, username, password)
_LOGGER.debug("Coordinator | Authentication completed with success")
return await self.hass.async_add_executor_job(self._device.update)
except DeviceDisconnectedError as err:
# If the device is disconnected, we keep the previous state and try again later
# This is required as the device might be temporarily disconnected, and we don't want
# to make all entities unavailable for a temporary issue. Furthermore, if the device goes
# in an unavailable state, it might trigger unwanted automations.
# See: https://github.com/palazzem/ha-econnect-alarm/issues/148
_LOGGER.error(f"Coordinator | {err}. Keeping the last known state.")
return {}
14 changes: 13 additions & 1 deletion custom_components/econnect_metronet/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
CodeError,
CommandError,
CredentialError,
DeviceDisconnectedError,
LockError,
ParseError,
)
Expand Down Expand Up @@ -49,6 +50,7 @@ class AlarmDevice:

def __init__(self, connection, config=None):
# Configuration and internals
self.connected = False
self._inventory = {}
self._sectors = {}
self._connection = connection
Expand Down Expand Up @@ -170,6 +172,7 @@ def connect(self, username, password):
"""
try:
self._connection.auth(username, password)
self.connected = True
except HTTPError as err:
_LOGGER.error(f"Device | Error while authenticating with e-Connect: {err}")
raise err
Expand All @@ -194,13 +197,18 @@ def has_updates(self):
"""
try:
self._connection.query(q.ALERTS)
return self._connection.poll({key: value for key, value in self._last_ids.items()})
data = self._connection.poll({key: value for key, value in self._last_ids.items()})
self.connected = True
return data
except HTTPError as err:
_LOGGER.error(f"Device | Error while polling for updates: {err.response.text}")
raise err
except ParseError as err:
_LOGGER.error(f"Device | Error parsing the poll response: {err}")
raise err
except DeviceDisconnectedError as err:
self.connected = False
raise err

def get_state(self):
"""Determine the alarm state based on the armed sectors.
Expand Down Expand Up @@ -278,8 +286,12 @@ def update(self):
except ParseError as err:
_LOGGER.error(f"Device | Error during the update: {err}")
raise err
except DeviceDisconnectedError as err:
self.connected = False
raise err

# Update the _inventory
self.connected = True
self._inventory.update({q.SECTORS: sectors["sectors"]})
self._inventory.update({q.INPUTS: inputs["inputs"]})
self._inventory.update({q.OUTPUTS: outputs["outputs"]})
Expand Down
2 changes: 1 addition & 1 deletion custom_components/econnect_metronet/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"elmo"
],
"requirements": [
"econnect-python==0.11.0"
"econnect-python==0.12.0a1"
],
"version": "2.3.0"
}
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ classifiers = [
"Programming Language :: Python :: Implementation :: CPython",
]
dependencies = [
"econnect-python==0.11.0",
"econnect-python==0.12.0a1",
"async_timeout",
"homeassistant",
]
Expand Down
62 changes: 61 additions & 1 deletion tests/test_coordinator.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from datetime import timedelta

import pytest
from elmo.api.exceptions import CredentialError, InvalidToken
from elmo.api.exceptions import CredentialError, DeviceDisconnectedError, InvalidToken
from homeassistant.exceptions import ConfigEntryNotReady
from requests.exceptions import HTTPError

Expand Down Expand Up @@ -216,3 +216,63 @@ async def test_coordinator_first_refresh_update_failed(mocker, coordinator):
# Test
with pytest.raises(ConfigEntryNotReady):
await coordinator.async_config_entry_first_refresh()


@pytest.mark.asyncio
async def test_coordinator_poll_with_disconnected_device(mocker, coordinator):
# Ensure the coordinator handles a disconnected device during polling
# Regression test for: https://github.com/palazzem/ha-econnect-alarm/issues/148
query = mocker.patch.object(coordinator._device._connection, "query")
coordinator._device._connection.query.side_effect = DeviceDisconnectedError()
# Test
await coordinator._async_update_data()
assert query.call_count == 1
assert coordinator._device.connected is False


@pytest.mark.asyncio
async def test_coordinator_poll_recover_disconnected_device(coordinator):
# Ensure the coordinator recovers the connection state from a previous disconnected device error
coordinator._device.connected = False
# Test
await coordinator._async_update_data()
assert coordinator._device.connected is True


@pytest.mark.asyncio
async def test_coordinator_update_with_disconnected_device(mocker, coordinator):
# Ensure the coordinator handles a disconnected device during updates
# Regression test for: https://github.com/palazzem/ha-econnect-alarm/issues/148
mocker.patch.object(coordinator._device, "has_updates")
mocker.patch.object(coordinator._device._connection, "query")
update = mocker.spy(coordinator._device, "update")
coordinator._device.has_updates.return_value = {"has_changes": True}
coordinator._device._connection.query.side_effect = DeviceDisconnectedError()
# Test
await coordinator._async_update_data()
assert update.call_count == 1
assert coordinator._device.connected is False


@pytest.mark.asyncio
async def test_coordinator_update_recover_disconnected_device(mocker, coordinator):
# Ensure the coordinator recovers the connection state from a previous disconnected device error
mocker.patch.object(coordinator._device, "has_updates")
coordinator._device.has_updates.return_value = {"has_changes": True}
coordinator._device.connected = False
# Test
await coordinator._async_update_data()
assert coordinator._device.connected is True


@pytest.mark.asyncio
async def test_coordinator_async_update_disconnected_device(mocker, coordinator):
# Ensure a full update is forced if the device was previously disconnected
# Regression test for: https://github.com/palazzem/ha-econnect-alarm/issues/148
coordinator._device.connected = False
mocker.spy(coordinator._device, "update")
mocker.spy(coordinator._device, "has_updates")
# Test
await coordinator._async_update_data()
assert coordinator._device.update.call_count == 1
assert coordinator._device.has_updates.call_count == 0
6 changes: 6 additions & 0 deletions tests/test_devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def test_device_constructor(client):
"""Should initialize defaults attributes to run properly."""
device = AlarmDevice(client)
# Test
assert device.connected is False
assert device._connection == client
assert device._inventory == {}
assert device._sectors == {}
Expand All @@ -50,6 +51,7 @@ def test_device_constructor_with_config(client):
}
device = AlarmDevice(client, config=config)
# Test
assert device.connected is False
assert device._connection == client
assert device._inventory == {}
assert device._sectors == {}
Expand All @@ -73,6 +75,7 @@ def test_device_constructor_with_config_empty(client):
}
device = AlarmDevice(client, config=config)
# Test
assert device.connected is False
assert device._connection == client
assert device._inventory == {}
assert device._sectors == {}
Expand Down Expand Up @@ -364,6 +367,7 @@ def test_device_connect(client, mocker):
assert device._connection.auth.call_count == 1
assert "username" == device._connection.auth.call_args[0][0]
assert "password" == device._connection.auth.call_args[0][1]
assert device.connected is True


def test_device_connect_error(client, mocker):
Expand All @@ -375,6 +379,7 @@ def test_device_connect_error(client, mocker):
with pytest.raises(HTTPError):
device.connect("username", "password")
assert device._connection.auth.call_count == 1
assert device.connected is False


def test_device_connect_credential_error(client, mocker):
Expand All @@ -386,6 +391,7 @@ def test_device_connect_credential_error(client, mocker):
with pytest.raises(CredentialError):
device.connect("username", "password")
assert device._connection.auth.call_count == 1
assert device.connected is False


def test_device_has_updates(client, mocker):
Expand Down

0 comments on commit 72ed533

Please sign in to comment.