Skip to content

Commit

Permalink
OTA Requestor and Provider fixes (#10037)
Browse files Browse the repository at this point in the history
* Fix for 9525: Close the BDX ExchangeContext when transfer is over

* Fix for 9478: Add configuration for sending "Busy" status and DelayedActionTime

Add the following command line options to the Provider Linux executable:
-q/--QueryImageBehavior <UpdateAvailable | Busy | UpdateNotAvailable>
          Status value in the Query Image Response
-d/--DelayedActionTimeSec <time>
        Value in seconds for the DelayedActionTime in the Query Image Response

* Address PR comments: Rename fields, add a log message.

Also revert to treating AckEOFReceived as an unexpected event on the Downloader

* Move gOtaFilepath declaration to be with the rest of CLI argument global variables

* Address review comment: Close the exchange in the additional error cases

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
2 people authored and pull[bot] committed Nov 5, 2021
1 parent 7f570de commit 1003540
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 14 deletions.
51 changes: 46 additions & 5 deletions examples/ota-provider-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,20 @@ using chip::Messaging::ExchangeManager;
// TODO: this should probably be done dynamically
constexpr chip::EndpointId kOtaProviderEndpoint = 0;

constexpr uint16_t kOptionFilepath = 'f';
const char * gOtaFilepath = nullptr;
constexpr uint16_t kOptionFilepath = 'f';
constexpr uint16_t kOptionQueryImageBehavior = 'q';
constexpr uint16_t kOptionDelayedActionTimeSec = 'd';

// Arbitrary BDX Transfer Params
constexpr uint32_t kMaxBdxBlockSize = 1024;
constexpr uint32_t kBdxTimeoutMs = 5 * 60 * 1000; // OTA Spec mandates >= 5 minutes
constexpr uint32_t kBdxPollFreqMs = 500;

// Global variables used for passing the CLI arguments to the OTAProviderExample object
OTAProviderExample::queryImageBehaviorType gQueryImageBehavior = OTAProviderExample::kRespondWithUpdateAvailable;
uint32_t gDelayedActionTimeSec = 0;
const char * gOtaFilepath = nullptr;

bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, const char * aName, const char * aValue)
{
bool retval = true;
Expand All @@ -75,6 +81,33 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
gOtaFilepath = aValue;
}
break;
case kOptionQueryImageBehavior:
if (aValue == NULL)
{
PrintArgError("%s: ERROR: NULL QueryImageBehavior parameter\n", aProgram);
retval = false;
}
else if (strcmp(aValue, "UpdateAvailable") == 0)
{
gQueryImageBehavior = OTAProviderExample::kRespondWithUpdateAvailable;
}
else if (strcmp(aValue, "Busy") == 0)
{
gQueryImageBehavior = OTAProviderExample::kRespondWithBusy;
}
else if (strcmp(aValue, "UpdateNotAvailable") == 0)
{
gQueryImageBehavior = OTAProviderExample::kRespondWithUpdateAvailable;
}
else
{
PrintArgError("%s: ERROR: Invalid QueryImageBehavior parameter: %s\n", aProgram, aValue);
retval = false;
}
break;
case kOptionDelayedActionTimeSec:
gDelayedActionTimeSec = static_cast<uint32_t>(strtol(aValue, NULL, 0));
break;
default:
PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", aProgram, aName);
retval = false;
Expand All @@ -86,13 +119,18 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,

OptionDef cmdLineOptionsDef[] = {
{ "filepath", chip::ArgParser::kArgumentRequired, kOptionFilepath },
{ "QueryImageBehavior", chip::ArgParser::kArgumentRequired, kOptionQueryImageBehavior },
{ "DelayedActionTimeSec", chip::ArgParser::kArgumentRequired, kOptionDelayedActionTimeSec },
{},
};

OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS",
" -f <file>\n"
" --filepath <file>\n"
" Path to a file containing an OTA image.\n" };
" -f/--filepath <file>\n"
" Path to a file containing an OTA image.\n"
" -q/--QueryImageBehavior <UpdateAvailable | Busy | UpdateNotAvailable>\n"
" Status value in the Query Image Response\n"
" -d/--DelayedActionTimeSec <time>\n"
" Value in seconds for the DelayedActionTime in the Query Image Response\n" };

HelpOptions helpOptions("ota-provider-app", "Usage: ota-provider-app [options]", "1.0");

Expand Down Expand Up @@ -143,6 +181,9 @@ int main(int argc, char * argv[])
bdxServer.SetFilepath(gOtaFilepath);
}

otaProvider.SetQueryImageBehavior(gQueryImageBehavior);
otaProvider.SetDelayedActionTimeSec(gDelayedActionTimeSec);

chip::app::clusters::OTAProvider::SetDelegate(kOtaProviderEndpoint, &otaProvider);

