diff --git a/readme.md b/readme.md index 8337c0f6..04a545df 100644 --- a/readme.md +++ b/readme.md @@ -127,6 +127,9 @@ MyOpenhabRule() ``` # Changelog +#### 24.08.1 (2024-08-02) +- Fixed a possible infinite loop during thing re-sync + #### 24.08.0 (2024-08-01) - Fixed an issue with thing re-sync - Updated number parsing logic diff --git a/src/HABApp/__version__.py b/src/HABApp/__version__.py index dd7e031b..f3be767d 100644 --- a/src/HABApp/__version__.py +++ b/src/HABApp/__version__.py @@ -10,4 +10,4 @@ # Development versions contain the DEV-COUNTER postfix: # - 24.01.0.DEV-1 -__version__ = '24.08.0' +__version__ = '24.08.1' diff --git a/src/HABApp/openhab/connection/plugins/load_items.py b/src/HABApp/openhab/connection/plugins/load_items.py index 04a6a7cb..0dfa9815 100644 --- a/src/HABApp/openhab/connection/plugins/load_items.py +++ b/src/HABApp/openhab/connection/plugins/load_items.py @@ -2,7 +2,7 @@ import logging from asyncio import sleep -from datetime import datetime +from typing import TYPE_CHECKING from immutables import Map @@ -13,16 +13,22 @@ from HABApp.openhab.connection.handler import map_null_str from HABApp.openhab.connection.handler.func_async import async_get_all_items_state, async_get_items, async_get_things from HABApp.openhab.definitions import QuantityValue -from HABApp.openhab.definitions.rest import ThingResp from HABApp.openhab.item_to_reg import ( add_thing_to_registry, add_to_registry, fresh_item_sync, + get_thing_status_from_resp, remove_from_registry, remove_thing_from_registry, ) +if TYPE_CHECKING: + from datetime import datetime + + from HABApp.openhab.definitions.rest import ThingResp + + log = logging.getLogger('HABApp.openhab.items') Items = uses_item_registry() @@ -30,19 +36,36 @@ class LoadOpenhabItemsPlugin(BaseConnectionPlugin[OpenhabConnection]): async def on_connected(self, context: OpenhabContext): + # The context will be created fresh for each connect if not context.created_items and not context.created_things: await self.load_items(context) await self.load_things(context) + + # We create the same plugin twice because it uses the same logic to load the objects, + # One plugin instance will create the objects the other one will sync the state. + # That's why this is in the else branch else: - # Sleep so the event handler is running + # Sleep so we make sure that the openhab event handler is running await sleep(0.1) + # First two delays are 0: First sync and the completion check after the first sync + delays = (0, 0, *(2 ** i for i in range(6))) + if context.created_items: - while await self.sync_items(context): - pass + for d in delays: + await sleep(d) + if not await self.sync_items(context): + break + else: + log.warning(f'Item state sync failed!') + if context.created_things: - while await self.sync_things(context): - pass + for d in delays: + await sleep(d) + if not await self.sync_things(context): + break + else: + log.warning(f'Thing sync failed!') async def load_items(self, context: OpenhabContext): from HABApp.openhab.map_items import map_item @@ -145,9 +168,11 @@ async def sync_things(self, context: OpenhabContext): existing_thing, existing_datetime = created_things[thing.uid] if thing_changed(existing_thing, thing) and existing_thing.last_update != existing_datetime: - existing_thing.status = thing.status.status - existing_thing.status_description = thing.status.description - existing_thing.status_detail = thing.status.detail if thing.status.detail else '' + new_status, new_status_detail, new_status_description = get_thing_status_from_resp(thing) + + existing_thing.status = new_status + existing_thing.status_detail = new_status_detail + existing_thing.status_description = new_status_description existing_thing.label = thing.label existing_thing.location = thing.location existing_thing.configuration = Map(thing.configuration) @@ -155,14 +180,17 @@ async def sync_things(self, context: OpenhabContext): log.debug(f'Re-synced {existing_thing.name:s}') synced += 1 + log.debug('Thing sync complete') return synced def thing_changed(old: HABApp.openhab.items.Thing, new: ThingResp) -> bool: - return old.status != new.status.status or \ - old.status_detail != new.status.detail or \ - old.status_description != ('' if not new.status.description else new.status.description) or \ + new_status, new_status_detail, new_status_description = get_thing_status_from_resp(new) + + return old.status != new_status or \ + old.status_detail != new_status_detail or \ + old.status_description != new_status_description or \ old.label != new.label or \ old.location != new.location or \ - old.configuration != new.configuration or \ - old.properties != new.properties + old.configuration != Map(new.configuration) or \ + old.properties != Map(new.properties) diff --git a/src/HABApp/openhab/item_to_reg.py b/src/HABApp/openhab/item_to_reg.py index 248ecfb2..791b97eb 100644 --- a/src/HABApp/openhab/item_to_reg.py +++ b/src/HABApp/openhab/item_to_reg.py @@ -1,5 +1,7 @@ +from __future__ import annotations + import logging -from typing import TYPE_CHECKING, Dict, Set, Tuple, Union +from typing import TYPE_CHECKING from immutables import Map @@ -15,14 +17,16 @@ if TYPE_CHECKING: - import HABApp.openhab.definitions.rest + from HABApp.openhab.definitions.rest import ThingResp + from HABApp.openhab.events import ThingAddedEvent + from HABApp.openhab.items import OpenhabItem log = logging.getLogger('HABApp.openhab.items') Items = uses_item_registry() -def add_to_registry(item: 'HABApp.openhab.items.OpenhabItem', set_value=False): +def add_to_registry(item: OpenhabItem, set_value=False): name = item.name for grp in item.groups: MEMBERS.setdefault(grp, set()).add(name) @@ -70,17 +74,17 @@ def remove_from_registry(name: str): return None -MEMBERS: Dict[str, Set[str]] = {} +MEMBERS: dict[str, set[str]] = {} def fresh_item_sync(): MEMBERS.clear() -def get_members(group_name: str) -> Tuple['HABApp.openhab.items.OpenhabItem', ...]: +def get_members(group_name: str) -> tuple[OpenhabItem, ...]: ret = [] for name in MEMBERS.get(group_name, []): - item = Items.get_item(name) # type: HABApp.openhab.items.OpenhabItem + item = Items.get_item(name) # type: OpenhabItem ret.append(item) return tuple(sorted(ret, key=lambda x: x.name)) @@ -88,21 +92,28 @@ def get_members(group_name: str) -> Tuple['HABApp.openhab.items.OpenhabItem', .. # ---------------------------------------------------------------------------------------------------------------------- # Thing handling # ---------------------------------------------------------------------------------------------------------------------- -def add_thing_to_registry(data: Union['HABApp.openhab.definitions.rest.ThingResp', - 'HABApp.openhab.events.thing_events.ThingAddedEvent']): + +def get_thing_status_from_resp( + obj: ThingResp | None) -> tuple[ThingStatusEnum, ThingStatusDetailEnum, str]: + if obj is None: + return THING_STATUS_DEFAULT, THING_STATUS_DETAIL_DEFAULT, '' + return ( + obj.status.status, + obj.status.detail, + obj.status.description if obj.status.description is not None else '' + ) + + +def add_thing_to_registry(data: ThingResp | ThingAddedEvent): if isinstance(data, HABApp.openhab.events.thing_events.ThingAddedEvent): name = data.name - status: ThingStatusEnum = THING_STATUS_DEFAULT - status_detail: ThingStatusDetailEnum = THING_STATUS_DETAIL_DEFAULT - status_description: str = '' + status, status_detail, status_description = get_thing_status_from_resp(None) elif isinstance(data, HABApp.openhab.definitions.rest.ThingResp): name = data.uid - status = data.status.status - status_detail = data.status.detail - status_description = data.status.description if data.status.description else '' + status, status_detail, status_description = get_thing_status_from_resp(data) else: - raise ValueError() + raise TypeError() if Items.item_exists(name): existing = Items.get_item(name) diff --git a/tests/test_openhab/test_items/test_commands.py b/tests/test_openhab/test_items/test_commands.py index e78c607c..3977c6ed 100644 --- a/tests/test_openhab/test_items/test_commands.py +++ b/tests/test_openhab/test_items/test_commands.py @@ -13,7 +13,7 @@ def test_OnOff(cls): c = cls('item_name') assert not c.is_on() - if not __version__.startswith('24.08.0'): + if not __version__.startswith('24.08.1'): assert not c.is_off() c.set_value(OnOffValue('ON')) diff --git a/tests/test_openhab/test_plugins/test_load_items.py b/tests/test_openhab/test_plugins/test_load_items.py index 17699231..407b7754 100644 --- a/tests/test_openhab/test_plugins/test_load_items.py +++ b/tests/test_openhab/test_plugins/test_load_items.py @@ -3,12 +3,15 @@ from typing import List import msgspec.json +from pendulum import DateTime import HABApp.openhab.connection.plugins.load_items as load_items_module from HABApp.core.internals import ItemRegistry from HABApp.openhab.connection.connection import OpenhabContext from HABApp.openhab.connection.plugins import LoadOpenhabItemsPlugin -from HABApp.openhab.definitions.rest import ItemResp, ShortItemResp +from HABApp.openhab.definitions.rest import ItemResp, ShortItemResp, ThingResp +from HABApp.openhab.definitions.rest.things import ThingStatusResp +from HABApp.openhab.items import Thing async def _mock_get_all_items(): @@ -64,14 +67,18 @@ async def _mock_get_all_items_state(): ] -async def _mock_get_things(): +async def _mock_get_empty(): return [] +async def _mock_raise(): + raise ValueError() + + async def test_item_sync(monkeypatch, ir: ItemRegistry, test_logs): monkeypatch.setattr(load_items_module, 'async_get_items', _mock_get_all_items) monkeypatch.setattr(load_items_module, 'async_get_all_items_state', _mock_get_all_items_state) - monkeypatch.setattr(load_items_module, 'async_get_things', _mock_get_things) + monkeypatch.setattr(load_items_module, 'async_get_things', _mock_get_empty) context = OpenhabContext.new_context(version=(1, 0, 0), session=None, session_options=None,) @@ -86,3 +93,62 @@ async def test_item_sync(monkeypatch, ir: ItemRegistry, test_logs): test_logs.add_expected('HABApp.openhab.items', logging.WARNING, 'Item ItemLength is a UoM item but "unit" is not found in item metadata') + + +async def test_thing_sync(monkeypatch, ir: ItemRegistry, test_logs): + monkeypatch.setattr(load_items_module, 'async_get_items', _mock_get_empty) + monkeypatch.setattr(load_items_module, 'async_get_all_items_state', _mock_raise) + + things_resp: list[ThingResp] = [] + + async def _mock_ret(): + return things_resp + + monkeypatch.setattr(load_items_module, 'async_get_things', _mock_ret) + + t1 = ThingResp( + uid='thing_1', thing_type='thing_type_1', editable=True, status=ThingStatusResp( + status='ONLINE', detail='NONE', description='' + ) + ) + + t2 = ThingResp( + uid='thing_2', thing_type='thing_type_2', editable=True, status=ThingStatusResp( + status='OFFLINE', detail='NONE', description='' + ) + ) + + things_resp = [t1, t2] + + context = OpenhabContext.new_context(version=(1, 0, 0), session=None, session_options=None,) + + # initial thing create + await LoadOpenhabItemsPlugin().on_connected(context) + + ir_thing = ir.get_item('thing_2') + assert isinstance(ir_thing, Thing) + assert ir_thing.status_description == '' + + ir_thing._last_update.set(DateTime(2001, 1, 1)) + t2.status.description = 'asdf' + + # sync state + await LoadOpenhabItemsPlugin().on_connected(context) + + assert ir.get_item('thing_2').status_description == 'asdf' + + assert test_logs.copy().set_min_level(10).update().get_messages() == [ + ' [HABApp.openhab.items] | DEBUG | Requesting items', + ' [HABApp.openhab.items] | DEBUG | Got response with 0 items', + ' [HABApp.openhab.items] | INFO | Updated 0 Items', + ' [HABApp.openhab.items] | DEBUG | Requesting things', + ' [HABApp.openhab.items] | DEBUG | Got response with 2 things', + ' [ HABApp.Items] | DEBUG | Added thing_1 (Thing)', + ' [ HABApp.Items] | DEBUG | Added thing_2 (Thing)', + ' [HABApp.openhab.items] | INFO | Updated 2 Things', + ' [HABApp.openhab.items] | DEBUG | Starting Thing sync', + ' [HABApp.openhab.items] | DEBUG | Re-synced thing_2', + ' [HABApp.openhab.items] | DEBUG | Thing sync complete', + ' [HABApp.openhab.items] | DEBUG | Starting Thing sync', + ' [HABApp.openhab.items] | DEBUG | Thing sync complete', + ]