From 0862b9824b72b4c62d8179470d823006fa46ba7e Mon Sep 17 00:00:00 2001 From: Carol Yang Date: Fri, 1 Apr 2022 05:22:47 -0700 Subject: [PATCH] [OTA] Make OTAProviderDelegate responsible for responding to commands (#16654) --- .../OTAProviderExample.cpp | 63 ++++++++++--------- .../ota-provider-common/OTAProviderExample.h | 38 ++++++----- .../ota-provider/ota-provider-delegate.h | 20 ++++-- .../clusters/ota-provider/ota-provider.cpp | 36 +++-------- 4 files changed, 77 insertions(+), 80 deletions(-) diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp index 7abfa8e3bb30a5..695a76fbb81526 100644 --- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp +++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp @@ -44,6 +44,7 @@ using chip::Server; using chip::Span; using chip::app::Clusters::OTAProviderDelegate; using chip::bdx::TransferControlFlags; +using chip::Protocols::InteractionModel::Status; using namespace chip; using namespace chip::ota; using namespace chip::app::Clusters::OtaSoftwareUpdateProvider; @@ -215,10 +216,11 @@ bool OTAProviderExample::ParseOTAHeader(const char * otaFilePath, OTAImageHeader return true; } -CHIP_ERROR OTAProviderExample::SendQueryImageResponse(chip::app::CommandHandler * commandObj, - const chip::app::ConcreteCommandPath & commandPath, - const QueryImage::DecodableType & commandData) +void OTAProviderExample::SendQueryImageResponse(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, + const QueryImage::DecodableType & commandData) { + VerifyOrReturn(commandObj != nullptr, ChipLogError(SoftwareUpdate, "Invalid commandObj, cannot send QueryImageResponse")); + QueryImageResponse::Type response; bool requestorCanConsent = commandData.requestorCanConsent.ValueOr(false); uint8_t updateToken[kUpdateTokenLen] = { 0 }; @@ -249,9 +251,14 @@ CHIP_ERROR OTAProviderExample::SendQueryImageResponse(chip::app::CommandHandler if (mBdxOtaSender.InitializeTransfer(commandObj->GetSubjectDescriptor().fabricIndex, commandObj->GetSubjectDescriptor().subject) == CHIP_NO_ERROR) { - ReturnErrorOnFailure(mBdxOtaSender.PrepareForTransfer(&chip::DeviceLayer::SystemLayer(), - chip::bdx::TransferRole::kSender, bdxFlags, kMaxBdxBlockSize, - kBdxTimeout, kBdxPollFreq)); + CHIP_ERROR error = mBdxOtaSender.PrepareForTransfer(&chip::DeviceLayer::SystemLayer(), chip::bdx::TransferRole::kSender, + bdxFlags, kMaxBdxBlockSize, kBdxTimeout, kBdxPollFreq); + if (error != CHIP_NO_ERROR) + { + ChipLogError(SoftwareUpdate, "Cannot prepare for transfer: %" CHIP_ERROR_FORMAT, error.Format()); + commandObj->AddStatus(commandPath, Status::Failure); + return; + } response.imageURI.Emplace(chip::CharSpan::fromCharString(uriBuf)); response.softwareVersion.Emplace(mSoftwareVersion); @@ -282,21 +289,20 @@ CHIP_ERROR OTAProviderExample::SendQueryImageResponse(chip::app::CommandHandler response.metadataForRequestor.Emplace(chip::ByteSpan()); } + // Either sends the response or an error status commandObj->AddResponse(commandPath, response); - return CHIP_NO_ERROR; } -EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * commandObj, - const chip::app::ConcreteCommandPath & commandPath, - const QueryImage::DecodableType & commandData) +void OTAProviderExample::HandleQueryImage(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, + const QueryImage::DecodableType & commandData) { bool requestorCanConsent = commandData.requestorCanConsent.ValueOr(false); if (mIgnoreQueryImageCount > 0) { - ChipLogDetail(SoftwareUpdate, "Skip HandleQueryImage response. mIgnoreQueryImageCount %" PRIu32, mIgnoreQueryImageCount); + ChipLogDetail(SoftwareUpdate, "Skip sending QueryImageResponse, ignore count: %" PRIu32, mIgnoreQueryImageCount); mIgnoreQueryImageCount--; - return EMBER_ZCL_STATUS_SUCCESS; + return; } if (mQueryImageStatus == OTAQueryStatus::kUpdateAvailable) @@ -351,25 +357,24 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c } } - VerifyOrReturnError(SendQueryImageResponse(commandObj, commandPath, commandData) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE); + // Guarantees that either a response or an error status is sent + SendQueryImageResponse(commandObj, commandPath, commandData); // After the first response is sent, default to these values for subsequent queries mQueryImageStatus = OTAQueryStatus::kUpdateAvailable; mDelayedQueryActionTimeSec = 0; - - return EMBER_ZCL_STATUS_SUCCESS; } -EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHandler * commandObj, - const chip::app::ConcreteCommandPath & commandPath, - const ApplyUpdateRequest::DecodableType & commandData) +void OTAProviderExample::HandleApplyUpdateRequest(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, + const ApplyUpdateRequest::DecodableType & commandData) { + VerifyOrReturn(commandObj != nullptr, ChipLogError(SoftwareUpdate, "Invalid commandObj, cannot handle ApplyUpdateRequest")); + if (mIgnoreApplyUpdateCount > 0) { - ChipLogDetail(SoftwareUpdate, "Skip HandleApplyUpdateRequest response. mIgnoreApplyUpdateCount %" PRIu32, - mIgnoreApplyUpdateCount); + ChipLogDetail(SoftwareUpdate, "Skip sending ApplyUpdateResponse, ignore count %" PRIu32, mIgnoreApplyUpdateCount); mIgnoreApplyUpdateCount--; - return EMBER_ZCL_STATUS_SUCCESS; + return; } // TODO: handle multiple transfers by tracking updateTokens @@ -378,8 +383,6 @@ EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHan GetUpdateTokenString(commandData.updateToken, tokenBuf, kUpdateTokenStrLen); ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, commandData.newVersion); - VerifyOrReturnError(commandObj != nullptr, EMBER_ZCL_STATUS_INVALID_VALUE); - ApplyUpdateResponse::Type response; response.action = mUpdateAction; response.delayedActionTime = mDelayedApplyActionTimeSec; @@ -389,21 +392,19 @@ EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHan // Reset back to success case for subsequent uses mUpdateAction = OTAApplyUpdateAction::kProceed; + // Either sends the response or an error status commandObj->AddResponse(commandPath, response); - - return EMBER_ZCL_STATUS_SUCCESS; } -EmberAfStatus OTAProviderExample::HandleNotifyUpdateApplied(chip::app::CommandHandler * commandObj, - const chip::app::ConcreteCommandPath & commandPath, - const NotifyUpdateApplied::DecodableType & commandData) +void OTAProviderExample::HandleNotifyUpdateApplied(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, + const NotifyUpdateApplied::DecodableType & commandData) { + VerifyOrReturn(commandObj != nullptr, ChipLogError(SoftwareUpdate, "Invalid commandObj, cannot handle NotifyUpdateApplied")); + char tokenBuf[kUpdateTokenStrLen] = { 0 }; GetUpdateTokenString(commandData.updateToken, tokenBuf, kUpdateTokenStrLen); ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, commandData.softwareVersion); - emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); - - return EMBER_ZCL_STATUS_SUCCESS; + commandObj->AddStatus(commandPath, Status::Success); } diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h index 60be91f8f8845d..a3b63b4398918b 100644 --- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h +++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h @@ -32,24 +32,10 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate { public: - using OTAQueryStatus = chip::app::Clusters::OtaSoftwareUpdateProvider::OTAQueryStatus; - using OTAApplyUpdateAction = chip::app::Clusters::OtaSoftwareUpdateProvider::OTAApplyUpdateAction; - OTAProviderExample(); - void SetOTAFilePath(const char * path); - BdxOtaSender * GetBdxOtaSender() { return &mBdxOtaSender; } - - // Inherited from OTAProviderDelegate - EmberAfStatus HandleQueryImage( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override; - EmberAfStatus HandleApplyUpdateRequest( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) override; - EmberAfStatus HandleNotifyUpdateApplied( - chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData) override; + using OTAQueryStatus = chip::app::Clusters::OtaSoftwareUpdateProvider::OTAQueryStatus; + using OTAApplyUpdateAction = chip::app::Clusters::OtaSoftwareUpdateProvider::OTAApplyUpdateAction; static constexpr uint16_t SW_VER_STR_MAX_LEN = 64; static constexpr uint16_t OTA_URL_MAX_LEN = 512; @@ -68,6 +54,21 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate char otaURL[OTA_URL_MAX_LEN]; } DeviceSoftwareVersionModel; + //////////// OTAProviderDelegate Implementation /////////////// + void HandleQueryImage( + chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, + const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override; + void HandleApplyUpdateRequest( + chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, + const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) override; + void HandleNotifyUpdateApplied( + chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, + const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData) override; + + //////////// OTAProviderExample public APIs /////////////// + void SetOTAFilePath(const char * path); + BdxOtaSender * GetBdxOtaSender() { return &mBdxOtaSender; } + void SetOTACandidates(std::vector candidates); void SetIgnoreQueryImageCount(uint32_t count) { mIgnoreQueryImageCount = count; } void SetIgnoreApplyUpdateCount(uint32_t count) { mIgnoreApplyUpdateCount = count; } @@ -93,7 +94,10 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate bool ParseOTAHeader(const char * otaFilePath, chip::OTAImageHeader & header); - CHIP_ERROR + /** + * Called to send the response for a QueryImage command. If an error is encountered, an error status will be sent. + */ + void SendQueryImageResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData); diff --git a/src/app/clusters/ota-provider/ota-provider-delegate.h b/src/app/clusters/ota-provider/ota-provider-delegate.h index f92f945339e19c..17065194bfbbb2 100644 --- a/src/app/clusters/ota-provider/ota-provider-delegate.h +++ b/src/app/clusters/ota-provider/ota-provider-delegate.h @@ -37,14 +37,26 @@ namespace Clusters { class OTAProviderDelegate { public: - virtual EmberAfStatus HandleQueryImage(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, - const OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) = 0; + /** + * Called to handle a QueryImage command and is responsible for sending the response (if success) or status (if error). The + * caller is responsible for validating fields in the command. + */ + virtual void HandleQueryImage(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, + const OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) = 0; - virtual EmberAfStatus + /** + * Called to handle an ApplyUpdateRequest command and is responsible for sending the response (if success) or status (if error). + * The caller is responsible for validating fields in the command. + */ + virtual void HandleApplyUpdateRequest(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, const OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) = 0; - virtual EmberAfStatus + /** + * Called to handle a NotifyUpdateApplied command and is responsible for sending the status. The caller is responsible for + * validating fields in the command. + */ + virtual void HandleNotifyUpdateApplied(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, const OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData) = 0; diff --git a/src/app/clusters/ota-provider/ota-provider.cpp b/src/app/clusters/ota-provider/ota-provider.cpp index fafe93269e5b3d..5d8ca364597f59 100644 --- a/src/app/clusters/ota-provider/ota-provider.cpp +++ b/src/app/clusters/ota-provider/ota-provider.cpp @@ -82,11 +82,8 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback( app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, const Commands::ApplyUpdateRequest::DecodableType & commandData) { - auto & updateToken = commandData.updateToken; - - EndpointId endpoint = commandPath.mEndpointId; - - EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS; + auto & updateToken = commandData.updateToken; + EndpointId endpoint = commandPath.mEndpointId; OTAProviderDelegate * delegate = GetDelegate(endpoint); ChipLogProgress(Zcl, "OTA Provider received ApplyUpdateRequest"); @@ -105,11 +102,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback( return true; } - status = delegate->HandleApplyUpdateRequest(commandObj, commandPath, commandData); - if (status != EMBER_ZCL_STATUS_SUCCESS) - { - commandObj->AddStatus(commandPath, app::ToInteractionModelStatus(status)); - } + delegate->HandleApplyUpdateRequest(commandObj, commandPath, commandData); return true; } @@ -121,11 +114,8 @@ bool emberAfOtaSoftwareUpdateProviderClusterNotifyUpdateAppliedCallback( app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, const Commands::NotifyUpdateApplied::DecodableType & commandData) { - auto & updateToken = commandData.updateToken; - - EndpointId endpoint = commandPath.mEndpointId; - - EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS; + auto & updateToken = commandData.updateToken; + EndpointId endpoint = commandPath.mEndpointId; OTAProviderDelegate * delegate = GetDelegate(endpoint); ChipLogProgress(Zcl, "OTA Provider received NotifyUpdateApplied"); @@ -144,11 +134,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterNotifyUpdateAppliedCallback( return true; } - status = delegate->HandleNotifyUpdateApplied(commandObj, commandPath, commandData); - if (status != EMBER_ZCL_STATUS_SUCCESS) - { - commandObj->AddStatus(commandPath, app::ToInteractionModelStatus(status)); - } + delegate->HandleNotifyUpdateApplied(commandObj, commandPath, commandData); return true; } @@ -173,9 +159,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl (void) productId; (void) softwareVersion; - EndpointId endpoint = commandPath.mEndpointId; - - EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS; + EndpointId endpoint = commandPath.mEndpointId; OTAProviderDelegate * delegate = GetDelegate(endpoint); if (SendStatusIfDelegateNull(commandObj, commandPath)) @@ -225,11 +209,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl return true; } - status = delegate->HandleQueryImage(commandObj, commandPath, commandData); - if (status != EMBER_ZCL_STATUS_SUCCESS) - { - commandObj->AddStatus(commandPath, app::ToInteractionModelStatus(status)); - } + delegate->HandleQueryImage(commandObj, commandPath, commandData); return true; }