From 8945c8e7967ea00842f1dbf865bc21d71a321ee9 Mon Sep 17 00:00:00 2001 From: Mickael Farina Date: Mon, 8 Jun 2026 15:41:10 +0200 Subject: [PATCH] fix(philips_hue): retry on transient bridge blips, not just IP changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-heal (#183) only retried when rediscovery returned a *different* IP, so a momentary ConnectionError while the bridge stayed at the same address still surfaced as 'Could not reach' (observed live). Now: a quick retry absorbs brief blips, and the rediscovery retry fires whenever the bridge is verified reachable — same IP or new. Adds a regression test (transient at unchanged IP must not error). Co-Authored-By: Claude Opus 4.8 --- skills/.manifest.json | 2 +- skills/philips_hue.py | 21 ++++++++++++++------- tests/test_philips_hue.py | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/skills/.manifest.json b/skills/.manifest.json index 8255c48..a76ba8d 100644 --- a/skills/.manifest.json +++ b/skills/.manifest.json @@ -61,7 +61,7 @@ "notes.py": "7d50d1544ea955f59917a1f0e7902d115e9dbccd3188b641057a55b6a5b2803a", "notification_reader.py": "681208ae4253dfe549512cce4c722c76ca85f2bbbdb8737e37c026ec9444c972", "password_generator.py": "f11a917299e14cbd2560111da0bb748cd08792cf715cc4098c64eb62da8c54e3", - "philips_hue.py": "1c311dfc1426e6a73975b76ed0f002c1715364d067a85b900c356b5561e90c88", + "philips_hue.py": "ac2b8c1418a04a14025032ce6f5b820ab4063589b4473e63f88c9c24e6a21e13", "pilot.py": "ded952504f8d44c3c6ea5b1da1ef2f09f0bbf1b771e2ad9dd90db6cd1701c3fb", "plugin_approve.py": "c93861353be2a9ebe08df212ca167bea646962dbeafe704b1864cd78a8cf57cc", "pm2_control.py": "53d34a9ddb7689b86f192694d6ccc18b154538626cbc72992445d0b74f2e5662", diff --git a/skills/philips_hue.py b/skills/philips_hue.py index 5d0ee83..60875d4 100644 --- a/skills/philips_hue.py +++ b/skills/philips_hue.py @@ -276,16 +276,16 @@ def _run_once(task, ip, user): return f"Set {target_name} to {description}." -def _try_rediscover(old_ip): - """The bridge IP may have changed (DHCP). Re-find it by id via the standard discovery - ladder (mDNS -> cloud -> scan) and persist the new IP. Returns a new IP different from - old_ip, or None. Never raises.""" +def _try_rediscover(): + """Re-find the bridge via the standard ladder (mDNS -> cloud -> scan) and persist its + current IP. Returns the verified-reachable IP — whether or not it changed — or None. + A confirmed-reachable bridge is worth a retry even at the SAME IP: that absorbs a + transient blip, not just a DHCP move. Never raises.""" try: import codec_hue_discovery - new_ip = codec_hue_discovery.rediscover_and_update_config() + return codec_hue_discovery.rediscover_and_update_config() except Exception: return None - return new_ip if new_ip and new_ip != old_ip else None def run(task, app="", ctx=""): @@ -302,7 +302,14 @@ def run(task, app="", ctx=""): try: return _run_once(task, ip, user) except requests.ConnectionError: - new_ip = _try_rediscover(ip) + # 1) quick retry at the same IP — absorbs a brief network/bridge blip. + try: + return _run_once(task, ip, user) + except requests.ConnectionError: + pass + # 2) still failing — the IP may have changed. Re-discover (which verifies the bridge + # is actually reachable) and retry at its current address, new or unchanged. + new_ip = _try_rediscover() if new_ip: try: return _run_once(task, new_ip, user) diff --git a/tests/test_philips_hue.py b/tests/test_philips_hue.py index 66cfe6f..fe5f2de 100644 --- a/tests/test_philips_hue.py +++ b/tests/test_philips_hue.py @@ -71,6 +71,26 @@ def fake_apply(ip, user, ttype, tid, state): assert philips_hue.run("lights off") == "Set all lights to off." +def test_run_absorbs_transient_connection_error_same_ip(monkeypatch): + # A brief blip at the SAME ip must NOT surface as an error — retry absorbs it. + IP = "192.168.1.81" + monkeypatch.setattr(philips_hue, "_load_config", lambda: (IP, "user")) + monkeypatch.setattr(philips_hue, "_resolve_target", lambda tl, ip, u: ("all", "0", "all lights")) + calls = {"n": 0} + + def flaky_apply(ip, user, ttype, tid, state): + calls["n"] += 1 + if calls["n"] == 1: + raise requests.ConnectionError("transient blip") + return [{"success": {}}] + + monkeypatch.setattr(philips_hue, "_apply_state", flaky_apply) + monkeypatch.setattr(codec_hue_discovery, "rediscover_and_update_config", lambda *a, **k: IP) + + assert philips_hue.run("lights off") == "Set all lights to off." + assert calls["n"] >= 2 # retried rather than giving up + + def test_run_friendly_error_when_rediscovery_fails(monkeypatch): monkeypatch.setattr(philips_hue, "_load_config", lambda: ("192.168.1.81", "user")) monkeypatch.setattr(philips_hue, "_resolve_target", lambda tl, ip, u: ("all", "0", "all lights"))