diff --git a/app/fanout/mqtt_community.py b/app/fanout/mqtt_community.py index 3d2433c..c1de421 100644 --- a/app/fanout/mqtt_community.py +++ b/app/fanout/mqtt_community.py @@ -98,15 +98,48 @@ class MqttCommunityModule(FanoutModule): @property def status(self) -> str: + if self.last_error: + return "error" if self._publisher._is_configured(): - if self._publisher.last_error: - return "error" return "connected" if self._publisher.connected else "disconnected" return "disconnected" @property def last_error(self) -> str | None: - return self._publisher.last_error + if self._publisher.last_error: + return self._publisher.last_error + return self._key_unavailable_reason() + + def _key_unavailable_reason(self) -> str | None: + """Explain a silent "disconnected" caused by a missing radio key. + + Community MQTT can only authenticate (and derive its topic/identity) + with the radio's own key, which is exported from the radio on connect. + When the radio is fully connected and set up but the keystore is still + empty, the key export was refused or never answered -- typically + firmware without ENABLE_PRIVATE_KEY_EXPORT, or a proxy transport + (e.g. Meshmonitor) that does not forward the key-export command. + + Surfacing this as the module's ``last_error`` promotes the fanout status + to "error" and lights up the existing per-card error detail, instead of + leaving the operator staring at a bare "Disconnected". See issue #321. + + Gated on ``is_setup_complete`` so we do not falsely accuse the radio + during the normal connect/key-export window. + """ + from app.keystore import get_public_key + from app.services.radio_runtime import radio_runtime as radio_manager + + if get_public_key() is not None: + return None + if not (radio_manager.is_connected and radio_manager.is_setup_complete): + return None + return ( + "Community MQTT needs the radio's private key to authenticate, but it " + "isn't available. Your radio firmware may not support key export " + "(ENABLE_PRIVATE_KEY_EXPORT=1), or you're connecting through a proxy " + "that doesn't forward the key-export command." + ) async def _publish_community_packet( diff --git a/tests/test_community_mqtt.py b/tests/test_community_mqtt.py index eb4b7b3..fcc06ae 100644 --- a/tests/test_community_mqtt.py +++ b/tests/test_community_mqtt.py @@ -24,6 +24,7 @@ from app.fanout.community_mqtt import ( _get_client_version, ) from app.fanout.mqtt_community import ( + MqttCommunityModule, _config_to_settings, _publish_community_packet, _render_packet_topic, @@ -659,6 +660,85 @@ class TestPublishFailureSetsDisconnected: assert "if it self-resolves" in caplog.text +class TestCommunityModuleKeyUnavailable: + """The module should explain a silent 'disconnected' caused by a missing radio key. + + Regression coverage for issue #321: connecting RemoteTerm through a proxy + (e.g. Meshmonitor) that doesn't forward the key-export command leaves the + keystore empty, so community MQTT is never configured. Previously that + showed a bare "Disconnected" with no reason. + """ + + @staticmethod + def _make_module() -> MqttCommunityModule: + return MqttCommunityModule("cfg-id", {"iata": "LAX"}, name="LetsMesh") + + def test_reports_reason_when_connected_setup_complete_and_no_key(self): + module = self._make_module() + with ( + patch("app.keystore.get_public_key", return_value=None), + patch( + "app.services.radio_runtime.radio_runtime", + SimpleNamespace(is_connected=True, is_setup_complete=True), + ), + ): + reason = module.last_error + assert reason is not None + assert "key export" in reason + assert "ENABLE_PRIVATE_KEY_EXPORT" in reason + assert "proxy" in reason + # A populated last_error promotes the status to "error" so the + # fanout card shows the actionable detail instead of "Disconnected". + assert module.status == "error" + + def test_no_reason_when_key_available(self): + module = self._make_module() + with ( + patch("app.keystore.get_public_key", return_value=b"x" * 32), + patch( + "app.services.radio_runtime.radio_runtime", + SimpleNamespace(is_connected=True, is_setup_complete=True), + ), + ): + assert module.last_error is None + + def test_no_reason_when_radio_not_connected(self): + module = self._make_module() + with ( + patch("app.keystore.get_public_key", return_value=None), + patch( + "app.services.radio_runtime.radio_runtime", + SimpleNamespace(is_connected=False, is_setup_complete=False), + ), + ): + assert module.last_error is None + + def test_no_reason_during_setup_window(self): + # Connected but setup still running: the key export may not have happened + # yet, so we must not falsely accuse the radio. + module = self._make_module() + with ( + patch("app.keystore.get_public_key", return_value=None), + patch( + "app.services.radio_runtime.radio_runtime", + SimpleNamespace(is_connected=True, is_setup_complete=False), + ), + ): + assert module.last_error is None + + def test_real_publisher_error_takes_precedence(self): + module = self._make_module() + module._publisher._last_error = "broker gone" + with ( + patch("app.keystore.get_public_key", return_value=None), + patch( + "app.services.radio_runtime.radio_runtime", + SimpleNamespace(is_connected=True, is_setup_complete=True), + ), + ): + assert module.last_error == "broker gone" + + class TestBuildStatusTopic: def test_builds_correct_topic(self): settings = SimpleNamespace(community_mqtt_iata="LAX")