From 14571662b31a95d2ea0c84646b4fb4159a4eb9bf Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Sat, 4 Dec 2021 11:49:50 -0800 Subject: [PATCH] Implement BDXDownloader for new OTA Requestor API (#12560) * WIP: OTADownloader and BDXDownloader * integrate BDXDownloader * fix exchange context closing * cleaning up * fix some namespacing Co-authored-by: Carol Yang * remove unused OTADownloader reference * update README Co-authored-by: Carol Yang --- .../linux/LinuxOTAImageProcessor.cpp | 42 ++-- .../linux/LinuxOTAImageProcessor.h | 4 + examples/ota-requestor-app/linux/main.cpp | 41 ++-- .../clusters/ota-requestor/BDXDownloader.cpp | 214 +++++++++++++----- .../clusters/ota-requestor/BDXDownloader.h | 59 ++++- .../clusters/ota-requestor/OTADownloader.h | 59 +++-- .../clusters/ota-requestor/OTARequestor.cpp | 47 +++- src/app/clusters/ota-requestor/OTARequestor.h | 82 ++++++- src/app/clusters/ota-requestor/README.md | 16 +- 9 files changed, 408 insertions(+), 156 deletions(-) diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp index 44afe5605a43e1..41f717bfa3be03 100644 --- a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp +++ b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp @@ -22,12 +22,6 @@ namespace chip { -// TODO: Dummy function to be removed once BDX downloader is implemented and can return a real instance -OTADownloader * GetDownloaderInstance() -{ - return nullptr; -} - CHIP_ERROR LinuxOTAImageProcessor::PrepareDownload() { if (mParams.imageFile.empty()) @@ -83,17 +77,15 @@ CHIP_ERROR LinuxOTAImageProcessor::ProcessBlock(ByteSpan & block) void LinuxOTAImageProcessor::HandlePrepareDownload(intptr_t context) { - OTADownloader * downloader = GetDownloaderInstance(); - if (downloader == nullptr) + auto * imageProcessor = reinterpret_cast(context); + if (imageProcessor == nullptr) { - ChipLogError(SoftwareUpdate, "No known OTA downloader"); + ChipLogError(SoftwareUpdate, "ImageProcessor context is null"); return; } - - auto * imageProcessor = reinterpret_cast(context); - if (imageProcessor == nullptr) + else if (imageProcessor->mDownloader == nullptr) { - downloader->OnPreparedForDownload(CHIP_ERROR_INVALID_ARGUMENT); + ChipLogError(SoftwareUpdate, "mDownloader is null"); return; } @@ -101,11 +93,13 @@ void LinuxOTAImageProcessor::HandlePrepareDownload(intptr_t context) std::ofstream::out | std::ofstream::ate | std::ofstream::app); if (!imageProcessor->mOfs.good()) { - downloader->OnPreparedForDownload(CHIP_ERROR_OPEN_FAILED); + imageProcessor->mDownloader->OnPreparedForDownload(CHIP_ERROR_OPEN_FAILED); return; } - downloader->OnPreparedForDownload(CHIP_NO_ERROR); + // TODO: if file already exists and is not empty, erase previous contents + + imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR); } void LinuxOTAImageProcessor::HandleFinalize(intptr_t context) @@ -118,6 +112,8 @@ void LinuxOTAImageProcessor::HandleFinalize(intptr_t context) imageProcessor->mOfs.close(); imageProcessor->ReleaseBlock(); + + ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mParams.imageFile.data()); } void LinuxOTAImageProcessor::HandleAbort(intptr_t context) @@ -135,17 +131,15 @@ void LinuxOTAImageProcessor::HandleAbort(intptr_t context) void LinuxOTAImageProcessor::HandleProcessBlock(intptr_t context) { - OTADownloader * downloader = GetDownloaderInstance(); - if (downloader == nullptr) + auto * imageProcessor = reinterpret_cast(context); + if (imageProcessor == nullptr) { - ChipLogError(SoftwareUpdate, "No known OTA downloader"); + ChipLogError(SoftwareUpdate, "ImageProcessor context is null"); return; } - - auto * imageProcessor = reinterpret_cast(context); - if (imageProcessor == nullptr) + else if (imageProcessor->mDownloader == nullptr) { - downloader->OnBlockProcessed(CHIP_ERROR_INVALID_ARGUMENT, OTADownloader::kEnd); + ChipLogError(SoftwareUpdate, "mDownloader is null"); return; } @@ -154,12 +148,12 @@ void LinuxOTAImageProcessor::HandleProcessBlock(intptr_t context) if (!imageProcessor->mOfs.write(reinterpret_cast(imageProcessor->mBlock.data()), static_cast(imageProcessor->mBlock.size()))) { - downloader->OnBlockProcessed(CHIP_ERROR_WRITE_FAILED, OTADownloader::kEnd); + imageProcessor->mDownloader->EndDownload(CHIP_ERROR_WRITE_FAILED); return; } imageProcessor->mParams.downloadedBytes += imageProcessor->mBlock.size(); - downloader->OnBlockProcessed(CHIP_NO_ERROR, OTADownloader::kGetNext); + imageProcessor->mDownloader->FetchNextData(); } CHIP_ERROR LinuxOTAImageProcessor::SetBlock(ByteSpan & block) diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h index 72e528749a6138..e36b78dc78abac 100644 --- a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h +++ b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include @@ -34,6 +35,8 @@ class LinuxOTAImageProcessor : public OTAImageProcessorInterface CHIP_ERROR Abort() override; CHIP_ERROR ProcessBlock(ByteSpan & block) override; + void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; } + private: //////////// Actual handlers for the OTAImageProcessorInterface /////////////// static void HandlePrepareDownload(intptr_t context); @@ -53,6 +56,7 @@ class LinuxOTAImageProcessor : public OTAImageProcessorInterface std::ofstream mOfs; MutableByteSpan mBlock; + OTADownloader * mDownloader; }; } // namespace chip diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index f966b95b18c30a..fc004f357376ed 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -25,9 +25,10 @@ #include "LinuxOTAImageProcessor.h" #include "LinuxOTARequestorDriver.h" -#include "app/clusters/ota-requestor/OTADownloader.h" +#include "app/clusters/ota-requestor/BDXDownloader.h" #include "app/clusters/ota-requestor/OTARequestor.h" +using chip::BDXDownloader; using chip::ByteSpan; using chip::CharSpan; using chip::DeviceProxy; @@ -37,7 +38,7 @@ using chip::LinuxOTAImageProcessor; using chip::NodeId; using chip::OnDeviceConnected; using chip::OnDeviceConnectionFailure; -using chip::OTADownloader; +using chip::OTAImageProcessorParams; using chip::PeerId; using chip::Server; using chip::VendorId; @@ -49,6 +50,11 @@ using namespace chip::ArgParser; using namespace chip::Messaging; using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands; +OTARequestor requestorCore; +LinuxOTARequestorDriver requestorUser; +BDXDownloader downloader; +LinuxOTAImageProcessor imageProcessor; + bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, const char * aName, const char * aValue); void OnStartDelayTimerHandler(Layer * systemLayer, void * appState); @@ -195,25 +201,24 @@ int main(int argc, char * argv[]) SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider()); // Initialize and interconnect the Requestor and Image Processor objects -- START - // Initialize the instance of the main Requestor Class - OTARequestor * requestorCore = new OTARequestor; - SetRequestorInstance(requestorCore); - - // Initialize an instance of the Requestor Driver - LinuxOTARequestorDriver * requestorUser = new LinuxOTARequestorDriver; + SetRequestorInstance(&requestorCore); // Connect the Requestor and Requestor Driver objects - requestorCore->SetOtaRequestorDriver(requestorUser); + requestorCore.SetOtaRequestorDriver(&requestorUser); - // Initialize the Downloader object - OTADownloader * downloaderCore = new OTADownloader; - // TODO: enable SetDownloaderInstance(downloaderCore); - - // Initialize the Image Processor object - LinuxOTAImageProcessor * downloaderUser = new LinuxOTAImageProcessor; + // WARNING: this is probably not realistic to know such details of the image or to even have an OTADownloader instantiated at + // the beginning of program execution. We're using hardcoded values here for now since this is a reference application. + // TODO: instatiate and initialize these values when QueryImageResponse tells us an image is available + // TODO: add API for OTARequestor to pass QueryImageResponse info to the application to use for OTADownloader init + OTAImageProcessorParams ipParams; + ipParams.imageFile = CharSpan("test.txt"); + imageProcessor.SetOTAImageProcessorParams(ipParams); + imageProcessor.SetOTADownloader(&downloader); // Connect the Downloader and Image Processor objects - downloaderCore->SetImageProcessorDelegate(downloaderUser); + downloader.SetImageProcessorDelegate(&imageProcessor); + + requestorCore.SetBDXDownloader(&downloader); // Initialize and interconnect the Requestor and Image Processor objects -- END // Pass the IP Address to the OTARequestor object: Use of explicit IP address is temporary @@ -221,14 +226,14 @@ int main(int argc, char * argv[]) { IPAddress ipAddr; IPAddress::FromString(ipAddress, ipAddr); - requestorCore->SetIpAddress(ipAddr); + requestorCore.SetIpAddress(ipAddr); } // Test Mode operation: If a delay is provided, QueryImage after the timer expires if (delayQueryTimeInSec > 0) { // In this mode Provider node ID and fabric idx must be supplied explicitly from program args - requestorCore->TestModeSetProviderParameters(providerNodeId, providerFabricIndex); + requestorCore.TestModeSetProviderParameters(providerNodeId, providerFabricIndex); chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Milliseconds32(delayQueryTimeInSec * 1000), OnStartDelayTimerHandler, nullptr); diff --git a/src/app/clusters/ota-requestor/BDXDownloader.cpp b/src/app/clusters/ota-requestor/BDXDownloader.cpp index 7dd9b33528a76d..96d3edc21bc729 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.cpp +++ b/src/app/clusters/ota-requestor/BDXDownloader.cpp @@ -1,6 +1,7 @@ /* * * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,101 +19,200 @@ #include "BDXDownloader.h" #include -#include -#include -#include +#include +#include +#include /* TODO:(#12520) remove */ +#include +#include -#include +using chip::OTADownloader; +using chip::bdx::TransferSession; -using namespace chip::bdx; +namespace chip { -uint32_t numBlocksRead = 0; -const char outFilePath[] = "test-ota-out.txt"; +void BDXDownloader::OnMessageReceived(const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle msg) +{ + VerifyOrReturn(mState == State::kInProgress, ChipLogError(BDX, "Can't accept messages, no transfer in progress")); + CHIP_ERROR err = + mBdxTransfer.HandleMessageReceived(payloadHeader, std::move(msg), /* TODO:(#12520) */ chip::System::Clock::Seconds16(0)); + if (err != CHIP_NO_ERROR) + { + ChipLogError(BDX, "unable to handle message: %" CHIP_ERROR_FORMAT, err.Format()); + } + + // HandleMessageReceived() will only decode/parse the message. Need to Poll() in order to do the message handling work in + // HandleBdxEvent(). + PollTransferSession(); +} + +CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData) +{ + VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE); + + // Must call StartTransfer() here to store the the pointer data contained in bdxInitData in the TransferSession object. + // Otherwise it could be freed before we can use it. + ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData, + /* TODO:(#12520) */ chip::System::Clock::Seconds16(30))); -void BdxDownloader::SetInitialExchange(chip::Messaging::ExchangeContext * ec) + return CHIP_NO_ERROR; +} + +CHIP_ERROR BDXDownloader::BeginPrepareDownload() { - mExchangeCtx = ec; + VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mImageProcessor->PrepareDownload()); + + mState = State::kPreparing; + + return CHIP_NO_ERROR; } -void BdxDownloader::HandleTransferSessionOutput(TransferSession::OutputEvent & event) +CHIP_ERROR BDXDownloader::OnPreparedForDownload(CHIP_ERROR status) { - CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrReturnError(mState == State::kPreparing, CHIP_ERROR_INCORRECT_STATE); - if (event.EventType != TransferSession::OutputEventType::kNone) + if (status == CHIP_NO_ERROR) { - ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(event.EventType)); + mState = State::kInProgress; + + // Must call here because StartTransfer() should have prepared a ReceiveInit message, and now we should send it. + PollTransferSession(); + } + else + { + ChipLogError(BDX, "failed to prepare download: %" CHIP_ERROR_FORMAT, status.Format()); + mBdxTransfer.Reset(); + mState = State::kIdle; } - switch (event.EventType) + return CHIP_NO_ERROR; +} + +CHIP_ERROR BDXDownloader::FetchNextData() +{ + VerifyOrReturnError(mState == State::kInProgress, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mBdxTransfer.PrepareBlockQuery()); + PollTransferSession(); + + return CHIP_NO_ERROR; +} + +void BDXDownloader::OnDownloadTimeout() +{ + if (mState == State::kInProgress) { - case TransferSession::OutputEventType::kNone: - if (mIsTransferComplete) + ChipLogDetail(BDX, "aborting due to timeout"); + mBdxTransfer.Reset(); + if (mImageProcessor != nullptr) { - ChipLogDetail(BDX, "Transfer complete! Contents written/appended to %s", outFilePath); - mTransfer.Reset(); - mIsTransferComplete = false; + mImageProcessor->Abort(); } - break; - case TransferSession::OutputEventType::kMsgToSend: { - chip::Messaging::SendFlags sendFlags; - VerifyOrReturn(mExchangeCtx != nullptr, ChipLogError(BDX, "mExchangeContext is null, cannot proceed")); - if (event.msgTypeData.MessageType == static_cast(MessageType::ReceiveInit)) + mState = State::kIdle; + } + else + { + ChipLogError(BDX, "no download in progress"); + } +} + +void BDXDownloader::EndDownload(CHIP_ERROR reason) +{ + if (mState == State::kInProgress) + { + // There's no method for a BDX receiving driver to cleanly end a transfer + mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + if (mImageProcessor != nullptr) { - sendFlags.Set(chip::Messaging::SendMessageFlags::kFromInitiator); + mImageProcessor->Abort(); } - if (event.msgTypeData.MessageType != static_cast(MessageType::BlockAckEOF)) + mState = State::kIdle; + + // Because AbortTransfer() will generate a StatusReport to send. + PollTransferSession(); + } + else + { + ChipLogError(BDX, "no download in progress"); + } +} + +void BDXDownloader::PollTransferSession() +{ + TransferSession::OutputEvent outEvent; + + // WARNING: Is this dangerous? What happens if the loop encounters two messages that need to be sent? Does the ExchangeContext + // allow that? + do + { + mBdxTransfer.PollOutput(outEvent, /* TODO:(#12520) */ chip::System::Clock::Seconds16(0)); + CHIP_ERROR err = HandleBdxEvent(outEvent); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "HandleBDXEvent: %" CHIP_ERROR_FORMAT, err.Format())); + } while (outEvent.EventType != TransferSession::OutputEventType::kNone); +} + +CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent) +{ + VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); + + switch (outEvent.EventType) + { + case TransferSession::OutputEventType::kNone: + break; + case TransferSession::OutputEventType::kAcceptReceived: + ReturnErrorOnFailure(mBdxTransfer.PrepareBlockQuery()); + // TODO: need to check ReceiveAccept parameters + break; + case TransferSession::OutputEventType::kMsgToSend: { + VerifyOrReturnError(mMsgDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mMsgDelegate->SendMessage(outEvent)); + if (outEvent.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF)) { - sendFlags.Set(chip::Messaging::SendMessageFlags::kExpectResponse); + // BDX transfer is not complete until BlockAckEOF has been sent + mState = State::kComplete; + + // TODO: how/when to reset the BDXDownloader to be ready to handle another download } - err = mExchangeCtx->SendMessage(event.msgTypeData.ProtocolId, event.msgTypeData.MessageType, std::move(event.MsgData), - sendFlags); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "SendMessage failed: %" CHIP_ERROR_FORMAT, err.Format())); break; } - case TransferSession::OutputEventType::kAcceptReceived: - VerifyOrReturn(CHIP_NO_ERROR == mTransfer.PrepareBlockQuery(), ChipLogError(BDX, "PrepareBlockQuery failed")); - break; case TransferSession::OutputEventType::kBlockReceived: { - ChipLogDetail(BDX, "Got block length %zu", event.blockdata.Length); + chip::ByteSpan blockData(outEvent.blockdata.Data, outEvent.blockdata.Length); + ReturnErrorOnFailure(mImageProcessor->ProcessBlock(blockData)); - // TODO: something more elegant than appending to a local file - // TODO: while convenient, we should not do a synchronous block write in our example application - this is bad practice - std::ofstream otaFile(outFilePath, std::ifstream::out | std::ifstream::ate | std::ifstream::app); - otaFile.write(reinterpret_cast(event.blockdata.Data), static_cast(event.blockdata.Length)); - - if (event.blockdata.IsEof) - { - err = mTransfer.PrepareBlockAck(); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "PrepareBlockAck failed: %" CHIP_ERROR_FORMAT, err.Format())); - mIsTransferComplete = true; - } - else + // TODO: this will cause problems if Finalize() is not guaranteed to do its work after ProcessBlock(). + if (outEvent.blockdata.IsEof) { - err = mTransfer.PrepareBlockQuery(); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "PrepareBlockQuery failed: %" CHIP_ERROR_FORMAT, err.Format())); + mBdxTransfer.PrepareBlockAck(); + ReturnErrorOnFailure(mImageProcessor->Finalize()); } + break; } case TransferSession::OutputEventType::kStatusReceived: - ChipLogError(BDX, "Got StatusReport %x", static_cast(event.statusData.statusCode)); - mTransfer.Reset(); - mExchangeCtx->Close(); + ChipLogError(BDX, "BDX StatusReport %x", static_cast(outEvent.statusData.statusCode)); + mBdxTransfer.Reset(); + ReturnErrorOnFailure(mImageProcessor->Abort()); break; case TransferSession::OutputEventType::kInternalError: - ChipLogError(BDX, "InternalError"); - mTransfer.Reset(); - mExchangeCtx->Close(); + ChipLogError(BDX, "TransferSession error"); + mBdxTransfer.Reset(); + ReturnErrorOnFailure(mImageProcessor->Abort()); break; case TransferSession::OutputEventType::kTransferTimeout: ChipLogError(BDX, "Transfer timed out"); - mTransfer.Reset(); - mExchangeCtx->Close(); + mBdxTransfer.Reset(); + ReturnErrorOnFailure(mImageProcessor->Abort()); break; case TransferSession::OutputEventType::kInitReceived: case TransferSession::OutputEventType::kAckReceived: case TransferSession::OutputEventType::kQueryReceived: case TransferSession::OutputEventType::kAckEOFReceived: default: - ChipLogError(BDX, "Unexpected BDX event type: %" PRIu16, static_cast(event.EventType)); + ChipLogError(BDX, "Unexpected BDX event: %u", static_cast(outEvent.EventType)); + break; } + + return CHIP_NO_ERROR; } + +} // namespace chip diff --git a/src/app/clusters/ota-requestor/BDXDownloader.h b/src/app/clusters/ota-requestor/BDXDownloader.h index b948bdc462c73d..d4c86d29736bd1 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.h +++ b/src/app/clusters/ota-requestor/BDXDownloader.h @@ -1,6 +1,7 @@ /* * * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,21 +16,61 @@ * limitations under the License. */ -#include -#include -#include -#include +/* This file contains a definition for a class that implements OTADownloader and downloads CHIP OTA images using the BDX protocol. + * It should not execute any logic that is application specific. + */ + +// TODO: unit tests #pragma once -class BdxDownloader : public chip::bdx::Initiator +#include "OTADownloader.h" + +#include +#include +#include +#include + +namespace chip { + +class BDXDownloader : public chip::OTADownloader { public: - void SetInitialExchange(chip::Messaging::ExchangeContext * ec); + // A delegate for passing messages to/from BDXDownloader and some messaging layer. This is mainly to make BDXDownloader more + // easily unit-testable. + class MessagingDelegate + { + public: + virtual CHIP_ERROR SendMessage(const chip::bdx::TransferSession::OutputEvent & msgEvent) = 0; + virtual ~MessagingDelegate() {} + }; + + BDXDownloader() : chip::OTADownloader() {} + + // To be called when there is an incoming message to handle (of any protocol type) + void OnMessageReceived(const chip::PayloadHeader & payloadHeader, chip::System::PacketBufferHandle msg); + + void SetMessageDelegate(MessagingDelegate * delegate) { mMsgDelegate = delegate; } + + // Initialize a BDX transfer session but will not proceed until OnPreparedForDownload() is called. + CHIP_ERROR SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData); + + // OTADownloader Overrides + CHIP_ERROR BeginPrepareDownload() override; + CHIP_ERROR OnPreparedForDownload(CHIP_ERROR status) override; + void OnDownloadTimeout() override; + // BDX does not provide a mechanism for the driver of a transfer to gracefully end the exchange, so it will abort the transfer + // instead. + void EndDownload(CHIP_ERROR reason = CHIP_NO_ERROR) override; + CHIP_ERROR FetchNextData() override; + // TODO: override SkipData private: - // inherited from bdx::Endpoint - void HandleTransferSessionOutput(chip::bdx::TransferSession::OutputEvent & event); + void PollTransferSession(); + CHIP_ERROR HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent); - bool mIsTransferComplete = false; + chip::bdx::TransferSession mBdxTransfer; + MessagingDelegate * mMsgDelegate; }; + +} // namespace chip diff --git a/src/app/clusters/ota-requestor/OTADownloader.h b/src/app/clusters/ota-requestor/OTADownloader.h index af355d54b472ea..5ae41e91603dc6 100644 --- a/src/app/clusters/ota-requestor/OTADownloader.h +++ b/src/app/clusters/ota-requestor/OTADownloader.h @@ -23,9 +23,11 @@ * must include this file */ +#pragma once + #include "OTAImageProcessor.h" -#pragma once +#include namespace chip { @@ -34,40 +36,47 @@ namespace chip { class OTADownloader { public: - // API declarations start + enum class State : uint8_t + { + kIdle, + kPreparing, + kInProgress, + kComplete, + }; - // Application calls this method to direct OTADownloader to begin the download - void virtual BeginDownload(){}; + OTADownloader() : mImageProcessor(nullptr), mState(State::kIdle) {} - // Platform calls this method upon the completion of PrepareDownload() processing - void virtual OnPreparedForDownload(CHIP_ERROR status){}; + // Application calls this method to direct OTADownloader to begin the download. + // OTADownloader should handle calling into OTAImageProcessorDriver::PrepareDownload(). + CHIP_ERROR virtual BeginPrepareDownload() = 0; - // Action parameter type for the OnBlockProcessed() - enum BlockActionType - { - kGetNext, - kEnd - }; + // Platform calls this method when it is ready to begin processing downloaded image data. + // Upon this call, the OTADownloader may begin downloading data. + CHIP_ERROR virtual OnPreparedForDownload(CHIP_ERROR status) = 0; + + // Should be called when it has been determined that the download has timed out. + void virtual OnDownloadTimeout() = 0; + + // Not all download protocols will be able to close gracefully from the receiver side. + // The reason parameter should be used to indicate if this is a graceful end or a forceful abort. + void virtual EndDownload(CHIP_ERROR reason = CHIP_NO_ERROR) = 0; - // Platform calls this method upon the completion of ProcessBlock() processing - void virtual OnBlockProcessed(CHIP_ERROR status, BlockActionType action){}; + // Fetch the next set of data. May be a no-op for asynchronous protocols. + CHIP_ERROR virtual FetchNextData() { return CHIP_ERROR_NOT_IMPLEMENTED; } + + // Skip ahead some number of bytes in the download of the image file. May not be supported by some transport protocols. + CHIP_ERROR virtual SkipData(uint32_t numBytes) { return CHIP_ERROR_NOT_IMPLEMENTED; } // A setter for the delegate class pointer - void SetImageProcessorDelegate(OTAImageProcessorInterface * delegate) { mImageProcessorDelegate = delegate; } + void SetImageProcessorDelegate(OTAImageProcessorInterface * delegate) { mImageProcessor = delegate; } - // API declarations end + State GetState() const { return mState; } - // Destructor virtual ~OTADownloader() = default; -private: - OTAImageProcessorInterface * mImageProcessorDelegate; +protected: + OTAImageProcessorInterface * mImageProcessor; + State mState; }; -// Set the object implementing OTADownloader -void SetDownloaderInstance(OTADownloader * instance); - -// Get the object implementing OTADownloaderInterface -OTADownloader * GetDownloaderInstance(); - } // namespace chip diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp index cc489768065bfa..44927a74147412 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.cpp +++ b/src/app/clusters/ota-requestor/OTARequestor.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "BDXDownloader.h" @@ -112,6 +113,18 @@ void OTARequestor::mOnQueryImageResponse(void * context, const QueryImageRespons { ChipLogDetail(SoftwareUpdate, "QueryImageResponse responded with action %" PRIu8, response.status); + CHIP_ERROR err = CHIP_NO_ERROR; + + // TODO: handle QueryImageResponse status types + if (response.status != EMBER_ZCL_OTA_QUERY_STATUS_UPDATE_AVAILABLE) + { + return; + } + + VerifyOrReturn(mBdxDownloader != nullptr, ChipLogError(SoftwareUpdate, "downloader is null")); + + // TODO: allow caller to provide their own OTADownloader instance and set BDX parameters + TransferSession::TransferInitData initOptions; initOptions.TransferCtlFlags = chip::bdx::TransferControlFlags::kReceiverDrive; initOptions.MaxBlockSize = 1024; @@ -126,7 +139,7 @@ void OTARequestor::mOnQueryImageResponse(void * context, const QueryImageRespons chip::Optional session = operationalDeviceProxy->GetSecureSession(); if (exchangeMgr != nullptr && session.HasValue()) { - exchangeCtx = exchangeMgr->NewContext(session.Value(), &bdxDownloader); + exchangeCtx = exchangeMgr->NewContext(session.Value(), &mBdxMessenger); } if (exchangeCtx == nullptr) @@ -136,11 +149,14 @@ void OTARequestor::mOnQueryImageResponse(void * context, const QueryImageRespons } } - bdxDownloader.SetInitialExchange(exchangeCtx); - - // This will kick of a timer which will regularly check for updates to the bdx::TransferSession state machine. - bdxDownloader.InitiateTransfer(&chip::DeviceLayer::SystemLayer(), chip::bdx::TransferRole::kReceiver, initOptions, - chip::System::Clock::Seconds16(20)); + mBdxMessenger.Init(mBdxDownloader, exchangeCtx); + mBdxDownloader->SetMessageDelegate(&mBdxMessenger); + err = mBdxDownloader->SetBDXParams(initOptions); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(SoftwareUpdate, "Error init BDXDownloader: %" CHIP_ERROR_FORMAT, err.Format())); + err = mBdxDownloader->BeginPrepareDownload(); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(SoftwareUpdate, "Error init BDXDownloader: %" CHIP_ERROR_FORMAT, err.Format())); } EmberAfStatus OTARequestor::HandleAnnounceOTAProvider( @@ -347,6 +363,10 @@ void OTARequestor::mOnConnected(void * context, chip::DeviceProxy * deviceProxy) break; } case kStartBDX: { + VerifyOrReturn(mBdxDownloader != nullptr, ChipLogError(SoftwareUpdate, "downloader is null")); + + // TODO: allow caller to provide their own OTADownloader instance and set BDX parameters + TransferSession::TransferInitData initOptions; initOptions.TransferCtlFlags = chip::bdx::TransferControlFlags::kReceiverDrive; initOptions.MaxBlockSize = 1024; @@ -360,7 +380,7 @@ void OTARequestor::mOnConnected(void * context, chip::DeviceProxy * deviceProxy) chip::Optional session = deviceProxy->GetSecureSession(); if (exchangeMgr != nullptr && session.HasValue()) { - exchangeCtx = exchangeMgr->NewContext(session.Value(), &bdxDownloader); + exchangeCtx = exchangeMgr->NewContext(session.Value(), &mBdxMessenger); } if (exchangeCtx == nullptr) @@ -370,11 +390,14 @@ void OTARequestor::mOnConnected(void * context, chip::DeviceProxy * deviceProxy) } } - bdxDownloader.SetInitialExchange(exchangeCtx); - - // This will kick of a timer which will regularly check for updates to the bdx::TransferSession state machine. - bdxDownloader.InitiateTransfer(&chip::DeviceLayer::SystemLayer(), chip::bdx::TransferRole::kReceiver, initOptions, - chip::System::Clock::Seconds16(20)); + mBdxMessenger.Init(mBdxDownloader, exchangeCtx); + mBdxDownloader->SetMessageDelegate(&mBdxMessenger); + CHIP_ERROR err = mBdxDownloader->SetBDXParams(initOptions); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(SoftwareUpdate, "Error init BDXDownloader: %" CHIP_ERROR_FORMAT, err.Format())); + err = mBdxDownloader->BeginPrepareDownload(); + VerifyOrReturn(err == CHIP_NO_ERROR, + ChipLogError(SoftwareUpdate, "Error init BDXDownloader: %" CHIP_ERROR_FORMAT, err.Format())); break; } default: diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h index 048776bbf246b8..a0755fe74ca956 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.h +++ b/src/app/clusters/ota-requestor/OTARequestor.h @@ -20,11 +20,13 @@ * Applications implementing the OTA Requestor functionality must include this file. */ +#pragma once + #include "BDXDownloader.h" #include "OTARequestorDriver.h" #include "OTARequestorInterface.h" #include -#pragma once +#include // This class implements all of the core logic of the OTA Requestor class OTARequestor : public OTARequestorInterface @@ -39,6 +41,11 @@ class OTARequestor : public OTARequestorInterface // A setter for the delegate class pointer void SetOtaRequestorDriver(OTARequestorDriver * driver) { mOtaRequestorDriver = driver; } + // TODO: this should really be OTADownloader, but right now OTARequestor has information that we need to initialize a + // BDXDownloader specifically. + // The BDXDownloader instance should already have the ImageProcessingDelegate set. + void SetBDXDownloader(chip::BDXDownloader * downloader) { mBdxDownloader = downloader; } + // Application directs the Requestor to abort any processing related to // the image update void AbortImageUpdate(); @@ -78,7 +85,75 @@ class OTARequestor : public OTARequestorInterface kStartBDX, }; + // TODO: the application should define this, along with initializing the BDXDownloader + + // This class is purely for delivering messages and sending outgoing messages to/from the BDXDownloader. + class BDXMessenger : public chip::BDXDownloader::MessagingDelegate, public chip::Messaging::ExchangeDelegate + { + public: + CHIP_ERROR SendMessage(const chip::bdx::TransferSession::OutputEvent & event) override + { + ChipLogDetail(SoftwareUpdate, "BDX::SendMessage"); + VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); + + chip::Messaging::SendFlags sendFlags; + if (event.msgTypeData.HasMessageType(chip::bdx::MessageType::ReceiveInit)) + { + sendFlags.Set(chip::Messaging::SendMessageFlags::kFromInitiator); + } + if (!event.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF) && + !event.msgTypeData.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport)) + { + sendFlags.Set(chip::Messaging::SendMessageFlags::kExpectResponse); + } + ReturnErrorOnFailure(mExchangeCtx->SendMessage(event.msgTypeData.ProtocolId, event.msgTypeData.MessageType, + event.MsgData.Retain(), sendFlags)); + return CHIP_NO_ERROR; + } + + CHIP_ERROR OnMessageReceived(chip::Messaging::ExchangeContext * ec, const chip::PayloadHeader & payloadHeader, + chip::System::PacketBufferHandle && payload) override + { + if (mDownloader == nullptr) + { + ChipLogError(BDX, "BDXDownloader instance is null, can't pass message"); + return CHIP_NO_ERROR; + } + else + { + mDownloader->OnMessageReceived(payloadHeader, payload.Retain()); + } + + // For a receiver using BDX Protocol, all received messages will require a response except for a StatusReport + if (!payloadHeader.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport)) + { + ec->WillSendMessage(); + } + return CHIP_NO_ERROR; + } + + void OnResponseTimeout(chip::Messaging::ExchangeContext * ec) override + { + ChipLogError(BDX, "exchange timed out"); + if (mDownloader != nullptr) + { + mDownloader->OnDownloadTimeout(); + } + } + + void Init(chip::BDXDownloader * downloader, chip::Messaging::ExchangeContext * ec) + { + mExchangeCtx = ec; + mDownloader = downloader; + } + + private: + chip::Messaging::ExchangeContext * mExchangeCtx; + chip::BDXDownloader * mDownloader; + }; + // Variables + // TODO: align on variable naming standard OTARequestorDriver * mOtaRequestorDriver; chip::NodeId mProviderNodeId; chip::FabricIndex mProviderFabricIndex; @@ -86,9 +161,10 @@ class OTARequestor : public OTARequestorInterface chip::CASESessionManager * mCASESessionManager = nullptr; OnConnectedState onConnectedState = kQueryImage; chip::Messaging::ExchangeContext * exchangeCtx = nullptr; - BdxDownloader bdxDownloader; + chip::BDXDownloader * mBdxDownloader; // TODO: this should be OTADownloader + BDXMessenger mBdxMessenger; // TODO: ideally this is held by the application - // Temporary until IP address resolution is implemented in the Exchange layer + // TODO: Temporary until IP address resolution is implemented in the Exchange layer chip::Inet::IPAddress mIpAddress; // Functions diff --git a/src/app/clusters/ota-requestor/README.md b/src/app/clusters/ota-requestor/README.md index cc04baecfd1492..e76e16a4e57e44 100644 --- a/src/app/clusters/ota-requestor/README.md +++ b/src/app/clusters/ota-requestor/README.md @@ -22,15 +22,15 @@ used by Matter applications for OTA software updates to the `OTARequestor` object by calling `OTARequestor::SetOtaRequestorDriver()` -- Implement a class derived from `OTAImageProcessorDriver`, see for example - `LinuxOTARequestorDriver`. This class would typically be a part of the +- Implement a class derived from `OTAImageProcessorInterface`, see for example + `LinuxOTAImageProcessor`. This class would typically be a part of the platform. - In the application initialization logic create an instance of the - `OTADownloader` class. (TODO: How do we register it?) Create an instance of - the `OTAImageProcessorDriver` implementation and connect it to the - `OTADownloader` object by calling - `OTADownloader::SetImageProcessorDelegate()` + `BDXDownloader` class. Create an instance of the + `OTAImageProcessorInterface`-derived implementation (e.g. + `LinuxOTAImageProcessor`) and connect it to the `BDXDownloader` object by + calling `BDXDownloader::SetImageProcessorDelegate()` - See `examples/ota-requestor-app/linux/main.cpp` for an example of the initialization code discussed above @@ -42,8 +42,8 @@ used by Matter applications for OTA software updates - The interface between the core OTA Requestor functionality and the platform/application logic is realized through the virtual functions of - `OTARequestorDriver` and `OTAImageProcessorDriver` and through the - application interface methods of `OTARequestor` and `OTADownloader`. + `OTARequestorDriver` and `OTAImageProcessorInterface` and through the + application interface methods of `OTARequestor` and `BDXDownloader`. ## Design Overview