Update profile logic

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 2024-08-31 (v0.6.1)
+
+  * Fix bug in channel profile store: while it would not happen in practice, 
+    downstream and upstream channels with same number would cause messages to be kept
+    while channel would be gone for their channel type.
+  * Improved tests of channel profile store.
+
 ## 2024-8-29 (v0.6.0)
 
   * Update dependencies

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "sagemcom-f3896-client"
-version = "0.6.0"
+version = "0.6.1"
 description = ""
 authors = ["Ties de Kock <ties@tiesdekock.nl>"]
 readme = "README.md"

sagemcom_f3896_client/profile_messages.py

@@ -26,20 +26,32 @@ def update_for_channels(
         ds_channels: List[ModemDownstreamChannelResult],
         us_channels: List[ModemUpstreamChannelResult],
     ) -> int:
-        all_channels = frozenset(c.channel_id for c in ds_channels) | frozenset(
-            c.channel_id for c in us_channels
-        )
-        removed = 0
+        ds_channels_ids = frozenset(c.channel_id for c in ds_channels)
+        us_channel_ids = frozenset(c.channel_id for c in us_channels)
+
+        removed = []
         for message in list(self._messages):
-            if message.channel_id not in all_channels:
-                LOG.info(
-                    "Dropping profile message for no longer present downstream channel %d",
-                    message.channel_id,
-                )
-                self._messages.remove(message)
-                removed += 1
-
-        return removed
+            match message:
+                case DownstreamProfileMessage(
+                    channel_id=channel_id
+                ) if channel_id not in ds_channels_ids:
+                    removed.append(message)
+                case UpstreamProfileMessage(
+                    channel_id=channel_id
+                ) if channel_id not in us_channel_ids:
+                    removed.append(message)
+
+        # do not remove while iterating
+        for message in removed:
+            self._messages.remove(message)
+            LOG.info(
+                "Dropping profile message for no longer present channel %d (previous profile: %s, new: %s)",
+                message.channel_id,
+                message.previous_profile,
+                message.profile,
+            )
+
+        return len(removed)
 
     def add(self, message: DownstreamProfileMessage | UpstreamProfileMessage):
         """Add a messsage, removing a message of that type for that channel if present."""

tests/test_profile_messages.py

@@ -30,7 +30,7 @@ def ds_message(
 
 
 def us_message(
-    channel_id: int, profile: Set[int], old_profile: Optional[Set[int]]
+    channel_id: int, profile: Set[int], old_profile: Optional[Set[int]] = None
 ) -> UpstreamProfileMessage:
     return UpstreamProfileMessage(
         channel_id=channel_id,
@@ -46,23 +46,46 @@ def test_profile_messages():
 
     store.add(ds_message(1, [1, 2, 3], None))
     assert len(store) == 1
+    assert ds_message(1, [1, 2, 3], None) in store
+
     # drop a profile/overwrite
     store.add(ds_message(1, [1, 3], None))
     assert len(store) == 1
+    assert ds_message(1, [1, 3], None) in store
 
     # but a upstream message can be added in parallel for the channel
     store.add(us_message(1, [9, 13], None))
     assert len(store) == 2
+    assert ds_message(1, [1, 3], None) in store
+    assert us_message(1, [9, 13]) in store
 
     # Add second upstream
-    store.add(us_message(2, [9, 13], None))
+    store.add(us_message(2, [10, 12], None))
     assert len(store) == 3
+    assert ds_message(1, [1, 3], None) in store
+    assert us_message(1, [9, 13]) in store
+    assert us_message(2, [10, 12]) in store
 
     # And if the channels are present they stay
     store.update_for_channels([channel(1)], [channel(1), channel(2)])
 
     assert len(store) == 3
+    assert ds_message(1, [1, 3], None) in store
+    assert us_message(1, [9, 13]) in store
+    assert us_message(2, [10, 12]) in store
 
     # But are removed when channel disappears
-    store.update_for_channels([], [channel(2)])
+    # remove an upstream first
+    store.update_for_channels([channel(1)], [channel(2)])
+    assert len(store) == 2
+    assert ds_message(1, [1, 3], None) in store
+    assert us_message(2, [10, 12]) in store
+
+    # Now remove a downstream
+    assert store.update_for_channels([], [channel(2)]) == 1
     assert len(store) == 1
+    assert us_message(2, [10, 12]) in store
+
+    # With no channels, it should be empty
+    assert store.update_for_channels([], []) == 1
+    assert len(store) == 0