From 50f7e9f18761edaf12102ec7f00431c6c09d5d3d Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Thu, 25 Nov 2021 00:28:27 +0000 Subject: [PATCH 1/7] WIP: OTADownloader and BDXDownloader --- .../clusters/ota-requestor/bdx-downloader.cpp | 139 ++++++++++++++++++ .../clusters/ota-requestor/bdx-downloader.h | 58 ++++++++ .../clusters/ota-requestor/ota-downloader.h | 52 ++++--- .../ota-requestor/ota-image-processor.h | 23 ++- 4 files changed, 253 insertions(+), 19 deletions(-) create mode 100644 src/app/clusters/ota-requestor/bdx-downloader.cpp create mode 100644 src/app/clusters/ota-requestor/bdx-downloader.h diff --git a/src/app/clusters/ota-requestor/bdx-downloader.cpp b/src/app/clusters/ota-requestor/bdx-downloader.cpp new file mode 100644 index 00000000000000..1f6379bd9aa1f2 --- /dev/null +++ b/src/app/clusters/ota-requestor/bdx-downloader.cpp @@ -0,0 +1,139 @@ +#include "bdx-downloader.h" + +#include +#include + +using chip::bdx::TransferSession; +using chip::bdx::TransferSession::OutputEventType; + +void BdxDownloader::OnMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg) +{ + VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); + CHIP_ERROR err = mBdxTransfer.HandleMessageReceived(payloadHeader, msg, /* TODO:(#12520) */ 0); + if (err != CHIP_NO_ERROR) + { + ChipLogError(BDX, "unable to handle message: %" CHIP_ERROR_FORMAT, err.Format()); + } +} + +CHIP_ERROR BdxDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData) +{ + VerifyOrReturnError(mState == kIdle, CHIP_ERROR_INCORRECT_STATE); + + // Must call StartTransfer() here or otherwise the pointer data contained in bdxInitData could be freed before we can use it. + ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData, 0)); +} + +CHIP_ERROR BdxDownloader::BeginPrepareDownload() +{ + VerifyOrReturnError(mState == kIdle, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mImageProcessor->PrepareDownload()); + + mState = kPreparing; +} + +CHIP_ERROR BdxDownloader::OnPreparedForDownload() +{ + VerifyOrReturnError(mState == kPreparing, CHIP_ERROR_INCORRECT_STATE); + PollTransferSession(); + + mState = kInProgress; +} + +CHIP_ERROR BdxDownloader::FetchNextData() +{ + VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mBdxTransfer.PrepareBlockQuery()); + PollTransferSession(); +} + +void BdxDownloader::OnDownloadTimedOut() +{ + if (mState == kInProgress) + { + mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + } + else + { + ChipLogError(BDX, "no download in progress"); + } +} + +void BdxDownloader::EndDownload(CHIP_ERROR reason = CHIP_NO_ERROR) +{ + if (mState == kInProgress) + { + mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + if (mImageProcessor != nullptr) + { + ReturnErrorOnFailure(mImageProcessor->Abort()); + } + } + else + { + ChipLogError(BDX, "no download in progress"); + } + + // Because AbortTransfer() will generate a StatusReport to send. + PollTransferSession(); +} + +void BdxDownloader::PollTransferSession() +{ + TransferSession::OutputEvent outEvent; + + // TODO: Is this dangerous? What happens if the loop encounters two messages that need to be sent? + while (outEvent.EventType != TransferSession::OutputEventType::kNone) + { + mBdxTransfer.PollOutput(outEvent, 0); + CHIP_ERROR err = HandleBdxEvent(outEvent); + VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "HandleBDXEvent: %" CHIP_ERROR_FORMAT, err.Format())); + } +} + +CHIP_ERROR BdxDownloader::HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent) +{ + VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); + + switch (outEvent.EventType) + { + case TransferSession::OutputEventType::kNone: + // TODO: + break; + case TransferSession::OutputEventType::kAcceptReceived: + // TODO: + break; + case OutputEventType::kMsgToSend: + VerifyOrReturnError(mMsgDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); + CHIP_ERROR err = mMsgDelegate->SendMessage(outEvent); + break; + case OutputEventType::kBlockReceived: { + chip::ByteSpan blockData(outEvent.blockdata.Data, outEvent.blockdata.Length); + CHIP_ERROR err = mImageProcessor->ProcessBlock(blockData); + break; + } + case OutputEventType::kStatusReceived: + ChipLogError(BDX, "BDX StatusReport %x", static_cast(event.statusData.statusCode)); + mBdxTransfer.Reset(); + ReturnErrorOnFailure(mImageProcessor->Abort()); + break; + case OutputEventType::kInternalError: + ChipLogError(BDX, "TransferSession error"); + mBdxTransfer.Reset(); + ReturnErrorOnFailure(mImageProcessor->Abort()); + break; + case OutputEventType::kTransferTimeout: + ChipLogError(BDX, "Transfer timed out"); + 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: %u", outEvent.EventType); + break; + } diff --git a/src/app/clusters/ota-requestor/bdx-downloader.h b/src/app/clusters/ota-requestor/bdx-downloader.h new file mode 100644 index 00000000000000..57b40d9e6f689f --- /dev/null +++ b/src/app/clusters/ota-requestor/bdx-downloader.h @@ -0,0 +1,58 @@ +/* + * + * 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "ota-downloader.h" + +#include +#include + +class BdxDownloader : public OTADownloader +{ +public: + class MessagingDelegate + { + virtual CHIP_ERROR SendMessage(const chip::bdx::TransferSession::OutputEvent & msgEvent) = 0; + }; + + BdxDownloader() : OTADownloader() {} + + void OnMessageReceived(const PayloadHeader & payloadHeader, 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() 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: + void PollTransferSession(); + CHIP_ERROR HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent); + + chip::bdx::TransferSession mBdxTransfer; + MessagingDelegate * mMsgDelegate; +}; diff --git a/src/app/clusters/ota-requestor/ota-downloader.h b/src/app/clusters/ota-requestor/ota-downloader.h index 1039a5994c14c3..893e74fbdeba03 100644 --- a/src/app/clusters/ota-requestor/ota-downloader.h +++ b/src/app/clusters/ota-requestor/ota-downloader.h @@ -23,38 +23,56 @@ * must include this file */ +#pragma once + #include "ota-image-processor.h" -#pragma once +#include // A class that abstracts the image download functionality from the particular // protocol used for that (BDX or possibly HTTPS) 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() : mState(State::kIdle), mImageProcessor(nullptr) {} - // Platform calls this method upon the completion of PrepareDownload() processing - void virtual OnPreparedForDownload(); + // 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() = 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(BlockActionType action); + // Fetch the next set of data. May not make sense 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(OTAImageProcessorDriver * delegate); + void SetImageProcessorDelegate(OTAImageProcessorDriver * delegate) { mImageProcessor = delegate; } + + State GetState() const { return mState; } // API declarations end -private: - OTAImageProcessorDriver * mImageProcessorDelegate; +protected: + OTAImageProcessorDriver * mImageProcessor; + State mState; }; diff --git a/src/app/clusters/ota-requestor/ota-image-processor.h b/src/app/clusters/ota-requestor/ota-image-processor.h index b9a7ee9856c261..d9f0fd68e82c33 100644 --- a/src/app/clusters/ota-requestor/ota-image-processor.h +++ b/src/app/clusters/ota-requestor/ota-image-processor.h @@ -23,6 +23,25 @@ #pragma once +#include + +// Action parameter type for the OnBlockProcessed() +enum class NextActionType : uint8_t +{ + kGetNext, + kSkipBytes, + kEnd +}; + +struct DownloadAction +{ + NextActionType Action; + chip::Optional BytesToSkip; +} + +typedef void (*OnDownloadPrepared)(void * context); +typedef void (*OnBlockProcessed)(void * context, DownloadAction action); + // This is a platform-agnostic interface for processing downloaded // chunks of OTA image data (data could be raw image data meant for flash or // metadata). Each platform should provide an implementation of this @@ -31,10 +50,10 @@ class OTAImageProcessorDriver { public: // Open file, find block of space in persistent memory, or allocate a buffer, etc. - virtual CHIP_ERROR PrepareDownload() = 0; + virtual CHIP_ERROR PrepareDownload(chip::Callback::Callback * callback) = 0; // Must not be a blocking call to support cases that require IO to elements such as // external peripherals/radios - virtual CHIP_ERROR ProcessBlock(ByteSpan & data) = 0; + virtual CHIP_ERROR ProcessBlock(ByteSpan & data, chip::Callback::Callback * callback) = 0; // Close file, close persistent storage, etc virtual CHIP_ERROR Finalize() = 0; From 793213e0b03e923ad1565902192286d6aced35d1 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 3 Dec 2021 14:43:26 +0000 Subject: [PATCH 2/7] integrate BDXDownloader --- .../esp32/main/OTARequesterImpl.cpp | 2 +- .../linux/LinuxOTAImageProcessor.cpp | 39 ++-- .../linux/LinuxOTAImageProcessor.h | 4 + examples/ota-requestor-app/linux/main.cpp | 34 +-- .../clusters/ota-requestor/BDXDownloader.cpp | 207 +++++++++++------- .../clusters/ota-requestor/BDXDownloader.h | 50 ++++- .../clusters/ota-requestor/OTADownloader.h | 8 +- .../clusters/ota-requestor/OTARequestor.cpp | 45 +++- src/app/clusters/ota-requestor/OTARequestor.h | 75 ++++++- .../clusters/ota-requestor/bdx-downloader.cpp | 139 ------------ .../clusters/ota-requestor/bdx-downloader.h | 58 ----- 11 files changed, 317 insertions(+), 344 deletions(-) delete mode 100644 src/app/clusters/ota-requestor/bdx-downloader.cpp delete mode 100644 src/app/clusters/ota-requestor/bdx-downloader.h diff --git a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp index b6c84742019da4..6324d7423ed2d3 100644 --- a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp +++ b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp @@ -72,7 +72,7 @@ enum OTARequestorCommands namespace { // TODO: Encapsulate these globals and the callbacks in some class ExchangeContext * exchangeCtx = nullptr; -BdxDownloader bdxDownloader; +BDXDownloader bdxDownloader; enum OTARequestorCommands operationalDeviceContext; constexpr uint8_t kMaxUpdateTokenLen = 32; // must be between 8 and 32 diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp index 44afe5605a43e1..d08929baec2dea 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()) @@ -60,6 +54,7 @@ CHIP_ERROR LinuxOTAImageProcessor::Abort() CHIP_ERROR LinuxOTAImageProcessor::ProcessBlock(ByteSpan & block) { + ChipLogDetail(BDX, "TREVOR %s", __FUNCTION__); if (!mOfs.is_open() || !mOfs.good()) { return CHIP_ERROR_INTERNAL; @@ -83,17 +78,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 +94,11 @@ 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); + imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR); } void LinuxOTAImageProcessor::HandleFinalize(intptr_t context) @@ -135,17 +128,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 +145,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..ccd8d20c9773d3 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(chip::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..48c4dad36b94dc 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -25,9 +25,11 @@ #include "LinuxOTAImageProcessor.h" #include "LinuxOTARequestorDriver.h" +#include "app/clusters/ota-requestor/BDXDownloader.h" #include "app/clusters/ota-requestor/OTADownloader.h" #include "app/clusters/ota-requestor/OTARequestor.h" +using chip::BDXDownloader; using chip::ByteSpan; using chip::CharSpan; using chip::DeviceProxy; @@ -38,6 +40,7 @@ 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 +52,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 +203,21 @@ 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); - - // Initialize the Downloader object - OTADownloader * downloaderCore = new OTADownloader; - // TODO: enable SetDownloaderInstance(downloaderCore); + requestorCore.SetOtaRequestorDriver(&requestorUser); // Initialize the Image Processor object - LinuxOTAImageProcessor * downloaderUser = new LinuxOTAImageProcessor; + 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 +225,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..9a2a9569e5cbce 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.cpp +++ b/src/app/clusters/ota-requestor/BDXDownloader.cpp @@ -1,118 +1,173 @@ -/* - * - * Copyright (c) 2021 Project CHIP Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - #include "BDXDownloader.h" #include -#include -#include -#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()); + } + PollTransferSession(); +} -void BdxDownloader::SetInitialExchange(chip::Messaging::ExchangeContext * ec) +CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData) { - mExchangeCtx = ec; + VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE); + + // Must call StartTransfer() here or otherwise the pointer data contained in bdxInitData could be freed before we can use it. + ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData, + /* TODO:(#12520) */ chip::System::Clock::Seconds16(30))); + + return CHIP_NO_ERROR; } -void BdxDownloader::HandleTransferSessionOutput(TransferSession::OutputEvent & event) +CHIP_ERROR BDXDownloader::BeginPrepareDownload() { - CHIP_ERROR err = CHIP_NO_ERROR; + VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mImageProcessor->PrepareDownload()); + + mState = State::kPreparing; - if (event.EventType != TransferSession::OutputEventType::kNone) + return CHIP_NO_ERROR; +} + +CHIP_ERROR BDXDownloader::OnPreparedForDownload(CHIP_ERROR status) +{ + VerifyOrReturnError(mState == State::kPreparing, CHIP_ERROR_INCORRECT_STATE); + + if (status == CHIP_NO_ERROR) { - ChipLogDetail(BDX, "OutputEvent type: %s", event.ToString(event.EventType)); + mState = State::kInProgress; + 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) + mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + } + else + { + ChipLogError(BDX, "no download in progress"); + } +} + +void BDXDownloader::EndDownload(CHIP_ERROR reason) +{ + if (mState == State::kInProgress) + { + mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + if (mImageProcessor != nullptr) { - ChipLogDetail(BDX, "Transfer complete! Contents written/appended to %s", outFilePath); - mTransfer.Reset(); - mIsTransferComplete = false; + mImageProcessor->Abort(); } + } + else + { + ChipLogError(BDX, "no download in progress"); + } + + // Because AbortTransfer() will generate a StatusReport to send. + PollTransferSession(); +} + +void BDXDownloader::PollTransferSession() +{ + TransferSession::OutputEvent outEvent; + + // TODO: Is this dangerous? What happens if the loop encounters two messages that need to be sent? + 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(mState == State::kInProgress, CHIP_ERROR_INCORRECT_STATE); + 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: { - chip::Messaging::SendFlags sendFlags; - VerifyOrReturn(mExchangeCtx != nullptr, ChipLogError(BDX, "mExchangeContext is null, cannot proceed")); - if (event.msgTypeData.MessageType == static_cast(MessageType::ReceiveInit)) - { - sendFlags.Set(chip::Messaging::SendMessageFlags::kFromInitiator); - } - if (event.msgTypeData.MessageType != static_cast(MessageType::BlockAckEOF)) - { - sendFlags.Set(chip::Messaging::SendMessageFlags::kExpectResponse); - } - 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())); + VerifyOrReturnError(mMsgDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); + ReturnErrorOnFailure(mMsgDelegate->SendMessage(outEvent)); 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); - - // 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)); + chip::ByteSpan blockData(outEvent.blockdata.Data, outEvent.blockdata.Length); + ReturnErrorOnFailure(mImageProcessor->ProcessBlock(blockData)); - 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: could calling Finalize immediately after ProcessBlock cause issues? + if (outEvent.blockdata.IsEof) { - err = mTransfer.PrepareBlockQuery(); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "PrepareBlockQuery failed: %" CHIP_ERROR_FORMAT, err.Format())); + 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..0cafc5d5d6dd45 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,52 @@ * limitations under the License. */ -#include -#include +#pragma once + +#include "OTADownloader.h" + +#include #include -#include +#include +#include +#include -#pragma once +namespace chip { -class BdxDownloader : public chip::bdx::Initiator +class BDXDownloader : public chip::OTADownloader { public: - void SetInitialExchange(chip::Messaging::ExchangeContext * ec); + class MessagingDelegate + { + public: + virtual CHIP_ERROR SendMessage(const chip::bdx::TransferSession::OutputEvent & msgEvent) = 0; + virtual ~MessagingDelegate() {} + }; + + BDXDownloader() : chip::OTADownloader() {} + + 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 8aba47c1e8a922..af720429db0c85 100644 --- a/src/app/clusters/ota-requestor/OTADownloader.h +++ b/src/app/clusters/ota-requestor/OTADownloader.h @@ -44,7 +44,7 @@ class OTADownloader kComplete, }; - OTADownloader() : mState(State::kIdle), mImageProcessor(nullptr) {} + OTADownloader() : mImageProcessor(nullptr), mState(State::kIdle) {} // Application calls this method to direct OTADownloader to begin the download. // OTADownloader should handle calling into OTAImageProcessorDriver::PrepareDownload(). @@ -81,10 +81,4 @@ class OTADownloader 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..b2ac46fbe2d491 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,8 @@ void OTARequestor::mOnConnected(void * context, chip::DeviceProxy * deviceProxy) break; } case kStartBDX: { + VerifyOrReturn(mBdxDownloader != nullptr, ChipLogError(SoftwareUpdate, "downloader is null")); + TransferSession::TransferInitData initOptions; initOptions.TransferCtlFlags = chip::bdx::TransferControlFlags::kReceiverDrive; initOptions.MaxBlockSize = 1024; @@ -360,7 +378,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 +388,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..9a257ba255fea1 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,68 @@ 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.MessageType == static_cast(chip::bdx::MessageType::ReceiveInit)) + { + sendFlags.Set(chip::Messaging::SendMessageFlags::kFromInitiator); + } + if (event.msgTypeData.MessageType != static_cast(chip::bdx::MessageType::BlockAckEOF)) + { + 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()); + 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 +154,10 @@ class OTARequestor : public OTARequestorInterface chip::CASESessionManager * mCASESessionManager = nullptr; OnConnectedState onConnectedState = kQueryImage; chip::Messaging::ExchangeContext * exchangeCtx = nullptr; - BdxDownloader bdxDownloader; + chip::BDXDownloader * mBdxDownloader; + BDXMessenger mBdxMessenger; - // 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/bdx-downloader.cpp b/src/app/clusters/ota-requestor/bdx-downloader.cpp deleted file mode 100644 index 1f6379bd9aa1f2..00000000000000 --- a/src/app/clusters/ota-requestor/bdx-downloader.cpp +++ /dev/null @@ -1,139 +0,0 @@ -#include "bdx-downloader.h" - -#include -#include - -using chip::bdx::TransferSession; -using chip::bdx::TransferSession::OutputEventType; - -void BdxDownloader::OnMessageReceived(const PayloadHeader & payloadHeader, System::PacketBufferHandle msg) -{ - VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = mBdxTransfer.HandleMessageReceived(payloadHeader, msg, /* TODO:(#12520) */ 0); - if (err != CHIP_NO_ERROR) - { - ChipLogError(BDX, "unable to handle message: %" CHIP_ERROR_FORMAT, err.Format()); - } -} - -CHIP_ERROR BdxDownloader::SetBDXParams(const chip::bdx::TransferSession::TransferInitData & bdxInitData) -{ - VerifyOrReturnError(mState == kIdle, CHIP_ERROR_INCORRECT_STATE); - - // Must call StartTransfer() here or otherwise the pointer data contained in bdxInitData could be freed before we can use it. - ReturnErrorOnFailure(mBdxTransfer.StartTransfer(chip::bdx::TransferRole::kReceiver, bdxInitData, 0)); -} - -CHIP_ERROR BdxDownloader::BeginPrepareDownload() -{ - VerifyOrReturnError(mState == kIdle, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); - ReturnErrorOnFailure(mImageProcessor->PrepareDownload()); - - mState = kPreparing; -} - -CHIP_ERROR BdxDownloader::OnPreparedForDownload() -{ - VerifyOrReturnError(mState == kPreparing, CHIP_ERROR_INCORRECT_STATE); - PollTransferSession(); - - mState = kInProgress; -} - -CHIP_ERROR BdxDownloader::FetchNextData() -{ - VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); - ReturnErrorOnFailure(mBdxTransfer.PrepareBlockQuery()); - PollTransferSession(); -} - -void BdxDownloader::OnDownloadTimedOut() -{ - if (mState == kInProgress) - { - mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); - } - else - { - ChipLogError(BDX, "no download in progress"); - } -} - -void BdxDownloader::EndDownload(CHIP_ERROR reason = CHIP_NO_ERROR) -{ - if (mState == kInProgress) - { - mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); - if (mImageProcessor != nullptr) - { - ReturnErrorOnFailure(mImageProcessor->Abort()); - } - } - else - { - ChipLogError(BDX, "no download in progress"); - } - - // Because AbortTransfer() will generate a StatusReport to send. - PollTransferSession(); -} - -void BdxDownloader::PollTransferSession() -{ - TransferSession::OutputEvent outEvent; - - // TODO: Is this dangerous? What happens if the loop encounters two messages that need to be sent? - while (outEvent.EventType != TransferSession::OutputEventType::kNone) - { - mBdxTransfer.PollOutput(outEvent, 0); - CHIP_ERROR err = HandleBdxEvent(outEvent); - VerifyOrReturn(err == CHIP_NO_ERROR, ChipLogError(BDX, "HandleBDXEvent: %" CHIP_ERROR_FORMAT, err.Format())); - } -} - -CHIP_ERROR BdxDownloader::HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent) -{ - VerifyOrReturnError(mState == kInProgress, CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); - - switch (outEvent.EventType) - { - case TransferSession::OutputEventType::kNone: - // TODO: - break; - case TransferSession::OutputEventType::kAcceptReceived: - // TODO: - break; - case OutputEventType::kMsgToSend: - VerifyOrReturnError(mMsgDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR err = mMsgDelegate->SendMessage(outEvent); - break; - case OutputEventType::kBlockReceived: { - chip::ByteSpan blockData(outEvent.blockdata.Data, outEvent.blockdata.Length); - CHIP_ERROR err = mImageProcessor->ProcessBlock(blockData); - break; - } - case OutputEventType::kStatusReceived: - ChipLogError(BDX, "BDX StatusReport %x", static_cast(event.statusData.statusCode)); - mBdxTransfer.Reset(); - ReturnErrorOnFailure(mImageProcessor->Abort()); - break; - case OutputEventType::kInternalError: - ChipLogError(BDX, "TransferSession error"); - mBdxTransfer.Reset(); - ReturnErrorOnFailure(mImageProcessor->Abort()); - break; - case OutputEventType::kTransferTimeout: - ChipLogError(BDX, "Transfer timed out"); - 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: %u", outEvent.EventType); - break; - } diff --git a/src/app/clusters/ota-requestor/bdx-downloader.h b/src/app/clusters/ota-requestor/bdx-downloader.h deleted file mode 100644 index 57b40d9e6f689f..00000000000000 --- a/src/app/clusters/ota-requestor/bdx-downloader.h +++ /dev/null @@ -1,58 +0,0 @@ -/* - * - * 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include "ota-downloader.h" - -#include -#include - -class BdxDownloader : public OTADownloader -{ -public: - class MessagingDelegate - { - virtual CHIP_ERROR SendMessage(const chip::bdx::TransferSession::OutputEvent & msgEvent) = 0; - }; - - BdxDownloader() : OTADownloader() {} - - void OnMessageReceived(const PayloadHeader & payloadHeader, 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() 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: - void PollTransferSession(); - CHIP_ERROR HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent); - - chip::bdx::TransferSession mBdxTransfer; - MessagingDelegate * mMsgDelegate; -}; From c6ba4e4fdf29e9feae3478fba76a0752a4153dea Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 3 Dec 2021 16:11:13 +0000 Subject: [PATCH 3/7] fix exchange context closing --- .../ota-requestor-app/linux/LinuxOTAImageProcessor.cpp | 1 - src/app/clusters/ota-requestor/BDXDownloader.cpp | 9 ++++++++- src/app/clusters/ota-requestor/OTARequestor.h | 7 ++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp index d08929baec2dea..75b404ded04133 100644 --- a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp +++ b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp @@ -54,7 +54,6 @@ CHIP_ERROR LinuxOTAImageProcessor::Abort() CHIP_ERROR LinuxOTAImageProcessor::ProcessBlock(ByteSpan & block) { - ChipLogDetail(BDX, "TREVOR %s", __FUNCTION__); if (!mOfs.is_open() || !mOfs.good()) { return CHIP_ERROR_INTERNAL; diff --git a/src/app/clusters/ota-requestor/BDXDownloader.cpp b/src/app/clusters/ota-requestor/BDXDownloader.cpp index 9a2a9569e5cbce..e98a7917e1b35f 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.cpp +++ b/src/app/clusters/ota-requestor/BDXDownloader.cpp @@ -2,6 +2,7 @@ #include #include +#include using chip::OTADownloader; using chip::bdx::TransferSession; @@ -130,17 +131,23 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu case TransferSession::OutputEventType::kMsgToSend: { VerifyOrReturnError(mMsgDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); ReturnErrorOnFailure(mMsgDelegate->SendMessage(outEvent)); + if (outEvent.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF)) + { + mState = State::kComplete; + } break; } case TransferSession::OutputEventType::kBlockReceived: { chip::ByteSpan blockData(outEvent.blockdata.Data, outEvent.blockdata.Length); ReturnErrorOnFailure(mImageProcessor->ProcessBlock(blockData)); - // TODO: could calling Finalize immediately after ProcessBlock cause issues? + // TODO: this will cause problems if Finalize() is not guaranteed to do its work after ProcessBlock(). if (outEvent.blockdata.IsEof) { + mBdxTransfer.PrepareBlockAck(); ReturnErrorOnFailure(mImageProcessor->Finalize()); } + break; } case TransferSession::OutputEventType::kStatusReceived: diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h index 9a257ba255fea1..692e0857f75f1b 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.h +++ b/src/app/clusters/ota-requestor/OTARequestor.h @@ -121,8 +121,13 @@ class OTARequestor : public OTARequestorInterface else { mDownloader->OnMessageReceived(payloadHeader, payload.Retain()); - return CHIP_NO_ERROR; } + + if (!payloadHeader.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport)) + { + ec->WillSendMessage(); + } + return CHIP_NO_ERROR; } void OnResponseTimeout(chip::Messaging::ExchangeContext * ec) override From a5374449fdc30fe7914e2b12882b30b9af75a637 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 3 Dec 2021 17:48:17 +0000 Subject: [PATCH 4/7] cleaning up --- .../esp32/main/OTARequesterImpl.cpp | 2 +- .../linux/LinuxOTAImageProcessor.cpp | 4 ++ examples/ota-requestor-app/linux/main.cpp | 5 +- .../clusters/ota-requestor/BDXDownloader.cpp | 54 ++++++++++++++++--- .../clusters/ota-requestor/BDXDownloader.h | 11 +++- .../clusters/ota-requestor/OTADownloader.h | 4 +- .../clusters/ota-requestor/OTARequestor.cpp | 2 + src/app/clusters/ota-requestor/OTARequestor.h | 10 ++-- 8 files changed, 74 insertions(+), 18 deletions(-) diff --git a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp index 6324d7423ed2d3..b6c84742019da4 100644 --- a/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp +++ b/examples/ota-requestor-app/esp32/main/OTARequesterImpl.cpp @@ -72,7 +72,7 @@ enum OTARequestorCommands namespace { // TODO: Encapsulate these globals and the callbacks in some class ExchangeContext * exchangeCtx = nullptr; -BDXDownloader bdxDownloader; +BdxDownloader bdxDownloader; enum OTARequestorCommands operationalDeviceContext; constexpr uint8_t kMaxUpdateTokenLen = 32; // must be between 8 and 32 diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp index 75b404ded04133..41f717bfa3be03 100644 --- a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp +++ b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.cpp @@ -97,6 +97,8 @@ void LinuxOTAImageProcessor::HandlePrepareDownload(intptr_t context) return; } + // TODO: if file already exists and is not empty, erase previous contents + imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR); } @@ -110,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) diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 48c4dad36b94dc..27542df409904c 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -208,7 +208,10 @@ int main(int argc, char * argv[]) // Connect the Requestor and Requestor Driver objects requestorCore.SetOtaRequestorDriver(&requestorUser); - // Initialize the Image Processor object + // 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); diff --git a/src/app/clusters/ota-requestor/BDXDownloader.cpp b/src/app/clusters/ota-requestor/BDXDownloader.cpp index e98a7917e1b35f..96d3edc21bc729 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.cpp +++ b/src/app/clusters/ota-requestor/BDXDownloader.cpp @@ -1,8 +1,29 @@ +/* + * + * 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #include "BDXDownloader.h" #include #include #include +#include /* TODO:(#12520) remove */ +#include +#include using chip::OTADownloader; using chip::bdx::TransferSession; @@ -18,6 +39,9 @@ void BDXDownloader::OnMessageReceived(const chip::PayloadHeader & payloadHeader, { 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(); } @@ -25,7 +49,8 @@ CHIP_ERROR BDXDownloader::SetBDXParams(const chip::bdx::TransferSession::Transfe { VerifyOrReturnError(mState == State::kIdle, CHIP_ERROR_INCORRECT_STATE); - // Must call StartTransfer() here or otherwise the pointer data contained in bdxInitData could be freed before we can use it. + // 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))); @@ -50,6 +75,8 @@ CHIP_ERROR BDXDownloader::OnPreparedForDownload(CHIP_ERROR status) if (status == CHIP_NO_ERROR) { mState = State::kInProgress; + + // Must call here because StartTransfer() should have prepared a ReceiveInit message, and now we should send it. PollTransferSession(); } else @@ -75,7 +102,13 @@ void BDXDownloader::OnDownloadTimeout() { if (mState == State::kInProgress) { - mBdxTransfer.AbortTransfer(chip::bdx::StatusCode::kUnknown); + ChipLogDetail(BDX, "aborting due to timeout"); + mBdxTransfer.Reset(); + if (mImageProcessor != nullptr) + { + mImageProcessor->Abort(); + } + mState = State::kIdle; } else { @@ -87,26 +120,29 @@ 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) { mImageProcessor->Abort(); } + mState = State::kIdle; + + // Because AbortTransfer() will generate a StatusReport to send. + PollTransferSession(); } else { ChipLogError(BDX, "no download in progress"); } - - // Because AbortTransfer() will generate a StatusReport to send. - PollTransferSession(); } void BDXDownloader::PollTransferSession() { TransferSession::OutputEvent outEvent; - // TODO: Is this dangerous? What happens if the loop encounters two messages that need to be sent? + // 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)); @@ -117,7 +153,6 @@ void BDXDownloader::PollTransferSession() CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::OutputEvent & outEvent) { - VerifyOrReturnError(mState == State::kInProgress, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mImageProcessor != nullptr, CHIP_ERROR_INCORRECT_STATE); switch (outEvent.EventType) @@ -126,14 +161,17 @@ CHIP_ERROR BDXDownloader::HandleBdxEvent(const chip::bdx::TransferSession::Outpu break; case TransferSession::OutputEventType::kAcceptReceived: ReturnErrorOnFailure(mBdxTransfer.PrepareBlockQuery()); - // TODO: need to check ReceiveAccept parameters? + // 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)) { + // 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 } break; } diff --git a/src/app/clusters/ota-requestor/BDXDownloader.h b/src/app/clusters/ota-requestor/BDXDownloader.h index 0cafc5d5d6dd45..d4c86d29736bd1 100644 --- a/src/app/clusters/ota-requestor/BDXDownloader.h +++ b/src/app/clusters/ota-requestor/BDXDownloader.h @@ -16,13 +16,18 @@ * limitations under the License. */ +/* 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 #include "OTADownloader.h" #include #include -#include #include #include @@ -31,6 +36,8 @@ namespace chip { class BDXDownloader : public chip::OTADownloader { public: + // 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: @@ -40,7 +47,9 @@ class BDXDownloader : public chip::OTADownloader 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. diff --git a/src/app/clusters/ota-requestor/OTADownloader.h b/src/app/clusters/ota-requestor/OTADownloader.h index af720429db0c85..5ae41e91603dc6 100644 --- a/src/app/clusters/ota-requestor/OTADownloader.h +++ b/src/app/clusters/ota-requestor/OTADownloader.h @@ -61,7 +61,7 @@ class OTADownloader // 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; - // Fetch the next set of data. May not make sense for asynchronous protocols. + // 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. @@ -72,8 +72,6 @@ class OTADownloader State GetState() const { return mState; } - // API declarations end - virtual ~OTADownloader() = default; protected: diff --git a/src/app/clusters/ota-requestor/OTARequestor.cpp b/src/app/clusters/ota-requestor/OTARequestor.cpp index b2ac46fbe2d491..44927a74147412 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.cpp +++ b/src/app/clusters/ota-requestor/OTARequestor.cpp @@ -365,6 +365,8 @@ void OTARequestor::mOnConnected(void * context, chip::DeviceProxy * deviceProxy) 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; diff --git a/src/app/clusters/ota-requestor/OTARequestor.h b/src/app/clusters/ota-requestor/OTARequestor.h index 692e0857f75f1b..a0755fe74ca956 100644 --- a/src/app/clusters/ota-requestor/OTARequestor.h +++ b/src/app/clusters/ota-requestor/OTARequestor.h @@ -97,11 +97,12 @@ class OTARequestor : public OTARequestorInterface VerifyOrReturnError(mExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE); chip::Messaging::SendFlags sendFlags; - if (event.msgTypeData.MessageType == static_cast(chip::bdx::MessageType::ReceiveInit)) + if (event.msgTypeData.HasMessageType(chip::bdx::MessageType::ReceiveInit)) { sendFlags.Set(chip::Messaging::SendMessageFlags::kFromInitiator); } - if (event.msgTypeData.MessageType != static_cast(chip::bdx::MessageType::BlockAckEOF)) + if (!event.msgTypeData.HasMessageType(chip::bdx::MessageType::BlockAckEOF) && + !event.msgTypeData.HasMessageType(chip::Protocols::SecureChannel::MsgType::StatusReport)) { sendFlags.Set(chip::Messaging::SendMessageFlags::kExpectResponse); } @@ -123,6 +124,7 @@ class OTARequestor : public OTARequestorInterface 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(); @@ -159,8 +161,8 @@ class OTARequestor : public OTARequestorInterface chip::CASESessionManager * mCASESessionManager = nullptr; OnConnectedState onConnectedState = kQueryImage; chip::Messaging::ExchangeContext * exchangeCtx = nullptr; - chip::BDXDownloader * mBdxDownloader; - BDXMessenger mBdxMessenger; + chip::BDXDownloader * mBdxDownloader; // TODO: this should be OTADownloader + BDXMessenger mBdxMessenger; // TODO: ideally this is held by the application // TODO: Temporary until IP address resolution is implemented in the Exchange layer chip::Inet::IPAddress mIpAddress; From db49c0e4aad17506e9d879b0e7ca457b6408b268 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Fri, 3 Dec 2021 15:10:03 -0800 Subject: [PATCH 5/7] fix some namespacing Co-authored-by: Carol Yang --- examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h index ccd8d20c9773d3..e36b78dc78abac 100644 --- a/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h +++ b/examples/ota-requestor-app/linux/LinuxOTAImageProcessor.h @@ -35,7 +35,7 @@ class LinuxOTAImageProcessor : public OTAImageProcessorInterface CHIP_ERROR Abort() override; CHIP_ERROR ProcessBlock(ByteSpan & block) override; - void SetOTADownloader(chip::OTADownloader * downloader) { mDownloader = downloader; } + void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; } private: //////////// Actual handlers for the OTAImageProcessorInterface /////////////// From ea9f33aac6c15a52a73cee8522fb3d92f13db32e Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Sat, 4 Dec 2021 00:36:46 +0000 Subject: [PATCH 6/7] remove unused OTADownloader reference --- examples/ota-requestor-app/linux/main.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 27542df409904c..fc004f357376ed 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -26,7 +26,6 @@ #include "LinuxOTAImageProcessor.h" #include "LinuxOTARequestorDriver.h" #include "app/clusters/ota-requestor/BDXDownloader.h" -#include "app/clusters/ota-requestor/OTADownloader.h" #include "app/clusters/ota-requestor/OTARequestor.h" using chip::BDXDownloader; @@ -39,7 +38,6 @@ using chip::LinuxOTAImageProcessor; using chip::NodeId; using chip::OnDeviceConnected; using chip::OnDeviceConnectionFailure; -using chip::OTADownloader; using chip::OTAImageProcessorParams; using chip::PeerId; using chip::Server; From 8cdc8fb92d5dd7a34927a710ba031e45d89e7520 Mon Sep 17 00:00:00 2001 From: Trevor Holbrook Date: Sat, 4 Dec 2021 01:52:49 +0000 Subject: [PATCH 7/7] update README --- src/app/clusters/ota-requestor/README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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