Skip to content

fix(providers): prevent startup race and false not connected errors (#708) #709

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.

Sign up for GitHub

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

Open
firstof9 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(providers): prevent startup race and false not connected errors (#708) #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions custom_components/keymaster/coordinator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2382,10 +2382,16 @@ async def _connect_and_update_lock(self, kmlock: KeymasterLock) -> bool:
kmlock.connected = await kmlock.provider.async_connect()

if not kmlock.connected:
_LOGGER.error(
"[Coordinator] %s: Provider failed to connect",
kmlock.lock_name,
)
if self.hass.is_running:
_LOGGER.error(
"[Coordinator] %s: Provider failed to connect",
kmlock.lock_name,
)
else:
_LOGGER.debug(
"[Coordinator] %s: Provider not connected yet during startup",
kmlock.lock_name,
)
return False

if kmlock.provider.lock_config_entry_id:
Expand Down Expand Up @@ -2634,8 +2640,13 @@ async def _update_lock_data(self, keymaster_config_entry_id: str) -> None:
failures,
backoff_secs,
)
else:
elif self.hass.is_running:
_LOGGER.error("[Coordinator] %s: Not Connected", kmlock.lock_name)
else:
_LOGGER.debug(
"[Coordinator] %s: Not connected yet during startup",
kmlock.lock_name,
)
return

if not kmlock.provider:
Expand Down
43 changes: 24 additions & 19 deletions custom_components/keymaster/providers/schlage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from typing import Any, cast

from custom_components.keymaster.const import CONF_SLOTS, CONF_START
from homeassistant.config_entries import ConfigEntryState
from homeassistant.exceptions import HomeAssistantError

from ._base import BaseLockProvider, CodeSlot
Expand Down Expand Up @@ -107,13 +108,19 @@ async def async_connect(self) -> bool:
)
return False

