Skip to content

Commit

Permalink
[ota] Remove two outdated/redundant methods (#16019)
Browse files Browse the repository at this point in the history
* [ota] Move test requestor feature to the application

* [ota] Remove SetOTAImageProcessorParams

The function was initially used to set name of image file
to be downloaded, but most platforms don't use that and
some platforms use the file name as the output file name,
that is, the file name where the downloaded image should
be saved.

Remove the method from the common interface and add
SetOTAImageFile setters in platforms that need to have
the output file name set.
  • Loading branch information
Damian-Nordic authored and pull[bot] committed Dec 14, 2023
1 parent 04930e6 commit 5449393
Show file tree
Hide file tree
Showing 17 changed files with 39 additions and 74 deletions.
6 changes: 0 additions & 6 deletions examples/all-clusters-app/ameba/main/chipinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,6 @@ static void InitOTARequestor(void)
// Set server instance used for session establishment
gRequestorCore.Init(chip::Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);

// 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;
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the Downloader and Image Processor objects
Expand Down
6 changes: 0 additions & 6 deletions examples/lighting-app/ameba/main/chipinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,6 @@ static void InitOTARequestor(void)
// Set server instance used for session establishment
gRequestorCore.Init(Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);

// 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;
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the Downloader and Image Processor objects
Expand Down
8 changes: 1 addition & 7 deletions examples/lighting-app/nxp/k32w/k32w0/main/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,7 @@ CHIP_ERROR AppTask::Init()
gRequestorCore.Init(Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);
gRequestorUser.Init(&gRequestorCore, &gImageProcessor);

// 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");
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTAImageFile(CharSpan("test.txt"));
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the gDownloader and Image Processor objects
Expand Down
7 changes: 0 additions & 7 deletions examples/ota-requestor-app/ameba/main/chipinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ using chip::NodeId;
using chip::OnDeviceConnected;
using chip::OnDeviceConnectionFailure;
using chip::OTADownloader;
using chip::OTAImageProcessorParams;
using chip::OTARequestor;
using chip::PeerId;
using chip::Server;
Expand Down Expand Up @@ -107,12 +106,6 @@ static void InitOTARequestor(void)
// Set server instance used for session establishment
gRequestorCore.Init(chip::Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);

// 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;
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the Downloader and Image Processor objects
Expand Down
4 changes: 1 addition & 3 deletions examples/ota-requestor-app/efr32/src/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,7 @@ void AppTask::InitOTARequestor()

gRequestorUser.Init(&gRequestorCore, &gImageProcessor);

OTAImageProcessorParams ipParams;
ipParams.imageFile = CharSpan("test.txt");
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTAImageFile(CharSpan("test.txt"));
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the Downloader and Image Processor objects
Expand Down
28 changes: 15 additions & 13 deletions examples/ota-requestor-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ using chip::OnDeviceConnected;
using chip::OnDeviceConnectionFailure;
using chip::OTADownloader;
using chip::OTAImageProcessorImpl;
using chip::OTAImageProcessorParams;
using chip::OTARequestor;
using chip::PeerId;
using chip::Server;
Expand All @@ -48,9 +47,15 @@ using namespace chip::ArgParser;
using namespace chip::Messaging;
using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands;

class CustomOTARequestorDriver : public DeviceLayer::ExtendedOTARequestorDriver
{
public:
bool CanConsent() override;
};

OTARequestor gRequestorCore;
DefaultOTARequestorStorage gRequestorStorage;
DeviceLayer::ExtendedOTARequestorDriver gRequestorUser;
CustomOTARequestorDriver gRequestorUser;
BDXDownloader gDownloader;
OTAImageProcessorImpl gImageProcessor;
chip::ota::DefaultOTARequestorUserConsentProvider gUserConsentProvider;
Expand Down Expand Up @@ -81,7 +86,8 @@ OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS"
" Periodic timeout for querying providers in the default OTA provider list\n"
" If none or zero is supplied the timeout is set to every 24 hours. \n"
" -c/--requestorCanConsent\n"
" If supplied, the RequestorCanConsent field of the QueryImage command is set to true.\n"
" If supplied, the RequestorCanConsent field of the QueryImage command is set to "
"true.\n"
" Otherwise, the value is determined by the driver.\n "
" -f/--otaDownloadPath <file path>\n"
" If supplied, the OTA image is downloaded to the given fully-qualified file-path.\n"
Expand All @@ -93,6 +99,11 @@ OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS"

OptionSet * allOptions[] = { &cmdLineOptions, nullptr };

bool CustomOTARequestorDriver::CanConsent()
{
return gRequestorCanConsent.ValueOr(DeviceLayer::ExtendedOTARequestorDriver::CanConsent());
}

static void InitOTARequestor(void)
{
// Set the global instance of the OTA requestor core component
Expand All @@ -105,11 +116,7 @@ static void InitOTARequestor(void)
gRequestorCore.Init(chip::Server::GetInstance(), gRequestorStorage, gRequestorUser, gDownloader);
gRequestorUser.Init(&gRequestorCore, &gImageProcessor);

// 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.
OTAImageProcessorParams ipParams;
ipParams.imageFile = CharSpan::fromCharString(gOtaDownloadPath);
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTAImageFile(CharSpan::fromCharString(gOtaDownloadPath));
gImageProcessor.SetOTADownloader(&gDownloader);

// Set the image processor instance used for handling image being downloaded
Expand Down Expand Up @@ -174,11 +181,6 @@ void ApplicationInit()
{
chip::Dnssd::Resolver::Instance().Init(chip::DeviceLayer::UDPEndPointManager());

if (gRequestorCanConsent.HasValue())
{
gRequestorCore.SetRequestorCanConsent(gRequestorCanConsent.Value());
}

// Initialize all OTA download components
InitOTARequestor();
}
Expand Down
1 change: 0 additions & 1 deletion examples/ota-requestor-app/p6/src/AppTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ using chip::GetRequestorInstance;
using chip::NodeId;
using chip::OTADownloader;
using chip::OTAImageProcessorImpl;
using chip::OTAImageProcessorParams;
using chip::OTARequestor;
using chip::System::Layer;

Expand Down
4 changes: 1 addition & 3 deletions examples/platform/efr32/OTAConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ void OTAConfig::Init()

gRequestorUser.Init(&gRequestorCore, &gImageProcessor);

chip::OTAImageProcessorParams ipParams;
ipParams.imageFile = chip::CharSpan("test.txt");
gImageProcessor.SetOTAImageProcessorParams(ipParams);
gImageProcessor.SetOTAImageFile(chip::CharSpan("test.txt"));
gImageProcessor.SetOTADownloader(&gDownloader);

// Connect the Downloader and Image Processor objects
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/ota-requestor/OTARequestor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ CHIP_ERROR OTARequestor::SendQueryImageRequest(OperationalDeviceProxy & devicePr
ReturnErrorOnFailure(DeviceLayer::ConfigurationMgr().GetSoftwareVersion(args.softwareVersion));

args.protocolsSupported = kProtocolsSupported;
args.requestorCanConsent.SetValue(mRequestorCanConsent.ValueOr(mOtaRequestorDriver->CanConsent()));
args.requestorCanConsent.SetValue(mOtaRequestorDriver->CanConsent());

uint16_t hardwareVersion;
if (DeviceLayer::ConfigurationMgr().GetHardwareVersion(hardwareVersion) == CHIP_NO_ERROR)
Expand Down
6 changes: 0 additions & 6 deletions src/app/clusters/ota-requestor/OTARequestor.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,6 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
reinterpret_cast<intptr_t>(this));
}

/**
* Called to set optional requestorCanConsent value provided by Requestor.
*/
void SetRequestorCanConsent(bool requestorCanConsent) { mRequestorCanConsent.SetValue(requestorCanConsent); }

private:
using QueryImageResponseDecodableType = app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImageResponse::DecodableType;
using ApplyUpdateResponseDecodableType = app::Clusters::OtaSoftwareUpdateProvider::Commands::ApplyUpdateResponse::DecodableType;
Expand Down Expand Up @@ -314,7 +309,6 @@ class OTARequestor : public OTARequestorInterface, public BDXDownloader::StateDe
CharSpan mFileDesignator;
OTAUpdateStateEnum mCurrentUpdateState = OTAUpdateStateEnum::kUnknown;
Server * mServer = nullptr;
chip::Optional<bool> mRequestorCanConsent;
ProviderLocationList mDefaultOtaProviderList;
Optional<ProviderLocationType> mProviderLocation; // Provider location used for the current update in progress
};
Expand Down
10 changes: 2 additions & 8 deletions src/include/platform/OTAImageProcessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@

