Skip to content

Commit

Permalink
[nrfconnect] Added more checks to OTA DFU multi-image process (#17397)
Browse files Browse the repository at this point in the history 
OTA DFU methods need more checks to faster reaction to errors.
  • Loading branch information
@ArekBalysNordic @pull
ArekBalysNordic authored and pull[bot] committed Sep 6, 2023
1 parent 604e51e commit 1691c64
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 44 deletions.
88 changes: 48 additions & 40 deletions src/platform/nrfconnect/OTAImageProcessorImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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),
Expand All @@ -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<uint8_t>(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<uint8_t>(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<uint64_t>(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<uint64_t>(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
{
Expand All @@ -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<uint8_t>(mCurrentImage.mImageType));
mDownloader->FetchNextData();
}
else
Expand All @@ -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);
Expand All @@ -174,22 +183,21 @@ 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);
ReturnErrorOnFailure(error);

if (OTAImageContentHeader::FileId::kAppMcuboot == mContentHeader.mFiles[0].mFileId)
{
mCurrentImage.mIndex = 0;
mCurrentImage.mFileInfo = &mContentHeader.mFiles[mCurrentImage.mIndex];
mCurrentImage.mFileInfo = &mContentHeader.mFiles[static_cast<int8_t>(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<uint8_t>(mCurrentImage.mImageType), /* size */ 0, nullptr)));
}

mContentHeaderParser.Clear();
Expand All @@ -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
Expand Down
15 changes: 11 additions & 4 deletions src/platform/nrfconnect/OTAImageProcessorImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,33 @@ 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;
};

CHIP_ERROR PrepareDownload() override;
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;
Expand All @@ -70,7 +77,7 @@ class ExtFlashHandler
SLEEP
};
virtual ~ExtFlashHandler() {}
virtual void DoAction(Action action);
virtual void DoAction(Action aAction);
};

class OTAImageProcessorImplPMDevice : public OTAImageProcessorImpl
Expand Down

0 comments on commit 1691c64

Please sign in to comment.