From 3a1a5b020fc528cb664cd9d3f04b34b777e6d2f8 Mon Sep 17 00:00:00 2001 From: Sergei Lissianoi <54454955+selissia@users.noreply.github.com> Date: Wed, 26 Jan 2022 13:25:16 -0500 Subject: [PATCH] [OTA Requestor] Implement the CancelImageUpdate() OTARequestor API (#13778) * Implement the CancelImageUpdate() OTARequestor API * Restyled by clang-format * Add CancelImageUpdate() to OTARequestorInterface * Add the override modifier * Add UpdateCancelled() API to OTARequestorDriver * Restyled by clang-format * Fix a bug in SetBlock() where varied block size will kill the download Issue: https://github.com/project-chip/connectedhomeip/issues/13393 Co-authored-by: Restyled.io --- .../clusters/ota-requestor/OTARequestor.cpp | 18 ++++++++++++++++++ src/app/clusters/ota-requestor/OTARequestor.h | 16 ++++++---------- src/include/platform/OTARequestorDriver.h | 3 +++ src/include/platform/OTARequestorInterface.h | 4 ++++ src/platform/GenericOTARequestorDriver.cpp | 5 +++++ src/platform/GenericOTARequestorDriver.h | 1 + src/platform/Linux/OTAImageProcessorImpl.cpp | 19 ++++++++++--------- 7 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp index c5c24314493d95..79f32b50134bfa 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.cpp +++ b/src/app/clusters/ota-requestor/OTARequestor.cpp @@ -269,6 +269,7 @@ void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction) { VerifyOrReturn(mOtaRequestorDriver != nullptr, ChipLogError(SoftwareUpdate, "OTA requestor driver not set")); VerifyOrReturn(mServer != nullptr, ChipLogError(SoftwareUpdate, "Server not set")); + VerifyOrReturn(mProviderNodeId != kUndefinedNodeId, ChipLogError(SoftwareUpdate, "Provider Node ID not set")); FabricInfo * fabricInfo = mServer->GetFabricTable().FindFabricWithIndex(mProviderFabricIndex); VerifyOrReturn(fabricInfo != nullptr, ChipLogError(SoftwareUpdate, "Cannot find fabric")); @@ -284,6 +285,23 @@ void OTARequestor::ConnectToProvider(OnConnectedAction onConnectedAction) ChipLogError(SoftwareUpdate, "Cannot establish connection to provider: %" CHIP_ERROR_FORMAT, err.Format())); } +// Application directs the Requestor to cancel image update in progress. All the Requestor state is +// cleared, UpdateState is reset to Idle +void OTARequestor::CancelImageUpdate() +{ + mBdxDownloader->EndDownload(CHIP_ERROR_CONNECTION_ABORTED); + + // All Provider info should be invalidated + mProviderNodeId = kUndefinedNodeId; + mProviderFabricIndex = kUndefinedFabricIndex; + mProviderEndpointId = kRootEndpointId; + + RecordNewUpdateState(OTAUpdateStateEnum::kIdle, OTAChangeReasonEnum::kUnknown); + + // Notify the platform, it might choose to take additional actions + mOtaRequestorDriver->UpdateCancelled(); +} + CHIP_ERROR OTARequestor::GetUpdateProgress(EndpointId endpointId, app::DataModel::Nullable & progress) { VerifyOrReturnError(OtaRequestorServerGetUpdateStateProgress(endpointId, progress) == EMBER_ZCL_STATUS_SUCCESS, diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h index 482d626a6d6526..d0ef303d171bc0 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.h +++ b/src/app/clusters/ota-requestor/OTARequestor.h @@ -120,13 +120,9 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe mProviderEndpointId = endpointId; } - // Application directs the Requestor to abort the download in progress. All the Requestor state (such - // as the QueryImageResponse content) is preserved - void AbortImageUpdate(); - - // Application directs the Requestor to abort the download in progress. All the Requestor state is - // cleared, UploadState is reset to Idle - void AbortAndResetState(); + // Application directs the Requestor to cancel image update in progress. All the Requestor state is + // cleared, UpdateState is reset to Idle + void CancelImageUpdate() override; // Application notifies the Requestor on the user consent action, TRUE if consent is given, // FALSE otherwise @@ -275,9 +271,9 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe static void OnNotifyUpdateAppliedFailure(void * context, CHIP_ERROR error); OTARequestorDriver * mOtaRequestorDriver = nullptr; - NodeId mProviderNodeId = kUndefinedNodeId; - FabricIndex mProviderFabricIndex = kUndefinedFabricIndex; - EndpointId mProviderEndpointId = kRootEndpointId; + NodeId mProviderNodeId = kUndefinedNodeId; // Only valid for the current update in progress + FabricIndex mProviderFabricIndex = kUndefinedFabricIndex; // Only valid for the current update in progress + EndpointId mProviderEndpointId = kRootEndpointId; // Only valid for the current update in progress uint32_t mOtaStartDelayMs = 0; CASESessionManager * mCASESessionManager = nullptr; OnConnectedAction mOnConnectedAction = kQueryImage; diff --git a/src/include/platform/OTARequestorDriver.h b/src/include/platform/OTARequestorDriver.h index 5a2e2f152c0ead..bd99df6abc3eea 100644 --- a/src/include/platform/OTARequestorDriver.h +++ b/src/include/platform/OTARequestorDriver.h @@ -89,6 +89,9 @@ class OTARequestorDriver /// Called when the current software update should be discontinued virtual void UpdateDiscontinued() = 0; + + /// Called when the current software update has been cancelled by the local application + virtual void UpdateCancelled() = 0; }; } // namespace chip diff --git a/src/include/platform/OTARequestorInterface.h b/src/include/platform/OTARequestorInterface.h index 1751e9de152739..5a132932a134c9 100644 --- a/src/include/platform/OTARequestorInterface.h +++ b/src/include/platform/OTARequestorInterface.h @@ -76,6 +76,10 @@ class OTARequestorInterface // Manually set OTA Provider parameters virtual void TestModeSetProviderParameters(NodeId nodeId, FabricIndex fabIndex, EndpointId endpointId) = 0; + + // Application directs the Requestor to cancel image update in progress. All the Requestor state is + // cleared, UpdateState is reset to Idle + virtual void CancelImageUpdate() = 0; }; // The instance of the class implementing OTARequestorInterface must be managed through diff --git a/src/platform/GenericOTARequestorDriver.cpp b/src/platform/GenericOTARequestorDriver.cpp index cccdfb3ffa63e9..7cb269cf0b3c31 100644 --- a/src/platform/GenericOTARequestorDriver.cpp +++ b/src/platform/GenericOTARequestorDriver.cpp @@ -103,6 +103,11 @@ void GenericOTARequestorDriver::UpdateDiscontinued() mImageProcessor->Abort(); } +void GenericOTARequestorDriver::UpdateCancelled() +{ + // Platform might choose to schedule a retry here +} + void GenericOTARequestorDriver::ScheduleDelayedAction(UpdateFailureState state, System::Clock::Seconds32 delay, System::TimerCompleteCallback action) { diff --git a/src/platform/GenericOTARequestorDriver.h b/src/platform/GenericOTARequestorDriver.h index 7e9ba69abed85c..053698aa916e41 100644 --- a/src/platform/GenericOTARequestorDriver.h +++ b/src/platform/GenericOTARequestorDriver.h @@ -52,6 +52,7 @@ class GenericOTARequestorDriver : public OTARequestorDriver void UpdateConfirmed(System::Clock::Seconds32 delay) override; void UpdateSuspended(System::Clock::Seconds32 delay) override; void UpdateDiscontinued() override; + void UpdateCancelled() override; private: void ScheduleDelayedAction(UpdateFailureState state, System::Clock::Seconds32 delay, System::TimerCompleteCallback action); diff --git a/src/platform/Linux/OTAImageProcessorImpl.cpp b/src/platform/Linux/OTAImageProcessorImpl.cpp index afcff9f326cfc1..39016699f98984 100644 --- a/src/platform/Linux/OTAImageProcessorImpl.cpp +++ b/src/platform/Linux/OTAImageProcessorImpl.cpp @@ -180,29 +180,30 @@ void OTAImageProcessorImpl::HandleProcessBlock(intptr_t context) CHIP_ERROR OTAImageProcessorImpl::SetBlock(ByteSpan & block) { - if ((block.data() == nullptr) || block.empty()) + if (!IsSpanUsable(block)) { + ReleaseBlock(); return CHIP_NO_ERROR; } - - // Allocate memory for block data if it has not been done yet - if (mBlock.empty()) + if (mBlock.size() < block.size()) { - mBlock = MutableByteSpan(static_cast(chip::Platform::MemoryAlloc(block.size())), block.size()); - if (mBlock.data() == nullptr) + if (!mBlock.empty()) + { + ReleaseBlock(); + } + uint8_t * mBlock_ptr = static_cast(chip::Platform::MemoryAlloc(block.size())); + if (mBlock_ptr == nullptr) { return CHIP_ERROR_NO_MEMORY; } + mBlock = MutableByteSpan(mBlock_ptr, block.size()); } - - // Store the actual block data CHIP_ERROR err = CopySpanToMutableSpan(block, mBlock); if (err != CHIP_NO_ERROR) { ChipLogError(SoftwareUpdate, "Cannot copy block data: %" CHIP_ERROR_FORMAT, err.Format()); return err; } - return CHIP_NO_ERROR; }