From 1691c64da2853a768da9eb2751cc81cfa166b1fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arkadiusz=20Ba=C5=82ys?= Date: Tue, 19 Apr 2022 15:37:53 +0200 Subject: [PATCH] [nrfconnect] Added more checks to OTA DFU multi-image process (#17397) OTA DFU methods need more checks to faster reaction to errors. --- .../nrfconnect/OTAImageProcessorImpl.cpp | 88 ++++++++++--------- .../nrfconnect/OTAImageProcessorImpl.h | 15 +++- 2 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/platform/nrfconnect/OTAImageProcessorImpl.cpp b/src/platform/nrfconnect/OTAImageProcessorImpl.cpp index fc4d49a9495172..90924b96493964 100644 --- a/src/platform/nrfconnect/OTAImageProcessorImpl.cpp +++ b/src/platform/nrfconnect/OTAImageProcessorImpl.cpp @@ -44,7 +44,6 @@ CHIP_ERROR OTAImageProcessorImpl::PrepareDownloadImpl() mHeaderParser.Init(); mContentHeaderParser.Init(); ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_mcuboot_set_buf(mBuffer, sizeof(mBuffer)))); - ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_reset())); return CHIP_NO_ERROR; } @@ -62,14 +61,14 @@ CHIP_ERROR OTAImageProcessorImpl::Abort() CHIP_ERROR OTAImageProcessorImpl::Apply() { int err = dfu_target_done(true); - if (err == 0) + if (!err) { // schedule update of all possible targets by caling this function with argument -1 err = dfu_target_schedule_update(-1); } #ifdef CONFIG_CHIP_OTA_REQUESTOR_REBOOT_ON_APPLY - if (err == 0) + if (!err) { return SystemLayer().StartTimer( System::Clock::Milliseconds32(CHIP_DEVICE_CONFIG_OTA_REQUESTOR_REBOOT_DELAY_MS), @@ -89,36 +88,46 @@ CHIP_ERROR OTAImageProcessorImpl::Apply() #endif } -CHIP_ERROR OTAImageProcessorImpl::ProcessBlock(ByteSpan & block) +CHIP_ERROR OTAImageProcessorImpl::SwitchToNextImage(const ByteSpan & aRemainingData) +{ + mCurrentImage.mFileInfo = &mContentHeader.mFiles[static_cast(ImageType::kNetImage)]; + mCurrentImage.mImageType = ImageType::kNetImage; + + if (mCurrentImage.mFileInfo->mFileSize > 0) + { + // finish app-core dfu target to inform mcuboot that all bytes were written + ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_done(true))); + // Reset app-core target to allow switching a target to the next one + ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_reset())); + // initialize next dfu target to store net-core image. + ReturnErrorOnFailure(System::MapErrorZephyr( + dfu_target_init(DFU_TARGET_IMAGE_TYPE_MCUBOOT, static_cast(mCurrentImage.mImageType), /* size */ 0, nullptr))); + // write remaining data to new image + ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_write(aRemainingData.data(), aRemainingData.size()))); + mCurrentImage.mCurrentOffset = aRemainingData.size(); + } + return CHIP_NO_ERROR; +} + +CHIP_ERROR OTAImageProcessorImpl::ProcessBlock(ByteSpan & aBlock) { VerifyOrReturnError(mDownloader != nullptr, CHIP_ERROR_INCORRECT_STATE); - CHIP_ERROR error = ProcessHeader(block); + CHIP_ERROR error = ProcessHeader(aBlock); if (error == CHIP_NO_ERROR) { - mCurrentImage.mCurrentOffset += block.size(); + mCurrentImage.mCurrentOffset += aBlock.size(); if (mCurrentImage.mCurrentOffset >= mCurrentImage.mFileInfo->mFileSize) { - // calculate how many data should be moved to the next image - uint64_t remainingDataSize = mCurrentImage.mCurrentOffset - static_cast(mCurrentImage.mFileInfo->mFileSize); + // create new subspan with data which should be moved to the next image + ByteSpan remainingData = aBlock.SubSpan( + aBlock.size() - (mCurrentImage.mCurrentOffset - static_cast(mCurrentImage.mFileInfo->mFileSize))); // write last data of previous image - error = System::MapErrorZephyr(dfu_target_write(block.data(), block.size() - remainingDataSize)); - // switch to net image - mCurrentImage.mIndex++; - mCurrentImage.mFileInfo = &mContentHeader.mFiles[mCurrentImage.mIndex]; - - if (OTAImageContentHeader::FileId::kNetMcuboot == mCurrentImage.mFileInfo->mFileId && - mCurrentImage.mFileInfo->mFileSize > 0 && CHIP_NO_ERROR == error) + error = System::MapErrorZephyr(dfu_target_write(aBlock.data(), aBlock.size() - remainingData.size())); + if (OTAImageContentHeader::FileId::kAppMcuboot == mCurrentImage.mFileInfo->mFileId && CHIP_NO_ERROR == error) { - // finish previous image and reset target - dfu_target_done(true); - dfu_target_reset(); - // initialize next dfu target to store net-core image. - dfu_target_init(DFU_TARGET_IMAGE_TYPE_MCUBOOT, mCurrentImage.mIndex, /* size */ 0, nullptr); - // write remaining data to new image - error = - System::MapErrorZephyr(dfu_target_write(block.data() + (block.size() - remainingDataSize), remainingDataSize)); - mCurrentImage.mCurrentOffset = remainingDataSize; + // switch to net image + error = SwitchToNextImage(remainingData); } else { @@ -129,17 +138,17 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessBlock(ByteSpan & block) else { // DFU target library buffers data internally, so do not clone the block data. - error = System::MapErrorZephyr(dfu_target_write(block.data(), block.size())); + error = System::MapErrorZephyr(dfu_target_write(aBlock.data(), aBlock.size())); } - ChipLogDetail(SoftwareUpdate, "Processed %llu/%u Bytes of image no. %u", mCurrentImage.mCurrentOffset, - mCurrentImage.mFileInfo->mFileSize, mCurrentImage.mIndex); } // Report the result back to the downloader asynchronously. - return DeviceLayer::SystemLayer().ScheduleLambda([this, error, block] { + return DeviceLayer::SystemLayer().ScheduleLambda([this, error, aBlock] { if (error == CHIP_NO_ERROR) { - mParams.downloadedBytes += block.size(); + mParams.downloadedBytes += aBlock.size(); + ChipLogDetail(SoftwareUpdate, "Processed %llu/%u Bytes of image no. %u", mCurrentImage.mCurrentOffset, + mCurrentImage.mFileInfo->mFileSize, static_cast(mCurrentImage.mImageType)); mDownloader->FetchNextData(); } else @@ -159,12 +168,12 @@ CHIP_ERROR OTAImageProcessorImpl::ConfirmCurrentImage() return System::MapErrorZephyr(boot_write_img_confirmed()); } -CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & block) +CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & aBlock) { if (mHeaderParser.IsInitialized()) { OTAImageHeader header; - CHIP_ERROR error = mHeaderParser.AccumulateAndDecode(block, header); + CHIP_ERROR error = mHeaderParser.AccumulateAndDecode(aBlock, header); // Needs more data to decode the header ReturnErrorCodeIf(error == CHIP_ERROR_BUFFER_TOO_SMALL, CHIP_NO_ERROR); @@ -174,9 +183,9 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & block) mHeaderParser.Clear(); } - if (mContentHeaderParser.IsInitialized() && !block.empty()) + if (mContentHeaderParser.IsInitialized() && !aBlock.empty()) { - CHIP_ERROR error = mContentHeaderParser.AccumulateAndDecode(block, mContentHeader); + CHIP_ERROR error = mContentHeaderParser.AccumulateAndDecode(aBlock, mContentHeader); // Needs more data to decode the header ReturnErrorCodeIf(error == CHIP_ERROR_BUFFER_TOO_SMALL, CHIP_NO_ERROR); @@ -184,12 +193,11 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & block) if (OTAImageContentHeader::FileId::kAppMcuboot == mContentHeader.mFiles[0].mFileId) { - mCurrentImage.mIndex = 0; - mCurrentImage.mFileInfo = &mContentHeader.mFiles[mCurrentImage.mIndex]; + mCurrentImage.mFileInfo = &mContentHeader.mFiles[static_cast(ImageType::kAppImage)]; + mCurrentImage.mImageType = ImageType::kAppImage; // Initialize dfu target to receive first image - error = - System::MapErrorZephyr(dfu_target_init(DFU_TARGET_IMAGE_TYPE_MCUBOOT, mCurrentImage.mIndex, /* size */ 0, nullptr)); - ReturnErrorOnFailure(error); + ReturnErrorOnFailure(System::MapErrorZephyr(dfu_target_init( + DFU_TARGET_IMAGE_TYPE_MCUBOOT, static_cast(mCurrentImage.mImageType), /* size */ 0, nullptr))); } mContentHeaderParser.Clear(); @@ -199,14 +207,14 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessHeader(ByteSpan & block) } // external flash power consumption optimization -void ExtFlashHandler::DoAction(Action action) +void ExtFlashHandler::DoAction(Action aAction) { #if CONFIG_PM_DEVICE && CONFIG_NORDIC_QSPI_NOR && !CONFIG_SOC_NRF52840 // nRF52 is optimized per default // utilize the QSPI driver sleep power mode const auto * qspi_dev = device_get_binding(DT_LABEL(DT_INST(0, nordic_qspi_nor))); if (qspi_dev) { - const auto requestedAction = Action::WAKE_UP == action ? PM_DEVICE_ACTION_RESUME : PM_DEVICE_ACTION_SUSPEND; + const auto requestedAction = Action::WAKE_UP == aAction ? PM_DEVICE_ACTION_RESUME : PM_DEVICE_ACTION_SUSPEND; (void) pm_device_action_run(qspi_dev, requestedAction); // not much can be done in case of a failure } #endif diff --git a/src/platform/nrfconnect/OTAImageProcessorImpl.h b/src/platform/nrfconnect/OTAImageProcessorImpl.h index 2e0507a5347422..8965c4d541209b 100644 --- a/src/platform/nrfconnect/OTAImageProcessorImpl.h +++ b/src/platform/nrfconnect/OTAImageProcessorImpl.h @@ -32,12 +32,18 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface public: static constexpr size_t kBufferSize = CONFIG_CHIP_OTA_REQUESTOR_BUFFER_SIZE; + enum class ImageType : uint8_t + { + kAppImage = 0, + kNetImage = 1 + }; + void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }; struct OTAImage { OTAImageContentHeader::FileInfo * mFileInfo; - uint8_t mIndex; + ImageType mImageType; uint64_t mCurrentOffset; }; @@ -45,13 +51,14 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface CHIP_ERROR Finalize() override; CHIP_ERROR Abort() override; CHIP_ERROR Apply() override; - CHIP_ERROR ProcessBlock(ByteSpan & block) override; + CHIP_ERROR ProcessBlock(ByteSpan & aBlock) override; bool IsFirstImageRun() override; CHIP_ERROR ConfirmCurrentImage() override; private: CHIP_ERROR PrepareDownloadImpl(); - CHIP_ERROR ProcessHeader(ByteSpan & block); + CHIP_ERROR ProcessHeader(ByteSpan & aBlock); + CHIP_ERROR SwitchToNextImage(const ByteSpan & aRemainingData); OTADownloader * mDownloader = nullptr; OTAImageHeaderParser mHeaderParser; @@ -70,7 +77,7 @@ class ExtFlashHandler SLEEP }; virtual ~ExtFlashHandler() {} - virtual void DoAction(Action action); + virtual void DoAction(Action aAction); }; class OTAImageProcessorImplPMDevice : public OTAImageProcessorImpl