namespace chip {

struct OTAImageProcessorParams
struct OTAImageProgress
{
CharSpan imageFile;
uint64_t downloadedBytes = 0;
uint64_t totalFileBytes = 0;
};
Expand Down Expand Up @@ -76,11 +75,6 @@ class DLL_EXPORT OTAImageProcessorInterface
*/
virtual CHIP_ERROR ProcessBlock(ByteSpan & block) = 0;

/**
* Called to setup params for the OTA image download
*/
virtual void SetOTAImageProcessorParams(OTAImageProcessorParams & params) { mParams = params; };

/**
* Called to check the current download status of the OTA image download.
*/
Expand All @@ -97,7 +91,7 @@ class DLL_EXPORT OTAImageProcessorInterface
virtual uint64_t GetBytesDownloaded() { return mParams.downloadedBytes; }

protected:
OTAImageProcessorParams mParams;
OTAImageProgress mParams;
};

} // namespace chip
6 changes: 3 additions & 3 deletions src/platform/EFR32/OTAImageProcessorImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ uint32_t OTAImageProcessorImpl::mWriteOffset;

CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand Down Expand Up @@ -80,7 +80,7 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()

CHIP_ERROR OTAImageProcessorImpl::Abort()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand Down Expand Up @@ -143,7 +143,7 @@ void OTAImageProcessorImpl::HandleFinalize(intptr_t context)

imageProcessor->ReleaseBlock();

ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mParams.imageFile.data());
ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mImageFile.data());
}

