From 68e860dc6873f6264a54641b5e7d305f1b30a80f Mon Sep 17 00:00:00 2001 From: Kumar Challa Date: Sun, 9 Aug 2026 04:21:08 -0500 Subject: [PATCH 1/3] updatre gitignore --- .gitignore | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 74169ae..20e180c 100644 --- a/.gitignore +++ b/.gitignore @@ -30,4 +30,9 @@ Thumbs.db htmlcov/ # Logs -*.log \ No newline at end of file +*.log + +# Agent tooling +docs/superpowers/ +.claude/ +CLAUDE.md From be97706a334f37ea8cd6b59239d8eaac02909afe Mon Sep 17 00:00:00 2001 From: Kumar Challa Date: Thu, 8 Oct 2026 02:09:10 -0500 Subject: [PATCH 2/3] Report real safety state to the external API and dashboard regardless of ASCOM connections The driver used to force IsSafe to False whenever no ASCOM client was connected, and that gate lived inside the shared is_safe() method, so it leaked into /api/ext/v1/status and the dashboard too. Webhook, script and microcontroller users of the external API therefore always saw is_safe:false unless NINA happened to be attached (issue #117), and the dashboard appeared frozen with no client (issue #90). MQTT had already bypassed the gate, so the three surfaces disagreed with each other. The gate was never an ASCOM requirement. The spec defines Connected per client and says IsSafe raises NotConnected when not connected; it never asks for a False value. ConformU dropped its "IsSafe must be False before connect" check on 2025-03-16 by Core Team decision. is_safe() now returns the debounced detector state. The Alpaca issafe and devicestate routes apply the gate themselves, per client (IP + ClientID), so a client that never connected still sees False exactly as before, while the external API, dashboard and MQTT all report the same real state. The dashboard tile is renamed Safety State with a tooltip describing the split. Verified with ConformU 4.5 (no issues) and a local Docker run. --- alpaca/device.py | 16 +++----- alpaca/routes/api.py | 19 ++++++++- docs/EXTERNAL_API.md | 2 +- templates/setup.html | 4 +- tests/test_issafe_gate.py | 80 ++++++++++++++++++++++++++++++++++++ tests/verify_multi_client.py | 32 ++++++++++----- 6 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 tests/test_issafe_gate.py diff --git a/alpaca/device.py b/alpaca/device.py index e1b956e..31a011a 100644 --- a/alpaca/device.py +++ b/alpaca/device.py @@ -423,11 +423,12 @@ def disconnect(self, client_ip: str = None, client_id: int = None): logger.warning(f"Attempted to disconnect unknown client: {client_ip} (ID: {client_id})") def is_safe(self) -> bool: - """Determine if conditions are safe based on latest detection""" - # Safety Fail-safe: Always return False if not connected - if not self.is_connected: - return False - + """Debounced safety state of the sky, independent of ASCOM client connections. + + The Alpaca routes apply the per-client Connected gate themselves + (see routes/api.py client_is_safe). Everything else (external REST + API, dashboard, MQTT) reports this value directly. + """ with self.detection_lock: return self._stable_safe_state @@ -478,8 +479,3 @@ def get_connected_clients_info(self) -> List[Dict[str, Any]]: "duration_seconds": (now - conn_time).total_seconds() }) return clients - - def get_device_state(self) -> list: - """Get current operational state""" - is_safe_val = self.is_safe() - return [{"Name": "IsSafe", "Value": is_safe_val}] diff --git a/alpaca/routes/api.py b/alpaca/routes/api.py index cce5875..066b491 100644 --- a/alpaca/routes/api.py +++ b/alpaca/routes/api.py @@ -53,6 +53,19 @@ def endpoint(device_number: int): )) return endpoint +def client_is_safe() -> bool: + """IsSafe as reported to the calling Alpaca client. + + Alpaca tracks Connected per client (IP + ClientID). A client that has not + connected is told False, matching the pre-connection behaviour ASCOM + clients have always seen from this driver. The detector itself keeps + running; monitor.is_safe() carries the real state for non-Alpaca consumers. + """ + client_id, _ = monitor.get_client_params() + if not monitor.is_client_connected(request.remote_addr, client_id): + return False + return monitor.is_safe() + @api_bp.route('/v1/safetymonitor//issafe', methods=['GET']) def get_issafe(device_number: int): """Get safety status""" @@ -61,7 +74,7 @@ def get_issafe(device_number: int): return error_response _, client_tx_id = monitor.get_client_params() return jsonify(monitor.create_response( - value=monitor.is_safe(), + value=client_is_safe(), client_transaction_id=client_tx_id )) @@ -186,7 +199,9 @@ def get_description(device_number: int): @api_bp.route('/v1/safetymonitor//devicestate', methods=['GET']) def get_devicestate(device_number: int): - return create_simple_get_endpoint(monitor.get_device_state)(device_number) + return create_simple_get_endpoint( + lambda: [{"Name": "IsSafe", "Value": client_is_safe()}] + )(device_number) @api_bp.route('/v1/safetymonitor//driverinfo', methods=['GET']) def get_driverinfo(device_number: int): diff --git a/docs/EXTERNAL_API.md b/docs/EXTERNAL_API.md index a2c3635..00bc22a 100644 --- a/docs/EXTERNAL_API.md +++ b/docs/EXTERNAL_API.md @@ -114,7 +114,7 @@ Returns current safety status and latest detection results. | Field | Type | Description | |-------|------|-------------| -| `is_safe` | boolean | Current safety state | +| `is_safe` | boolean | Debounced detector state. Does not depend on any ASCOM client being connected. | | `safety_status` | string | "Safe" or "Unsafe" | | `detection.class_name` | string | Detected condition (Clear, Wisps, Mostly Cloudy, etc.) | | `detection.confidence_score` | number | ML model confidence (0.0 - 1.0) | diff --git a/templates/setup.html b/templates/setup.html index 454c518..c6e2521 100644 --- a/templates/setup.html +++ b/templates/setup.html @@ -780,8 +780,8 @@

