Switch from block list to allow list, add test to ensure certain nodes are skipped, fix test

app/fanout/map_upload.py

@@ -2,7 +2,7 @@
 
 Mirrors the logic of the standalone map.meshcore.dev-uploader project:
 - Listens on raw RF packets via on_raw
-- Filters for ADVERT packets, skips CHAT nodes (device_role == 1)
+- Filters for ADVERT packets, only processes repeaters (role 2) and rooms (role 3)
 - Skips nodes with no valid location (lat/lon None)
 - Applies per-pubkey rate-limiting (1-hour window, matching the uploader)
 - Signs the upload request with the radio's own Ed25519 private key
@@ -52,8 +52,10 @@ _DEFAULT_API_URL = "https://map.meshcore.dev/api/v1/uploader/node"
 # Re-upload guard: skip re-uploading a pubkey seen within this window (AU parity)
 _REUPLOAD_SECONDS = 3600
 
-# Device role 1 = CHAT — skip these; repeaters (2) and rooms (3) are the map targets
-_SKIP_DEVICE_ROLES = {1}
+# Only upload repeaters (2) and rooms (3). Any other role — including future
+# roles not yet defined — is rejected. An allowlist is used rather than a
+# blocklist so that new roles cannot accidentally start populating the map.
+_ALLOWED_DEVICE_ROLES = {2, 3}
 
 # Ed25519 group order (L)
 _L = 2**252 + 27742317777372353535851937790883648493
@@ -108,12 +110,12 @@ def _get_radio_params() -> dict:
             "sf": sf,
             "bw": bw / 1000.0 if bw else 0,
         }
-    except Exception:
-        pass
+    except Exception as exc:
+        logger.debug("MapUpload: could not read radio params: %s", exc)
     return {"freq": 0, "cr": 0, "sf": 0, "bw": 0}
 
 
-_ROLE_NAMES: dict[int, str] = {2: "repeater", 3: "room", 4: "sensor"}
+_ROLE_NAMES: dict[int, str] = {2: "repeater", 3: "room"}
 
 
 def _haversine_km(lat1: float, lon1: float, lat2: float, lon2: float) -> float:
@@ -145,6 +147,7 @@ class MapUploadModule(FanoutModule):
         if self._client:
             await self._client.aclose()
             self._client = None
+        self._last_error = None
 
     async def on_raw(self, data: dict) -> None:
         if data.get("payload_type") != "ADVERT":
@@ -173,8 +176,8 @@ class MapUploadModule(FanoutModule):
         # nacl.bindings.crypto_sign_open(sig + (pubkey_bytes || timestamp_bytes),
         # advert.public_key_bytes) succeeds before proceeding.
 
-        # Skip CHAT-type nodes (role 1) — map only shows repeaters (2) and rooms (3)
-        if advert.device_role in _SKIP_DEVICE_ROLES:
+        # Only process repeaters (2) and rooms (3) — any other role is rejected
+        if advert.device_role not in _ALLOWED_DEVICE_ROLES:
             return
 
         # Skip nodes with no valid location — the decoder already nulls out

tests/test_map_upload.py

@@ -3,7 +3,7 @@
 from __future__ import annotations
 
 import json
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch
 
 import pytest
 
@@ -72,8 +72,10 @@ class TestMapUploadLifecycle:
     async def test_stop_clears_client(self):
         mod = _make_module()
         await mod.start()
+        mod._last_error = "HTTP 500"  # simulate a prior error
         await mod.stop()
         assert mod._client is None
+        assert mod._last_error is None
         assert mod.status == "disconnected"
 
     @pytest.mark.asyncio
@@ -184,6 +186,28 @@ class TestOnRawFiltering:
 
         await mod.stop()
 
+    @pytest.mark.asyncio
+    async def test_sensor_advert_skipped(self):
+        """device_role == 4 (Sensor) must be skipped."""
+        mod = _make_module()
+        await mod.start()
+
+        mock_packet = MagicMock()
+        mock_packet.payload = b"\x00" * 101
+
+        with (
+            patch("app.fanout.map_upload.parse_packet", return_value=mock_packet),
+            patch(
+                "app.fanout.map_upload.parse_advertisement",
+                return_value=_fake_advert(device_role=4),
+            ),
+            patch.object(mod, "_upload", new_callable=AsyncMock) as mock_upload,
+        ):
+            await mod.on_raw(_advert_raw_data())
+            mock_upload.assert_not_called()
+
+        await mod.stop()
+
     @pytest.mark.asyncio
     async def test_repeater_advert_processed(self):
         """device_role == 2 (Repeater) must be uploaded."""
@@ -646,7 +670,9 @@ class TestGetRadioParams:
         assert params == {"freq": 0, "cr": 0, "sf": 0, "bw": 0}
 
     def test_returns_zeros_on_exception(self):
-        with patch("app.fanout.map_upload.radio_runtime", side_effect=Exception("boom")):
+        mock_rt = MagicMock()
+        type(mock_rt).meshcore = PropertyMock(side_effect=Exception("boom"))
+        with patch("app.fanout.map_upload.radio_runtime", mock_rt):
             params = _get_radio_params()
         assert params == {"freq": 0, "cr": 0, "sf": 0, "bw": 0}