fix (#451)

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

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'

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,36 +13,59 @@
 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()
 
 
 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,24 +168,29 @@ 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)
                 existing_thing.properties = Map(thing.properties)
                 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)

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,39 +74,46 @@ 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))
 
 
 # ----------------------------------------------------------------------------------------------------------------------
 # 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)

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'))

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',
+    ]