void OTAImageProcessorImpl::HandleAbort(intptr_t context)
Expand Down
2 changes: 2 additions & 0 deletions src/platform/EFR32/OTAImageProcessorImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
CHIP_ERROR ProcessBlock(ByteSpan & block) override;

void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
void SetOTAImageFile(CharSpan name) { mImageFile = name; }

private:
//////////// Actual handlers for the OTAImageProcessorInterface ///////////////
Expand All @@ -59,6 +60,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
static uint8_t mSlotId; // Bootloader storage slot
MutableByteSpan mBlock;
OTADownloader * mDownloader;
CharSpan mImageFile;
};

} // namespace chip
11 changes: 5 additions & 6 deletions src/platform/Linux/OTAImageProcessorImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace chip {

CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand All @@ -49,7 +49,7 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()

CHIP_ERROR OTAImageProcessorImpl::Abort()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand Down Expand Up @@ -97,8 +97,7 @@ void OTAImageProcessorImpl::HandlePrepareDownload(intptr_t context)
}

imageProcessor->mHeaderParser.Init();
imageProcessor->mOfs.open(imageProcessor->mParams.imageFile.data(),
std::ofstream::out | std::ofstream::ate | std::ofstream::app);
imageProcessor->mOfs.open(imageProcessor->mImageFile.data(), std::ofstream::out | std::ofstream::ate | std::ofstream::app);
if (!imageProcessor->mOfs.good())
{
imageProcessor->mDownloader->OnPreparedForDownload(CHIP_ERROR_OPEN_FAILED);
Expand All @@ -121,7 +120,7 @@ void OTAImageProcessorImpl::HandleFinalize(intptr_t context)
imageProcessor->mOfs.close();
imageProcessor->ReleaseBlock();

ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mParams.imageFile.data());
ChipLogProgress(SoftwareUpdate, "OTA image downloaded to %s", imageProcessor->mImageFile.data());
}

void OTAImageProcessorImpl::HandleApply(intptr_t context)
Expand Down Expand Up @@ -149,7 +148,7 @@ void OTAImageProcessorImpl::HandleAbort(intptr_t context)
}

imageProcessor->mOfs.close();
remove(imageProcessor->mParams.imageFile.data());
remove(imageProcessor->mImageFile.data());
imageProcessor->ReleaseBlock();
}

Expand Down
2 changes: 2 additions & 0 deletions src/platform/Linux/OTAImageProcessorImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
CHIP_ERROR ProcessBlock(ByteSpan & block) override;

void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
void SetOTAImageFile(CharSpan name) { mImageFile = name; }

private:
//////////// Actual handlers for the OTAImageProcessorInterface ///////////////
Expand All @@ -64,6 +65,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
OTADownloader * mDownloader;
OTAImageHeaderParser mHeaderParser;
uint32_t mSoftwareVersion;
CharSpan mImageFile;
};

} // namespace chip
8 changes: 4 additions & 4 deletions src/platform/nxp/k32w/k32w0/OTAImageProcessorImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace chip {

CHIP_ERROR OTAImageProcessorImpl::PrepareDownload()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand All @@ -51,7 +51,7 @@ CHIP_ERROR OTAImageProcessorImpl::Apply()

CHIP_ERROR OTAImageProcessorImpl::Abort()
{
if (mParams.imageFile.empty())
if (mImageFile.empty())
{
ChipLogError(SoftwareUpdate, "Invalid output image file supplied");
return CHIP_ERROR_INTERNAL;
Expand Down Expand Up @@ -95,7 +95,7 @@ void OTAImageProcessorImpl::HandlePrepareDownload(intptr_t context)

if (gOtaSuccess_c == OTA_ClientInit())
{
if (gOtaSuccess_c == OTA_StartImage(imageProcessor->mParams.imageFile.size()))
if (gOtaSuccess_c == OTA_StartImage(imageProcessor->mImageFile.size()))
{
imageProcessor->mDownloader->OnPreparedForDownload(CHIP_NO_ERROR);
}
Expand All @@ -110,7 +110,7 @@ void OTAImageProcessorImpl::HandleAbort(intptr_t context)
return;
}

remove(imageProcessor->mParams.imageFile.data());
remove(imageProcessor->mImageFile.data());
imageProcessor->ReleaseBlock();
}

Expand Down
2 changes: 2 additions & 0 deletions src/platform/nxp/k32w/k32w0/OTAImageProcessorImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface
CHIP_ERROR ProcessBlock(ByteSpan & block) override;

void SetOTADownloader(OTADownloader * downloader) { mDownloader = downloader; }
void SetOTAImageFile(CharSpan name) { mImageFile = name; }

private:
//////////// Actual handlers for the OTAImageProcessorInterface ///////////////
Expand All @@ -55,6 +56,7 @@ class OTAImageProcessorImpl : public OTAImageProcessorInterface

OTADownloader * mDownloader;
MutableByteSpan mBlock;
CharSpan mImageFile;
};

} // namespace chip

0 comments on commit 5449393

Please sign in to comment.