fix(channel): improve channel fetching with retries and handle empty results cleanly (#266)

CHANGELOG.md

@@ -8,6 +8,11 @@ semantic versioning.
 
 ### Fixed
 
+- A radio connection is no longer accepted when every channel read times out or
+  returns no usable channel data (#266). Startup and reconnect now retry the
+  channel scan three times, keep an empty result out of the valid cache and
+  database, then fail the connection cleanly so the normal restart/reconnect
+  path can try again instead of running a bot that cannot route replies.
 - Daily Weather Service forecasts now retry transient Open-Meteo failures at
   5, 15, and 30 minutes after the original run (#264). HTTP 429, 500, 502, 503,
   and 504 responses plus transport failures use one replaceable retry job,

modules/channel_manager.py

@@ -49,15 +49,26 @@ class ChannelManager:
         """Lowercase channel name without a leading # for cache lookups."""
         return name.removeprefix("#").lower()
 
-    async def fetch_channels(self):
-        """Fetch channels from the MeshCore node using enhanced concurrent fetching"""
+    async def fetch_channels(self, max_attempts: int = 3, retry_delay: float = 2.0) -> bool:
+        """Fetch configured channels, retrying when the node returns no usable data."""
         self.logger.info("Fetching channels from MeshCore node using enhanced concurrent method...")
-        try:
-            # Wait a moment for the device to be ready
-            await asyncio.sleep(2)
+        if max_attempts < 1:
+            raise ValueError("max_attempts must be at least 1")
 
-            # Fetch all channels concurrently
-            channels = await self.fetch_all_channels(force_refresh=True)
+        # Give the companion protocol a moment to settle after transport connect.
+        await asyncio.sleep(2)
+
+        for attempt in range(1, max_attempts + 1):
+            try:
+                channels = await self.fetch_all_channels(force_refresh=True)
+            except Exception as e:
+                channels = []
+                self.logger.warning(
+                    "Channel fetch attempt %d/%d failed: %s",
+                    attempt,
+                    max_attempts,
+                    e,
+                )
 
             if channels:
                 self.logger.info(f"Successfully fetched {len(channels)} channels from MeshCore node")
@@ -68,13 +79,25 @@ class ChannelManager:
                         self.logger.info(f"  Channel {channel_idx}: {channel_name}")
                     else:
                         self.logger.debug(f"  Channel {channel_idx}: (empty)")
-            else:
-                self.logger.warning("No channels found on MeshCore node")
-                self.bot.meshcore.channels = {}
+                return True
 
-        except Exception as e:
-            self.logger.error(f"Failed to fetch channels: {e}")
+            self._cache_valid = False
             self.bot.meshcore.channels = {}
+            if attempt < max_attempts:
+                self.logger.warning(
+                    "No channels received from MeshCore node on attempt %d/%d; "
+                    "retrying in %.1fs",
+                    attempt,
+                    max_attempts,
+                    retry_delay,
+                )
+                await asyncio.sleep(retry_delay)
+
+        self.logger.error(
+            "No channels received from MeshCore node after %d attempt(s)",
+            max_attempts,
+        )
+        return False
 
     async def fetch_all_channels(self, force_refresh: bool = False) -> list[dict[str, Any]]:
         """
@@ -136,14 +159,15 @@ class ChannelManager:
                     break
                 continue
 
-        self._cache_valid = True
+        self._cache_valid = bool(valid_channels)
         self.logger.info(f"Successfully fetched {len(valid_channels)} channels")
 
         # Update the bot's meshcore channels for compatibility
         self.bot.meshcore.channels = self._channels_cache
 
-        # Store channels in database for web viewer access
-        self._store_channels_in_db(valid_channels)
+        # Do not erase the last known channel table on a transient all-empty scan.
+        if valid_channels:
+            self._store_channels_in_db(valid_channels)
 
         return valid_channels
 

modules/core.py

@@ -1839,8 +1839,11 @@ long_jokes = false
                 # Wait for contacts to load
                 await self.wait_for_contacts()
 
-                # Fetch channels
-                await self.channel_manager.fetch_channels()
+                # A connected transport without channel data cannot route replies.
+                if not await self.channel_manager.fetch_channels():
+                    raise ConnectionError(
+                        "MeshCore node returned no channels after retries"
+                    )
 
                 # Setup message event handlers
                 await self.setup_message_handlers()

tests/test_channel_manager_logic.py

@@ -187,6 +187,38 @@ class TestFetchAllChannelsCacheLifecycle:
         # Should have aborted before fetching all 8 channels
         assert call_count < cm.max_channels
         assert channels == []
+        assert cm._cache_valid is False
+        cm._store_channels_in_db.assert_not_called()
+
+
+class TestFetchChannelsRetry:
+    """A connected node with no returned channels must not look ready (#266)."""
+
+    @pytest.mark.asyncio
+    async def test_retries_empty_scan_then_succeeds(self, cm):
+        cm.bot.connected = True
+        channel = {"channel_name": "general", "channel_idx": 0}
+        cm.fetch_all_channels = AsyncMock(side_effect=[[], [channel]])
+
+        with patch("asyncio.sleep", new_callable=AsyncMock):
+            result = await cm.fetch_channels(max_attempts=3, retry_delay=0)
+
+        assert result is True
+        assert cm.fetch_all_channels.await_count == 2
+
+    @pytest.mark.asyncio
+    async def test_exhausted_empty_scans_fail_and_invalidate_cache(self, cm):
+        cm.bot.connected = True
+        cm._cache_valid = True
+        cm.fetch_all_channels = AsyncMock(return_value=[])
+
+        with patch("asyncio.sleep", new_callable=AsyncMock):
+            result = await cm.fetch_channels(max_attempts=3, retry_delay=0)
+
+        assert result is False
+        assert cm.fetch_all_channels.await_count == 3
+        assert cm._cache_valid is False
+        assert cm.bot.meshcore.channels == {}
 
 
 # ---------------------------------------------------------------------------

tests/test_core.py

@@ -868,6 +868,28 @@ class TestTransportReconnect:
         assert bot._radio_relinks_in_progress == 0
         assert bot.keep_running is False
 
+    def test_connect_fails_when_channel_fetch_never_returns_channels(self, tmp_path):
+        bot = self._make_bot(tmp_path, connection_type="serial")
+        new_meshcore = MagicMock()
+        new_meshcore.is_connected = True
+        new_meshcore.self_info = {}
+        new_meshcore.disconnect = AsyncMock()
+
+        bot.wait_for_contacts = AsyncMock()
+        bot.channel_manager.fetch_channels = AsyncMock(return_value=False)
+        bot.setup_message_handlers = AsyncMock()
+
+        with patch(
+            "modules.core.meshcore.MeshCore.create_serial",
+            AsyncMock(return_value=new_meshcore),
+        ):
+            assert asyncio.run(bot.connect()) is False
+
+        new_meshcore.disconnect.assert_awaited_once()
+        bot.setup_message_handlers.assert_not_awaited()
+        assert bot.meshcore is None
+        assert bot.connected is False
+
     def test_keep_running_through_web_viewer_reboot(self, tmp_path):
         bot = self._make_bot(tmp_path)
         bot.meshcore = MagicMock()