fix(ota): settle stale firmware update states

talkingq-url/banban/service/device.py

@@ -308,7 +308,7 @@ class DeviceService(DatabaseServiceBase):
         latest_parts.extend([0] * (max_len - len(latest_parts)))
         return latest_parts > current_parts
 
-    def _firmware_sent_timed_out(self, updated_at: Any) -> bool:
+    def _firmware_update_timed_out(self, updated_at: Any) -> bool:
         timeout_seconds = max(0, int(settings.ota_sent_timeout_seconds or 0))
         if timeout_seconds <= 0 or not updated_at:
             return False
@@ -331,6 +331,16 @@ class DeviceService(DatabaseServiceBase):
         now = datetime.now(timezone.utc) if updated_at.tzinfo else datetime.now()
         return (now - updated_at).total_seconds() >= timeout_seconds
 
+    def _firmware_update_reached_target(self, current_version: str | None, target_version: str | None) -> bool:
+        current = (current_version or "").strip().lower()
+        target = (target_version or "").strip().lower()
+        placeholders = {"", "unknown", "updating", "0.0.0", "--"}
+        if current in placeholders or target in placeholders:
+            return False
+
+        # Treat an equal or newer reported version as completion of this OTA attempt.
+        return not self._compare_versions(current, target)
+
     async def get_firmware_status(
         self,
         *,
@@ -348,9 +358,29 @@ class DeviceService(DatabaseServiceBase):
         update_record = await device_firmware_update_manager.get_firmware_update_dict(device_id)
         update_status = update_record.get("update_status") if update_record else "idle"
         progress = update_record.get("progress") if update_record else 0.0
-        target_version = update_record.get("firmware_version") if update_record else None
+        target_version = (
+            (update_record.get("target_version") or update_record.get("firmware_version"))
+            if update_record else None
+        )
         updated_at = update_record.get("updated_at") if update_record else None
-        if str(update_status or "").strip().lower() == "sent" and self._firmware_sent_timed_out(updated_at):
+        normalized_update_status = str(update_status or "").strip().lower()
+        active_update_statuses = {"sent", "accepted", "updating"}
+        if normalized_update_status in active_update_statuses and self._firmware_update_reached_target(
+            current_version,
+            target_version,
+        ):
+            update_status = "success"
+            progress = 100.0
+            await device_firmware_update_manager.update_firmware_update(
+                device_id=device_id,
+                firmware_version=target_version or latest_version or current_version or "unknown",
+                update_status="success",
+                progress=100.0,
+            )
+            update_record = await device_firmware_update_manager.get_firmware_update_dict(device_id)
+            if update_record:
+                updated_at = update_record.get("updated_at") or updated_at
+        elif normalized_update_status in active_update_statuses and self._firmware_update_timed_out(updated_at):
             update_status = "failed"
             progress = 0.0
             await device_firmware_update_manager.update_firmware_update(

talkingq-url/tests/test_ota_channel_selection.py

@@ -1,3 +1,5 @@
+from datetime import datetime, timedelta
+
 import pytest
 from fastapi import HTTPException
 
@@ -78,6 +80,143 @@ async def test_firmware_status_unknown_channel_cannot_update(monkeypatch):
     assert result["can_update"] is False
 
 
+@pytest.mark.asyncio
+async def test_firmware_status_settles_active_update_when_target_is_installed(monkeypatch):
+    service = DeviceService()
+    calls = []
+    update_record = {
+        "device_id": "TalkingQ_ZHWK00003111",
+        "firmware_version": "1.8.3",
+        "target_version": "1.8.3",
+        "firmware_url": "https://example.com/gps-1.8.3.bin",
+        "firmware_id": 12,
+        "update_status": "accepted",
+        "progress": 0.0,
+        "updated_at": datetime.now(),
+    }
+
+    async def fake_get_device_status(**kwargs):
+        del kwargs
+        return {
+            "device_id": "TalkingQ_ZHWK00003111",
+            "version": "1.8.3",
+            "device_type": 2,
+            "ota_channel": "gps",
+        }
+
+    async def fake_select_latest_for_device(status_row):
+        del status_row
+        return OtaFirmwareSelection(
+            channel_key="gps",
+            version="1.8.4",
+            firmware_url="https://example.com/gps-1.8.4.bin",
+            firmware_id=13,
+        )
+
+    async def fake_get_firmware_update_dict(device_id):
+        assert device_id == "TalkingQ_ZHWK00003111"
+        return update_record
+
+    async def fake_update_firmware_update(**kwargs):
+        calls.append(kwargs)
+        return True
+
+    monkeypatch.setattr(service, "get_device_status", fake_get_device_status)
+    monkeypatch.setattr("banban.service.device.ota_firmware_service.select_latest_for_device", fake_select_latest_for_device)
+    monkeypatch.setattr("banban.service.device.device_firmware_update_manager.get_firmware_update_dict", fake_get_firmware_update_dict)
+    monkeypatch.setattr("banban.service.device.device_firmware_update_manager.update_firmware_update", fake_update_firmware_update)
+
+    result = await service.get_firmware_status(device_id="TalkingQ_ZHWK00003111", user_id=1)
+
+    assert result["update_status"] == "success"
+    assert result["progress"] == 100.0
+    assert result["update_available"] is True
+    assert calls == [{
+        "device_id": "TalkingQ_ZHWK00003111",
+        "firmware_version": "1.8.3",
+        "update_status": "success",
+        "progress": 100.0,
+    }]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("active_status", ["sent", "accepted", "updating"])
+async def test_firmware_status_times_out_stale_active_update(monkeypatch, active_status):
+    service = DeviceService()
+    calls = []
+    update_record = {
+        "device_id": "TalkingQ_ZHWK00003111",
+        "firmware_version": "1.8.3",
+        "target_version": "1.8.3",
+        "firmware_url": "https://example.com/gps-1.8.3.bin",
+        "firmware_id": 12,
+        "update_status": active_status,
+        "progress": 42.0,
+        "updated_at": datetime.now() - timedelta(seconds=301),
+    }
+
+    async def fake_get_device_status(**kwargs):
+        del kwargs
+        return {
+            "device_id": "TalkingQ_ZHWK00003111",
+            "version": "1.8.2",
+            "device_type": 2,
+            "ota_channel": "gps",
+        }
+
+    async def fake_select_latest_for_device(status_row):
+        del status_row
+        return OtaFirmwareSelection(
+            channel_key="gps",
+            version="1.8.4",
+            firmware_url="https://example.com/gps-1.8.4.bin",
+            firmware_id=13,
+        )
+
+    async def fake_get_firmware_update_dict(device_id):
+        assert device_id == "TalkingQ_ZHWK00003111"
+        return update_record
+
+    async def fake_update_firmware_update(**kwargs):
+        calls.append(kwargs)
+        return True
+
+    monkeypatch.setattr(service, "get_device_status", fake_get_device_status)
+    monkeypatch.setattr("banban.service.device.ota_firmware_service.select_latest_for_device", fake_select_latest_for_device)
+    monkeypatch.setattr("banban.service.device.device_firmware_update_manager.get_firmware_update_dict", fake_get_firmware_update_dict)
+    monkeypatch.setattr("banban.service.device.device_firmware_update_manager.update_firmware_update", fake_update_firmware_update)
+
+    result = await service.get_firmware_status(device_id="TalkingQ_ZHWK00003111", user_id=1)
+
+    assert result["update_status"] == "failed"
+    assert result["progress"] == 0.0
+    assert result["can_update"] is True
+    assert calls == [{
+        "device_id": "TalkingQ_ZHWK00003111",
+        "firmware_version": "1.8.3",
+        "update_status": "failed",
+        "progress": 0.0,
+    }]
+
+
+def test_firmware_update_not_complete_until_target_version_is_reported():
+    service = DeviceService()
+
+    assert not service._firmware_update_reached_target("1.8.2", "1.8.3")
+    assert not service._firmware_update_reached_target("unknown", "1.8.3")
+    assert not service._firmware_update_reached_target("1.8.4", "unknown")
+    assert service._firmware_update_reached_target("1.8.3", "1.8.3")
+    assert service._firmware_update_reached_target("1.8.4", "1.8.3")
+
+
+def test_firmware_update_timeout_does_not_fail_recent_progress(monkeypatch):
+    service = DeviceService()
+    monkeypatch.setattr("banban.service.device.settings.ota_sent_timeout_seconds", 300)
+
+    assert not service._firmware_update_timed_out(datetime.now())
+    assert service._firmware_update_timed_out(datetime.now() - timedelta(seconds=301))
+
+
 @pytest.mark.asyncio
 async def test_start_firmware_update_records_channel_and_keeps_mqtt_payload(monkeypatch):
     service = DeviceService()