From 8ee68e69e8d96457ede081cde000382d1e9be286 Mon Sep 17 00:00:00 2001 From: Ingo Kodba Date: Thu, 6 Aug 2026 07:57:04 +0200 Subject: [PATCH 1/2] Bound the DFU wait for the sleep wake lock WritePacketHandler spins until SystemTask disables sleeping, with no upper bound. This runs on the BLE host task, so if the wake lock never arrives the host task never returns: GATT operations stop being served, disconnect events are never processed, and the watch cannot advertise again until it is rebooted. A failed update then looks like a watch whose Bluetooth has died. Cap the wait at 1s and continue without the lock. Racing the sleep timer is a far better failure mode than a radio that needs a reboot to recover. --- src/components/ble/DfuService.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/components/ble/DfuService.cpp b/src/components/ble/DfuService.cpp index ad9c99e9e0..f49bada553 100644 --- a/src/components/ble/DfuService.cpp +++ b/src/components/ble/DfuService.cpp @@ -138,11 +138,21 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf* om) { bootloaderSize, applicationSize); - // Wait until SystemTask has disabled sleeping + // Wait until SystemTask has disabled sleeping. // This isn't quite correct, as we don't actually know - // if BleFirmwareUpdateStarted has been received yet - while (!systemTask.IsSleepDisabled()) { + // if BleFirmwareUpdateStarted has been received yet. + // Bounded on purpose: this runs on the BLE host task, so waiting here forever stalls + // every GATT operation and leaves disconnect events unprocessed, and the watch cannot + // advertise again until it is rebooted. If the wake lock never arrives, continue + // without it - a DFU that races the sleep timer beats a radio wedged until reboot. + constexpr uint8_t sleepLockWaitTicks = 200; // 200 * 5ms = 1s + uint8_t waited = 0; + while (!systemTask.IsSleepDisabled() && waited < sleepLockWaitTicks) { vTaskDelay(pdMS_TO_TICKS(5)); + waited++; + } + if (!systemTask.IsSleepDisabled()) { + NRF_LOG_INFO("[DFU] -> Wake lock never arrived, continuing anyway"); } dfuImage.Erase(); From bb16c539cbe7aa932e96abc06ca75aa42a6b56bf Mon Sep 17 00:00:00 2001 From: Ingo Kodba Date: Thu, 6 Aug 2026 07:58:06 +0200 Subject: [PATCH 2/2] Answer DFU commands that arrive in the wrong state DfuService silently drops any control point command it cannot handle in its current state, and silently drops data packets the same way. A host that reconnects to a watch left parked mid-transfer by an interrupted update therefore gets no reply at all: from its side, Start DFU simply never answers, which is indistinguishable from a watch that is not listening. Every subsequent attempt looks like an unexplained hang until the watch is rebooted or the 10s inactivity timer happens to reset the state machine. The protocol already defines ErrorCodes::InvalidState, and it was never used. Reply with [Response, opcode, InvalidState, state] instead of returning silently; the fourth byte names the state the watch is actually in, which makes the failure diagnosable from the host without a debugger. Stray data packets report once per session rather than per packet, so the notification path cannot be flooded by a host that keeps sending. --- src/components/ble/DfuService.cpp | 22 +++++++++++++++++++++- src/components/ble/DfuService.h | 2 ++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/components/ble/DfuService.cpp b/src/components/ble/DfuService.cpp index f49bada553..1a280dbbcb 100644 --- a/src/components/ble/DfuService.cpp +++ b/src/components/ble/DfuService.cpp @@ -211,12 +211,26 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf* om) { } return 0; default: - // Invalid state + // Data arriving in a state that cannot consume it. Report it once - reporting every + // stray packet would flood the notification path - so the host learns it is talking + // to a watch parked mid-transfer instead of waiting for a reply that never comes. + if (!strayPacketReported) { + strayPacketReported = true; + SendInvalidState(connectionHandle, Opcodes::ReceiveFirmwareImage); + } return 0; } return 0; } +void DfuService::SendInvalidState(uint16_t connectionHandle, Opcodes opcode) { + uint8_t data[4] {static_cast(Opcodes::Response), + static_cast(opcode), + static_cast(ErrorCodes::InvalidState), + static_cast(state)}; + notificationManager.Send(connectionHandle, controlPointCharacteristicHandle, data, 4); +} + int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { auto opcode = static_cast(om->om_data[0]); NRF_LOG_INFO("[DFU] -> ControlPointHandler"); @@ -225,6 +239,7 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { case Opcodes::StartDFU: { if (state != States::Idle && state != States::Start) { NRF_LOG_INFO("[DFU] -> Start DFU requested, but we are not in Idle state"); + SendInvalidState(connectionHandle, Opcodes::StartDFU); return 0; } if (state == States::Start) { @@ -249,6 +264,7 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { case Opcodes::InitDFUParameters: { if (state != States::Init) { NRF_LOG_INFO("[DFU] -> Init DFU requested, but we are not in Init state"); + SendInvalidState(connectionHandle, Opcodes::InitDFUParameters); return 0; } bool isInitComplete = (om->om_data[1] != 0); @@ -270,6 +286,7 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { case Opcodes::ReceiveFirmwareImage: if (state != States::Init) { NRF_LOG_INFO("[DFU] -> Receive firmware image requested, but we are not in Start Init"); + SendInvalidState(connectionHandle, Opcodes::ReceiveFirmwareImage); return 0; } // TODO the chunk size is dependent of the implementation of the host application... @@ -280,6 +297,7 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { case Opcodes::ValidateFirmware: { if (state != States::Validate) { NRF_LOG_INFO("[DFU] -> Validate firmware image requested, but we are not in Data state %d", state); + SendInvalidState(connectionHandle, Opcodes::ValidateFirmware); return 0; } @@ -310,6 +328,7 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf* om) { case Opcodes::ActivateImageAndReset: if (state != States::Validated) { NRF_LOG_INFO("[DFU] -> Activate image and reset requested, but we are not in Validated state"); + SendInvalidState(connectionHandle, Opcodes::ActivateImageAndReset); return 0; } NRF_LOG_INFO("[DFU] -> Activate image and reset!"); @@ -328,6 +347,7 @@ void DfuService::OnTimeout() { void DfuService::Reset() { state = States::Idle; + strayPacketReported = false; nbPacketsToNotify = 0; nbPacketReceived = 0; bytesReceived = 0; diff --git a/src/components/ble/DfuService.h b/src/components/ble/DfuService.h index 99be27b92d..b781ae01cb 100644 --- a/src/components/ble/DfuService.h +++ b/src/components/ble/DfuService.h @@ -119,6 +119,7 @@ namespace Pinetime { enum class States : uint8_t { Idle, Init, Start, Data, Validate, Validated }; States state = States::Idle; + bool strayPacketReported = false; enum class ImageTypes : uint8_t { NoImage = 0x00, @@ -160,6 +161,7 @@ namespace Pinetime { int SendDfuRevision(os_mbuf* om) const; int WritePacketHandler(uint16_t connectionHandle, os_mbuf* om); int ControlPointHandler(uint16_t connectionHandle, os_mbuf* om); + void SendInvalidState(uint16_t connectionHandle, Opcodes opcode); TimerHandle_t timeoutTimer; };