☁️ SimpleCloudDetect

- ASCOM Status - ⓘ + Safety State + ⓘ
{{ ascom_safe_status }}
diff --git a/tests/test_issafe_gate.py b/tests/test_issafe_gate.py new file mode 100644 index 0000000..3b38432 --- /dev/null +++ b/tests/test_issafe_gate.py @@ -0,0 +1,80 @@ +"""IsSafe is gated per Alpaca client; the external API reports raw detector state.""" +import threading +from types import SimpleNamespace + +import pytest +from flask import Flask, request + +from alpaca.routes.api import api_bp, init_api +from alpaca.routes.external_api import external_api_bp, init_external_api + + +class FakeMonitor: + def __init__(self, safe=True): + self.alpaca_config = SimpleNamespace(device_number=0) + self._safe = safe + self.connected = set() + self.detection_lock = threading.Lock() + self.latest_detection = None + + def get_client_params(self): + return int(request.args.get("ClientID", 0)), int(request.args.get("ClientTransactionID", 0)) + + def register_heartbeat(self, ip, client_id): + pass + + def create_response(self, value=None, error_number=0, error_message="", client_transaction_id=0): + return { + "Value": value, + "ErrorNumber": error_number, + "ErrorMessage": error_message, + "ClientTransactionID": client_transaction_id, + } + + def is_client_connected(self, ip, client_id): + return (ip, client_id) in self.connected + + def is_safe(self): + return self._safe + + +@pytest.fixture +def harness(): + monitor = FakeMonitor(safe=True) + app = Flask(__name__) + init_api(monitor) + init_external_api(monitor) + app.register_blueprint(api_bp, url_prefix="/api") + app.register_blueprint(external_api_bp) + return app.test_client(), monitor + + +def test_issafe_false_for_unconnected_client(harness): + client, _ = harness + resp = client.get("/api/v1/safetymonitor/0/issafe?ClientID=5") + assert resp.get_json()["Value"] is False + + +def test_issafe_true_for_connected_client_only(harness): + client, monitor = harness + monitor.connected.add(("127.0.0.1", 5)) + assert client.get("/api/v1/safetymonitor/0/issafe?ClientID=5").get_json()["Value"] is True + assert client.get("/api/v1/safetymonitor/0/issafe?ClientID=6").get_json()["Value"] is False + + +def test_devicestate_uses_same_gate(harness): + client, monitor = harness + assert client.get("/api/v1/safetymonitor/0/devicestate?ClientID=5").get_json()["Value"] == [ + {"Name": "IsSafe", "Value": False} + ] + monitor.connected.add(("127.0.0.1", 5)) + assert client.get("/api/v1/safetymonitor/0/devicestate?ClientID=5").get_json()["Value"] == [ + {"Name": "IsSafe", "Value": True} + ] + + +def test_external_status_ignores_client_connections(harness): + client, _ = harness + data = client.get("/api/ext/v1/status").get_json() + assert data["is_safe"] is True + assert data["safety_status"] == "Safe" diff --git a/tests/verify_multi_client.py b/tests/verify_multi_client.py index d8b9917..2f4e7c0 100644 --- a/tests/verify_multi_client.py +++ b/tests/verify_multi_client.py @@ -20,9 +20,9 @@ def check_connected(): log(f"Error checking connected status: {e}") return None -def check_issafe(): +def check_issafe(client_id=0): try: - resp = requests.get(f"{BASE_URL}/issafe") + resp = requests.get(f"{BASE_URL}/issafe", params={'ClientID': client_id}) resp.raise_for_status() return resp.json()["Value"] except Exception as e: @@ -139,19 +139,31 @@ def run_tests(): log("Scenario B Passed") - # Scenario C: Fail-safe - log("--- Scenario C: Fail-safe ---") + # Scenario C: Per-client IsSafe gate + log("--- Scenario C: Per-client IsSafe gate ---") # Ensure fully disconnected if check_connected(): log("FAIL: System should be disconnected") sys.exit(1) - - is_safe = check_issafe() - log(f"Is Safe (while disconnected)? {is_safe}") - if is_safe: - log("FAIL: Should report Unsafe (False) when disconnected") + + log("Connecting Client A (ID 1)...") + connect(1) + + is_safe_other = check_issafe(client_id=99) + log(f"Is Safe for unconnected Client 99? {is_safe_other}") + if is_safe_other: + log("FAIL: A client that never connected must see IsSafe=False") sys.exit(1) - + + is_safe_a = check_issafe(client_id=1) + log(f"Is Safe for connected Client 1? {is_safe_a}") + if is_safe_a is None: + log("FAIL: Connected client could not read IsSafe") + sys.exit(1) + + log("Disconnecting Client A...") + disconnect(1) + log("Scenario C Passed") log("ALL TESTS PASSED") From 64fb4f104b2e437163c4445e985c042aa80c3866 Mon Sep 17 00:00:00 2001 From: Kumar Challa Date: Thu, 8 Oct 2026 02:13:52 -0500 Subject: [PATCH 3/3] Restore Home Assistant availability after an MQTT reconnect When the broker session dropped, the broker published the retained "offline" Last Will for the device. paho's loop_start() then reconnected silently, but discovery and availability were only ever published once at startup, so nothing set the device back to "online". Home Assistant showed the cloud detector as permanently unavailable even though its state topics kept updating every detection cycle. Both MQTT setup paths now install an on_connect handler that republishes the discovery configs, which includes the retained "online" availability message, on every successful connect rather than only the first. The handler accepts the paho 1.x and 2.x callback shapes and ignores failed connects. The ha_discovery attribute is initialised before the client is created because the callback fires from the network thread and may run before discovery has been set up; it simply no-ops in that case and the normal startup publish covers it. --- .gitignore | 3 ++ alpaca/device.py | 26 +++++++-- detect.py | 23 ++++++-- tests/test_mqtt_reconnect.py | 102 +++++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 7 deletions(-) create mode 100644 tests/test_mqtt_reconnect.py diff --git a/.gitignore b/.gitignore index 20e180c..8e4eec0 100644 --- a/.gitignore +++ b/.gitignore @@ -34,5 +34,8 @@ htmlcov/ # Agent tooling docs/superpowers/ + +# Local reference docs (fetched, not tracked) +docs/reference/ .claude/ CLAUDE.md diff --git a/alpaca/device.py b/alpaca/device.py index 31a011a..6a72751 100644 --- a/alpaca/device.py +++ b/alpaca/device.py @@ -68,10 +68,11 @@ def __init__(self, alpaca_config: AlpacaConfig, detect_config: DetectConfig): self.detection_thread: Optional[threading.Thread] = None self.stop_detection = threading.Event() - # Setup MQTT client first - self.mqtt_client = self._setup_mqtt() + # Setup MQTT client first. ha_discovery must exist before _setup_mqtt() + # because the on_connect callback fires from the network thread and reads it. self.ha_discovery = None - + self.mqtt_client = self._setup_mqtt() + # Pre-load cloud detector at startup logger.info("Pre-loading ML model...") self.cloud_detector = CloudDetector(self.detect_config, mqtt_client=self.mqtt_client) @@ -216,7 +217,24 @@ def _setup_mqtt(self): if self.detect_config.mqtt_discovery_mode == 'homeassistant': availability_topic = f"{self.detect_config.mqtt_discovery_prefix}/sensor/clouddetect_{self.detect_config.device_id}/availability" client.will_set(availability_topic, "offline", retain=True) - + + # Republish discovery and availability on every connect, not just the + # first. loop_start() reconnects silently after a dropped session, and + # the broker publishes the retained "offline" will when that happens. + # Without this the device stays unavailable in Home Assistant forever + # even though state topics keep updating. + # Signature accepts paho 1.x (client, userdata, flags, rc) and + # 2.x (..., reason_code, properties). + def on_connect(client, userdata, flags, rc, properties=None): + if rc != 0: + logger.error(f"MQTT connect failed with code {rc}") + return + if self.ha_discovery: + self.ha_discovery.publish_discovery_configs() + logger.info("Republished HA discovery and availability after connect") + + client.on_connect = on_connect + try: client.connect(self.detect_config.broker, self.detect_config.port) client.loop_start() diff --git a/detect.py b/detect.py index 39e68c0..8d7649b 100755 --- a/detect.py +++ b/detect.py @@ -190,9 +190,11 @@ def __init__(self, config: Config, mqtt_client=None): self.model = self._load_model() self.class_names = self._load_class_names() self.session = requests.Session() # Reuse HTTP connections - self.mqtt_client = mqtt_client if mqtt_client is not None else self._setup_mqtt() + # ha_discovery must exist before _setup_mqtt() because the on_connect + # callback fires from the network thread and reads it. self.ha_discovery = None - + self.mqtt_client = mqtt_client if mqtt_client is not None else self._setup_mqtt() + # Initialize HA discovery if enabled if self.mqtt_client and self.config.mqtt_discovery_mode == 'homeassistant': self.ha_discovery = HADiscoveryManager(self.config, self.mqtt_client) @@ -230,7 +232,22 @@ def _setup_mqtt(self): if self.config.mqtt_discovery_mode == 'homeassistant': availability_topic = f"{self.config.mqtt_discovery_prefix}/sensor/clouddetect_{self.config.device_id}/availability" client.will_set(availability_topic, "offline", retain=True) - + + # Republish discovery and availability on every connect, not just the + # first. loop_start() reconnects silently after a dropped session, and + # the broker publishes the retained "offline" will when that happens. + # Signature accepts paho 1.x (client, userdata, flags, rc) and + # 2.x (..., reason_code, properties). + def on_connect(client, userdata, flags, rc, properties=None): + if rc != 0: + logger.error(f"MQTT connect failed with code {rc}") + return + if self.ha_discovery: + self.ha_discovery.publish_discovery_configs() + logger.info("Republished HA discovery and availability after connect") + + client.on_connect = on_connect + try: client.connect(self.config.broker, self.config.port) # Start background network loop to handle keepalive pings and reconnections diff --git a/tests/test_mqtt_reconnect.py b/tests/test_mqtt_reconnect.py new file mode 100644 index 0000000..426f197 --- /dev/null +++ b/tests/test_mqtt_reconnect.py @@ -0,0 +1,102 @@ +""" +Regression test: MQTT availability must be restored after a reconnect. + +The broker publishes the retained "offline" Last Will when a session drops. +paho's loop_start() reconnects silently, so without an on_connect handler that +republishes discovery, the device stays permanently unavailable in Home +Assistant while its state topics keep updating. +""" +import sys +import types +import unittest +from unittest import mock + +# detect.py imports TensorFlow at module level. Stub it so this test runs +# without the ML stack installed. Everything else is a real dependency. +if 'tensorflow' not in sys.modules: + keras_models = types.ModuleType('tensorflow.keras.models') + keras_models.load_model = lambda *a, **k: None + keras = types.ModuleType('tensorflow.keras') + keras.models = keras_models + tf = types.ModuleType('tensorflow') + tf.keras = keras + sys.modules['tensorflow'] = tf + sys.modules['tensorflow.keras'] = keras + sys.modules['tensorflow.keras.models'] = keras_models + sys.modules['keras'] = keras + sys.modules['keras.models'] = keras_models + +sys.path.insert(0, '.') + +from alpaca.device import AlpacaSafetyMonitor # noqa: E402 + + +class FakeConfig: + broker = '192.168.1.250' + port = 1883 + mqtt_username = None + mqtt_password = None + mqtt_discovery_mode = 'homeassistant' + mqtt_discovery_prefix = 'homeassistant' + device_id = 'testdevice' + + +class FakeMonitor: + """Minimal stand-in so _setup_mqtt can run without loading the ML model.""" + + def __init__(self): + self.detect_config = FakeConfig() + self.ha_discovery = None + + +class MqttReconnectTest(unittest.TestCase): + + def _setup(self): + monitor = FakeMonitor() + fake_client = mock.MagicMock() + with mock.patch('alpaca.device.mqtt.Client', return_value=fake_client): + AlpacaSafetyMonitor._setup_mqtt(monitor) + return monitor, fake_client + + def test_last_will_is_offline_and_retained(self): + _, client = self._setup() + client.will_set.assert_called_once_with( + 'homeassistant/sensor/clouddetect_testdevice/availability', + 'offline', + retain=True, + ) + + def test_reconnect_republishes_discovery(self): + monitor, client = self._setup() + monitor.ha_discovery = mock.MagicMock() + + client.on_connect(client, None, {}, 0) + + monitor.ha_discovery.publish_discovery_configs.assert_called_once() + + def test_failed_connect_does_not_republish(self): + monitor, client = self._setup() + monitor.ha_discovery = mock.MagicMock() + + client.on_connect(client, None, {}, 5) # 5 = not authorised + + monitor.ha_discovery.publish_discovery_configs.assert_not_called() + + def test_first_connect_before_discovery_exists_does_not_raise(self): + # on_connect fires from the network thread during _setup_mqtt, before + # ha_discovery is assigned. It must not raise there. + _, client = self._setup() + client.on_connect(client, None, {}, 0) + + def test_paho2_signature_accepted(self): + # paho 2.x calls on_connect with an extra properties argument. + monitor, client = self._setup() + monitor.ha_discovery = mock.MagicMock() + + client.on_connect(client, None, {}, 0, None) + + monitor.ha_discovery.publish_discovery_configs.assert_called_once() + + +if __name__ == '__main__': + unittest.main()