BitFlags<TransferControlFlags> bdxFlags;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ bool GenerateBdxUri(const Span<char> & fileDesignator, Span<char> outUri, size_t
OTAProviderExample::OTAProviderExample()
{
memset(mOTAFilePath, 0, kFilepathBufLen);
mQueryImageBehavior = kRespondWithNotAvailable;
mDelayedActionTimeSec = 0;
}

void OTAProviderExample::SetOTAFilePath(const char * path)
Expand All @@ -102,11 +104,9 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
{
// TODO: add confiuration for returning BUSY status

EmberAfOTAQueryStatus queryStatus =
(strlen(mOTAFilePath) ? EMBER_ZCL_OTA_QUERY_STATUS_UPDATE_AVAILABLE : EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE);
uint32_t delayedActionTimeSec = 0;
uint32_t softwareVersion = currentVersion + 1; // This implementation will always indicate that an update is available
// (if the user provides a file).
EmberAfOTAQueryStatus queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
uint32_t softwareVersion = currentVersion + 1; // This implementation will always indicate that an update is available
// (if the user provides a file).
bool userConsentNeeded = false;
uint8_t updateToken[kUpdateTokenLen] = { 0 };
char strBuf[kUpdateTokenStrLen] = { 0 };
Expand All @@ -123,14 +123,41 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
ChipLogDetail(SoftwareUpdate, "generated URI: %s", uriBuf);
}

// Set Status for the Query Image Response
switch (mQueryImageBehavior)
{
case kRespondWithUpdateAvailable: {
if (strlen(mOTAFilePath) != 0)
{
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_UPDATE_AVAILABLE;
}
else
{
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
ChipLogError(SoftwareUpdate, "No OTA file configured on the Provider");
}
break;
}
case kRespondWithBusy: {
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_BUSY;
break;
}
case kRespondWithNotAvailable: {
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
break;
}
default:
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
}

CommandPathParams cmdParams = { emberAfCurrentEndpoint(), 0 /* mGroupId */, ZCL_OTA_PROVIDER_CLUSTER_ID,
ZCL_QUERY_IMAGE_RESPONSE_COMMAND_ID, (CommandPathFlags::kEndpointIdValid) };
TLVWriter * writer = nullptr;
uint8_t tagNum = 0;
VerifyOrReturnError((commandObj->PrepareCommand(cmdParams) == CHIP_NO_ERROR), EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError((writer = commandObj->GetCommandDataElementTLVWriter()) != nullptr, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(tagNum++), queryStatus) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(tagNum++), delayedActionTimeSec) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(tagNum++), mDelayedActionTimeSec) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->PutString(ContextTag(tagNum++), uriBuf) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(tagNum++), softwareVersion) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->PutBytes(ContextTag(tagNum++), updateToken, kUpdateTokenLen) == CHIP_NO_ERROR,
Expand All @@ -147,10 +174,8 @@ EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHan
const chip::ByteSpan & updateToken, uint32_t newVersion)
{
// TODO: handle multiple transfers by tracking updateTokens
// TODO: add configuration for sending different updateAction and delayedActionTime values

EmberAfOTAApplyUpdateAction updateAction = EMBER_ZCL_OTA_APPLY_UPDATE_ACTION_PROCEED; // For now, just allow any update request
uint32_t delayedActionTimeSec = 0;
char tokenBuf[kUpdateTokenStrLen] = { 0 };

GetUpdateTokenString(updateToken, tokenBuf, kUpdateTokenStrLen);
Expand All @@ -165,7 +190,7 @@ EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHan
VerifyOrReturnError((commandObj->PrepareCommand(cmdParams) == CHIP_NO_ERROR), EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError((writer = commandObj->GetCommandDataElementTLVWriter()) != nullptr, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(0), updateAction) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(1), delayedActionTimeSec) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError(writer->Put(ContextTag(1), mDelayedActionTimeSec) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
VerifyOrReturnError((commandObj->FinishCommand() == CHIP_NO_ERROR), EMBER_ZCL_STATUS_FAILURE);

return EMBER_ZCL_STATUS_SUCCESS;
Expand Down
11 changes: 11 additions & 0 deletions examples/ota-provider-app/ota-provider-common/OTAProviderExample.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,18 @@ class OTAProviderExample : public OTAProviderDelegate
uint32_t newVersion) override;
EmberAfStatus HandleNotifyUpdateApplied(const chip::ByteSpan & updateToken, uint32_t currentVersion) override;

enum queryImageBehaviorType
{
kRespondWithUpdateAvailable,
kRespondWithBusy,
kRespondWithNotAvailable
};
void SetQueryImageBehavior(queryImageBehaviorType behavior) { mQueryImageBehavior = behavior; }
void SetDelayedActionTimeSec(uint32_t time) { mDelayedActionTimeSec = time; }

private:
static constexpr size_t kFilepathBufLen = 256;
char mOTAFilePath[kFilepathBufLen]; // null-terminated
queryImageBehaviorType mQueryImageBehavior;
uint32_t mDelayedActionTimeSec;
};
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,17 @@ void BdxDownloader::HandleTransferSessionOutput(TransferSession::OutputEvent & e
case TransferSession::OutputEventType::kStatusReceived:
ChipLogError(BDX, "Got StatusReport %x", static_cast<uint16_t>(event.statusData.statusCode));
mTransfer.Reset();
mExchangeCtx->Close();
break;
case TransferSession::OutputEventType::kInternalError:
ChipLogError(BDX, "InternalError");
mTransfer.Reset();
mExchangeCtx->Close();
break;
case TransferSession::OutputEventType::kTransferTimeout:
ChipLogError(BDX, "Transfer timed out");
mTransfer.Reset();
mExchangeCtx->Close();
break;
case TransferSession::OutputEventType::kInitReceived:
case TransferSession::OutputEventType::kAckReceived:
Expand Down

0 comments on commit 1003540

Please sign in to comment.