diff --git a/src/components/ble/DfuService.cpp b/src/components/ble/DfuService.cpp index ad9c99e9e0..1a280dbbcb 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(); @@ -201,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"); @@ -215,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) { @@ -239,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); @@ -260,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... @@ -270,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; } @@ -300,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!"); @@ -318,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; };