From 0d34cae441625c00cd5c4cd2c26b6d0a95c7a489 Mon Sep 17 00:00:00 2001 From: Matt Hills Date: Mon, 19 Jan 2026 17:18:57 -0500 Subject: [PATCH] v1.3.1: Fix stale devices in admin UI and firmware device type persistence Admin UI fixes: - Track deleted devices to prevent MQTT re-registration from stale messages - Broadcast device_deleted event via WebSocket to sync all clients - Add removeDeviceFromUI() to clean up device cards and pending timers - Improve delete error handling with user feedback Firmware fixes (v1.0.7): - Fix device type strings to match API expectations (heart_rate vs Heart Rate) - Clean up BLE services when switching device types (prevents service accumulation) - Call stop() before setupHeartRate()/setupTreadmill() to reset NimBLE state - Track service pointers for proper cleanup with pServer->removeService() Co-Authored-By: Claude Opus 4.5 --- firmware/esp32_ble_sim/include/config.h | 2 +- firmware/esp32_ble_sim/src/device_state.cpp | 6 +-- .../src/services/ble_service.cpp | 54 +++++++++++++------ src/api/device_registry.py | 42 ++++++++++++--- src/api/main.py | 12 +++-- src/api/routes.py | 17 ++++-- src/web/static/index.html | 37 ++++++++++++- 7 files changed, 136 insertions(+), 34 deletions(-) diff --git a/firmware/esp32_ble_sim/include/config.h b/firmware/esp32_ble_sim/include/config.h index f9af1d0..46b02fe 100644 --- a/firmware/esp32_ble_sim/include/config.h +++ b/firmware/esp32_ble_sim/include/config.h @@ -4,7 +4,7 @@ // ============================================ // Firmware Version // ============================================ -#define FIRMWARE_VERSION "1.0.6" +#define FIRMWARE_VERSION "1.0.7" // ============================================ // AP Mode Configuration diff --git a/firmware/esp32_ble_sim/src/device_state.cpp b/firmware/esp32_ble_sim/src/device_state.cpp index bce0077..f067795 100644 --- a/firmware/esp32_ble_sim/src/device_state.cpp +++ b/firmware/esp32_ble_sim/src/device_state.cpp @@ -7,9 +7,9 @@ DeviceState& DeviceState::getInstance() { const char* DeviceState::getDeviceTypeString() const { switch (deviceType) { - case DeviceType::HEART_RATE: return "Heart Rate"; - case DeviceType::TREADMILL: return "Treadmill"; - default: return "Not configured"; + case DeviceType::HEART_RATE: return "heart_rate"; + case DeviceType::TREADMILL: return "treadmill"; + default: return ""; } } diff --git a/firmware/esp32_ble_sim/src/services/ble_service.cpp b/firmware/esp32_ble_sim/src/services/ble_service.cpp index 04ada84..707492a 100644 --- a/firmware/esp32_ble_sim/src/services/ble_service.cpp +++ b/firmware/esp32_ble_sim/src/services/ble_service.cpp @@ -12,6 +12,11 @@ static NimBLECharacteristic* pHeartRateMeasurement = nullptr; static NimBLECharacteristic* pBatteryLevel = nullptr; static NimBLECharacteristic* pTreadmillData = nullptr; +// Track created services for cleanup +static NimBLEService* pHeartRateService = nullptr; +static NimBLEService* pBatteryService = nullptr; +static NimBLEService* pFitnessMachineService = nullptr; + static bool bleInitialized = false; // Forward declare for callback @@ -97,13 +102,32 @@ void BleService::initBLE() { void BleService::stop() { if (pAdvertising) { pAdvertising->stop(); + // Clear all service UUIDs from advertising + pAdvertising->reset(); } + // Remove existing services from the server + if (pServer) { + if (pHeartRateService) { + pServer->removeService(pHeartRateService); + pHeartRateService = nullptr; + } + if (pBatteryService) { + pServer->removeService(pBatteryService); + pBatteryService = nullptr; + } + if (pFitnessMachineService) { + pServer->removeService(pFitnessMachineService); + pFitnessMachineService = nullptr; + } + } + + // Clear characteristic pointers pHeartRateMeasurement = nullptr; pBatteryLevel = nullptr; pTreadmillData = nullptr; - Serial.println("BLE stopped"); + Serial.println("BLE stopped and services cleaned up"); } // ============================================ @@ -112,29 +136,28 @@ void BleService::stop() { void BleService::setupHeartRate() { Serial.println("Setting up Heart Rate Service..."); - if (pAdvertising) { - pAdvertising->stop(); - } + // Clean up any existing services first + stop(); // Create Heart Rate Service - NimBLEService* pHRService = pServer->createService(HEART_RATE_SERVICE_UUID); + pHeartRateService = pServer->createService(HEART_RATE_SERVICE_UUID); - pHeartRateMeasurement = pHRService->createCharacteristic( + pHeartRateMeasurement = pHeartRateService->createCharacteristic( HEART_RATE_MEASUREMENT_UUID, NIMBLE_PROPERTY::NOTIFY ); - NimBLECharacteristic* pBodySensorLocation = pHRService->createCharacteristic( + NimBLECharacteristic* pBodySensorLocation = pHeartRateService->createCharacteristic( BODY_SENSOR_LOCATION_UUID, NIMBLE_PROPERTY::READ ); uint8_t sensorLocation = 1; // Chest pBodySensorLocation->setValue(&sensorLocation, 1); - pHRService->start(); + pHeartRateService->start(); // Create Battery Service - NimBLEService* pBatteryService = pServer->createService(BATTERY_SERVICE_UUID); + pBatteryService = pServer->createService(BATTERY_SERVICE_UUID); pBatteryLevel = pBatteryService->createCharacteristic( BATTERY_LEVEL_UUID, @@ -164,13 +187,12 @@ void BleService::setupHeartRate() { void BleService::setupTreadmill() { Serial.println("Setting up Fitness Machine Service (Treadmill)..."); - if (pAdvertising) { - pAdvertising->stop(); - } + // Clean up any existing services first + stop(); - NimBLEService* pService = pServer->createService(FITNESS_MACHINE_SERVICE_UUID); + pFitnessMachineService = pServer->createService(FITNESS_MACHINE_SERVICE_UUID); - NimBLECharacteristic* pFeature = pService->createCharacteristic( + NimBLECharacteristic* pFeature = pFitnessMachineService->createCharacteristic( FITNESS_MACHINE_FEATURE_UUID, NIMBLE_PROPERTY::READ ); @@ -181,12 +203,12 @@ void BleService::setupTreadmill() { }; pFeature->setValue(featureData, 8); - pTreadmillData = pService->createCharacteristic( + pTreadmillData = pFitnessMachineService->createCharacteristic( TREADMILL_DATA_UUID, NIMBLE_PROPERTY::NOTIFY ); - pService->start(); + pFitnessMachineService->start(); pAdvertising->addServiceUUID(FITNESS_MACHINE_SERVICE_UUID); pAdvertising->setScanResponse(true); diff --git a/src/api/device_registry.py b/src/api/device_registry.py index f5fc593..6fc2a59 100644 --- a/src/api/device_registry.py +++ b/src/api/device_registry.py @@ -9,9 +9,18 @@ class DeviceRegistry: def __init__(self): self._devices: dict[str, dict[str, Any]] = {} + # Track recently deleted devices to prevent MQTT re-registration + self._deleted_devices: set[str] = set() + + def register_device(self, device_id: str, info: dict | None = None) -> bool: + """Register a new device or update existing. + + Returns False if device was recently deleted (won't be registered). + """ + # Skip recently deleted devices - prevents stale MQTT messages from re-registering + if device_id in self._deleted_devices: + return False - def register_device(self, device_id: str, info: dict | None = None): - """Register a new device or update existing.""" now = datetime.utcnow().isoformat() if device_id not in self._devices: @@ -30,10 +39,16 @@ class DeviceRegistry: if info: self._devices[device_id].update(info) - def update_device(self, device_id: str, updates: dict): - """Update device state.""" + return True + + def update_device(self, device_id: str, updates: dict) -> bool: + """Update device state. + + Returns False if device was recently deleted (won't be updated). + """ if device_id not in self._devices: - self.register_device(device_id) + if not self.register_device(device_id): + return False # Device was deleted, don't update for key, value in updates.items(): if key == "values" and isinstance(value, dict): @@ -45,6 +60,7 @@ class DeviceRegistry: self._devices[device_id][key] = value self._devices[device_id]["last_seen"] = datetime.utcnow().isoformat() + return True def mark_offline(self, device_id: str): """Mark a device as offline.""" @@ -60,8 +76,22 @@ class DeviceRegistry: return list(self._devices.values()) def remove_device(self, device_id: str): - """Remove a device from the registry.""" + """Remove a device from the registry and mark as deleted.""" self._devices.pop(device_id, None) + # Track as deleted to prevent MQTT re-registration + self._deleted_devices.add(device_id) + + def is_deleted(self, device_id: str) -> bool: + """Check if a device was recently deleted.""" + return device_id in self._deleted_devices + + def clear_deleted(self, device_id: str): + """Allow a deleted device to be re-registered (for manual re-add).""" + self._deleted_devices.discard(device_id) + + def clear_all_deleted(self): + """Clear all deleted device tracking (e.g., on server restart).""" + self._deleted_devices.clear() # Global registry instance diff --git a/src/api/main.py b/src/api/main.py index 849b68f..741f49f 100644 --- a/src/api/main.py +++ b/src/api/main.py @@ -5,7 +5,7 @@ from contextlib import asynccontextmanager from fastapi import FastAPI, WebSocket, WebSocketDisconnect # Version info -__version__ = "1.3.0" +__version__ = "1.3.1" from fastapi.middleware.cors import CORSMiddleware from fastapi.staticfiles import StaticFiles from fastapi.responses import FileResponse @@ -56,7 +56,10 @@ async def handle_device_status(topic: str, payload: str): device_id = parts[1] try: data = json.loads(payload) - device_registry.register_device(device_id) + # Skip if device was recently deleted (prevents stale MQTT messages from re-registering) + if not device_registry.register_device(device_id): + print(f"Ignoring status from deleted device: {device_id}") + return update_data = { "online": data.get("online", True), "type": data.get("type"), @@ -85,7 +88,10 @@ async def handle_device_values(topic: str, payload: str): device_id = parts[1] try: data = json.loads(payload) - device_registry.update_device(device_id, {"values": data}) + # Skip if device was recently deleted + if not device_registry.update_device(device_id, {"values": data}): + print(f"Ignoring values from deleted device: {device_id}") + return print(f"Device {device_id} values updated: {data}") # Broadcast to WebSocket clients await ws_manager.broadcast({ diff --git a/src/api/routes.py b/src/api/routes.py index 3b43661..44d972b 100644 --- a/src/api/routes.py +++ b/src/api/routes.py @@ -63,12 +63,23 @@ async def delete_device(device_id: str): try: await mqtt_manager.clear_retained(f"ble-sim/{device_id}/status") await mqtt_manager.clear_retained(f"ble-sim/{device_id}/values") - except Exception: - pass # MQTT might not be connected + except Exception as e: + # Log the error but continue with deletion + print(f"Warning: Failed to clear MQTT retained messages for {device_id}: {e}") - # Remove from registry + # Remove from registry (also marks as deleted to block re-registration) device_registry.remove_device(device_id) + # Broadcast deletion to all WebSocket clients so they remove the device card + try: + from .main import ws_manager + await ws_manager.broadcast({ + "type": "device_deleted", + "device_id": device_id + }) + except Exception as e: + print(f"Warning: Failed to broadcast device deletion: {e}") + return {"status": "ok", "device_id": device_id, "message": "Device removed"} diff --git a/src/web/static/index.html b/src/web/static/index.html index c284b14..5b7a5f4 100644 --- a/src/web/static/index.html +++ b/src/web/static/index.html @@ -446,12 +446,36 @@ } else if (message.type === 'device_update') { // Single device updated - update UI efficiently updateDeviceInUI(message.device_id, message.data); + } else if (message.type === 'device_deleted') { + // Device was deleted - remove from UI + removeDeviceFromUI(message.device_id); } else if (message.type === 'hr_update') { // HR value update from server-side variation updateHrDisplay(message.device_id, message.heart_rate); } } + function removeDeviceFromUI(deviceId) { + const container = document.getElementById('devices'); + const card = container.querySelector(`[data-id="${deviceId}"]`); + if (card) { + card.remove(); + console.log(`Removed deleted device card: ${deviceId}`); + } + // Clean up any pending timers for this device + if (hrTargetDebounceTimers[deviceId]) { + clearTimeout(hrTargetDebounceTimers[deviceId]); + delete hrTargetDebounceTimers[deviceId]; + } + // Clean up any pending value update timers + Object.keys(updateTimeouts).forEach(key => { + if (key.startsWith(deviceId + '-')) { + clearTimeout(updateTimeouts[key]); + delete updateTimeouts[key]; + } + }); + } + function updateHrDisplay(deviceId, hr) { const display = document.getElementById(`hr-display-${deviceId}`); if (display) { @@ -925,12 +949,21 @@ async function deleteDevice(deviceId) { if (!confirm(`Remove device ${deviceId} from the list?`)) return; try { - await fetch(`${API_BASE}/devices/${deviceId}`, { + const response = await fetch(`${API_BASE}/devices/${deviceId}`, { method: 'DELETE' }); - loadDevices(); + if (!response.ok) { + const error = await response.json().catch(() => ({})); + console.error('Failed to delete device:', error.detail || response.statusText); + alert(`Failed to delete device: ${error.detail || response.statusText}`); + return; + } + // WebSocket broadcast will handle removing the device from UI + // But also remove locally in case WebSocket is not connected + removeDeviceFromUI(deviceId); } catch (error) { console.error('Failed to delete device:', error); + alert('Failed to delete device. Check console for details.'); } }