try:
coordinator = schlage_entry.runtime_data
except (AttributeError, TypeError) as e:
_LOGGER.error(
"[SchlageProvider] Can't access Schlage coordinator: %s: %s",
e.__class__.__qualname__,
e,
if getattr(schlage_entry, "state", None) != ConfigEntryState.LOADED:
_LOGGER.debug(
"[SchlageProvider] Schlage config entry %s is not loaded yet (state: %s)",
self.lock_config_entry_id,
getattr(schlage_entry, "state", None),
)
return False

coordinator = getattr(schlage_entry, "runtime_data", None)
if coordinator is None:
_LOGGER.debug(
"[SchlageProvider] Schlage coordinator runtime_data not yet available: %s",
self.lock_config_entry_id,
)
return False

Expand Down Expand Up @@ -176,21 +183,19 @@ async def async_is_connected(self) -> bool:
return False

schlage_entry = self.hass.config_entries.async_get_entry(lock_entry.config_entry_id)
if not schlage_entry:
if not schlage_entry or getattr(schlage_entry, "state", None) != ConfigEntryState.LOADED:
self._connected = False
return False

try:
coordinator = schlage_entry.runtime_data
connected = self._schlage_device_id in coordinator.data.locks
except (
AttributeError,
TypeError,
):
connected = False

self._connected = connected
return connected
coordinator = getattr(schlage_entry, "runtime_data", None)
if coordinator is None or not hasattr(coordinator, "data"):
self._connected = False
return False

self._connected = bool(
coordinator.data.locks and self._schlage_device_id in coordinator.data.locks
)
return self._connected

async def _async_get_codes(self) -> dict[str, dict[str, str]]:
"""Call ``schlage.get_codes`` and return the response dict.
Expand Down
43 changes: 29 additions & 14 deletions custom_components/keymaster/providers/zwave_js.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from homeassistant.components.lock import LockState
from homeassistant.components.zwave_js import ZWAVE_JS_NOTIFICATION_EVENT
from homeassistant.components.zwave_js.const import ATTR_PARAMETERS, DOMAIN as ZWAVE_JS_DOMAIN
from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import ATTR_DEVICE_ID, STATE_UNAVAILABLE, STATE_UNKNOWN
from homeassistant.core import Event, EventStateChangedData
from homeassistant.helpers.device_registry import DeviceEntry
Expand Down Expand Up @@ -400,21 +401,35 @@ async def async_connect(self) -> bool:
)
return False

try:
zwave_entry = self.hass.config_entries.async_get_entry(self.lock_config_entry_id)
if not zwave_entry:
_LOGGER.error(
"[ZWaveJSProvider] Can't find Z-Wave JS config entry: %s",
self.lock_config_entry_id,
)
return False

self._client = zwave_entry.runtime_data.client
except (KeyError, TypeError, AttributeError) as e:
zwave_entry = self.hass.config_entries.async_get_entry(self.lock_config_entry_id)
if not zwave_entry:
_LOGGER.error(
"[ZWaveJSProvider] Can't access Z-Wave JS client: %s: %s",
e.__class__.__qualname__,
e,
"[ZWaveJSProvider] Can't find Z-Wave JS config entry: %s",
self.lock_config_entry_id,
)
return False

if getattr(zwave_entry, "state", None) != ConfigEntryState.LOADED:
_LOGGER.debug(
"[ZWaveJSProvider] Z-Wave JS config entry %s is not loaded yet (state: %s)",
self.lock_config_entry_id,
getattr(zwave_entry, "state", None),
)
return False

runtime_data = getattr(zwave_entry, "runtime_data", None)
if runtime_data is None:
_LOGGER.debug(
"[ZWaveJSProvider] Z-Wave JS config entry %s runtime_data not yet available",
self.lock_config_entry_id,
)
return False

self._client = getattr(runtime_data, "client", None)
if self._client is None:
_LOGGER.debug(
"[ZWaveJSProvider] Z-Wave JS client not yet available on runtime_data: %s",
self.lock_config_entry_id,
)
return False

Expand Down
39 changes: 38 additions & 1 deletion tests/providers/test_schlage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
_make_tagged_name,
_parse_tag,
)
from homeassistant.config_entries import ConfigEntryState
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError

Expand Down Expand Up @@ -151,6 +152,7 @@ async def test_connect_success(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
coordinator = MagicMock()
coordinator.data.locks = {"schlage_device_123": MagicMock()}
schlage_entry.runtime_data = coordinator
Expand Down Expand Up @@ -187,13 +189,26 @@ async def test_connect_schlage_entry_not_found(self, schlage_provider):
schlage_provider.hass.config_entries.async_get_entry.return_value = None
assert await schlage_provider.async_connect() is False

async def test_connect_schlage_entry_not_loaded(self, schlage_provider):
"""Test connection fails when schlage config entry is not loaded."""
lock_entry = MagicMock()
lock_entry.config_entry_id = "schlage_entry"
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.SETUP_IN_PROGRESS
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry
assert await schlage_provider.async_connect() is False

async def test_connect_coordinator_not_available(self, schlage_provider):
"""Test connection fails when coordinator unavailable."""
lock_entry = MagicMock()
lock_entry.config_entry_id = "schlage_entry"
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock(spec_set=[])
schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
schlage_entry.runtime_data = None
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry
assert await schlage_provider.async_connect() is False

Expand All @@ -205,6 +220,7 @@ async def test_connect_no_device_entry(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
schlage_entry.runtime_data = MagicMock()
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry
schlage_provider.device_registry.async_get.return_value = None
Expand All @@ -218,6 +234,7 @@ async def test_connect_no_schlage_identifier(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
schlage_entry.runtime_data = MagicMock()
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry

Expand All @@ -234,6 +251,7 @@ async def test_connect_lock_not_in_coordinator(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
coordinator = MagicMock()
coordinator.data.locks = {} # Empty
schlage_entry.runtime_data = coordinator
Expand All @@ -252,6 +270,7 @@ async def test_connect_coordinator_data_missing(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
coordinator = type("Coordinator", (), {})()
schlage_entry.runtime_data = coordinator
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry
Expand Down Expand Up @@ -284,6 +303,7 @@ async def test_connected_lock_in_coordinator(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
coordinator = MagicMock()
coordinator.data.locks = {"dev123": MagicMock()}
schlage_entry.runtime_data = coordinator
Expand All @@ -300,13 +320,29 @@ async def test_not_connected_lock_removed(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
coordinator = MagicMock()
coordinator.data.locks = {}
schlage_entry.runtime_data = coordinator
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry

assert await schlage_provider.async_is_connected() is False

async def test_not_connected_schlage_entry_not_loaded(self, schlage_provider):
"""Test returns False when Schlage config entry is not loaded."""
schlage_provider._schlage_device_id = "dev123"

lock_entry = MagicMock()
lock_entry.config_entry_id = "schlage_entry"
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.SETUP_IN_PROGRESS
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry

assert await schlage_provider.async_is_connected() is False
assert schlage_provider._connected is False

async def test_not_connected_lock_entry_missing(self, schlage_provider):
"""Test returns False when entity registry entry is missing."""
schlage_provider._schlage_device_id = "dev123"
Expand Down Expand Up @@ -347,6 +383,7 @@ async def test_not_connected_coordinator_error(self, schlage_provider):
schlage_provider.entity_registry.async_get.return_value = lock_entry

schlage_entry = MagicMock()
schlage_entry.state = ConfigEntryState.LOADED
schlage_entry.runtime_data = None # causes AttributeError on .data.locks
schlage_provider.hass.config_entries.async_get_entry.return_value = schlage_entry

Expand Down
49 changes: 49 additions & 0 deletions tests/providers/test_zwave_js.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
)
from custom_components.keymaster.providers.zwave_js import ZWaveJSLockProvider
from homeassistant.components.lock.const import LockState
from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import EVENT_HOMEASSISTANT_STARTED
from homeassistant.core import HomeAssistant
from tests.common import async_capture_events
Expand Down Expand Up @@ -88,6 +89,7 @@ def setup_successful_connect(
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock()
mock_zwave_entry.runtime_data.client = mock_zwave_client
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry
Expand Down Expand Up @@ -216,6 +218,49 @@ async def test_connect_zwave_entry_not_found(self, zwave_provider):

assert result is False

async def test_connect_zwave_entry_not_loaded(self, zwave_provider):
"""Test connect fails when Z-Wave config entry is not loaded."""
mock_entity = MagicMock()
mock_entity.config_entry_id = "zwave_entry_id"
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.SETUP_IN_PROGRESS
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry

result = await zwave_provider.async_connect()

assert result is False

async def test_connect_runtime_data_missing(self, zwave_provider):
"""Test connect fails when Z-Wave runtime_data is missing."""
mock_entity = MagicMock()
mock_entity.config_entry_id = "zwave_entry_id"
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock(spec=["state"])
mock_zwave_entry.state = ConfigEntryState.LOADED
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry

result = await zwave_provider.async_connect()

assert result is False

async def test_connect_client_missing_on_runtime_data(self, zwave_provider):
"""Test connect fails when Z-Wave client attribute is None on runtime_data."""
mock_entity = MagicMock()
mock_entity.config_entry_id = "zwave_entry_id"
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock(spec=[])
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry

result = await zwave_provider.async_connect()

assert result is False

async def test_connect_client_not_connected(self, zwave_provider):
"""Test connect fails when Z-Wave client not connected."""
mock_entity = MagicMock()
Expand All @@ -224,6 +269,7 @@ async def test_connect_client_not_connected(self, zwave_provider):
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock()
mock_zwave_entry.runtime_data.client = MagicMock()
mock_zwave_entry.runtime_data.client.connected = False
Expand All @@ -241,6 +287,7 @@ async def test_connect_device_not_found(self, zwave_provider, mock_zwave_client)
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock()
mock_zwave_entry.runtime_data.client = mock_zwave_client
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry
Expand All @@ -258,6 +305,7 @@ async def test_connect_node_id_not_found(self, zwave_provider, mock_zwave_client
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock()
mock_zwave_entry.runtime_data.client = mock_zwave_client
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry
Expand Down Expand Up @@ -1447,6 +1495,7 @@ async def test_connect_warns_when_dead_but_proceeds(
zwave_provider.entity_registry.async_get.return_value = mock_entity

mock_zwave_entry = MagicMock()
mock_zwave_entry.state = ConfigEntryState.LOADED
mock_zwave_entry.runtime_data = MagicMock()
mock_zwave_entry.runtime_data.client = mock_zwave_client
zwave_provider.hass.config_entries.async_get_entry.return_value = mock_zwave_entry
Expand Down
Loading
Loading