Skip to content

Commit

Permalink
Align QueryImage and QueryImageResponse XML with the spec (#11336)
Browse files Browse the repository at this point in the history 
* Use type-safe InvokeCommand to do QueryImage in OTA requestor app.

* Fully align the XML for QueryImage and QueryImageResponse with the spec.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Apr 13, 2022
1 parent a7e1f3f commit 2392136
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 114 deletions.
51 changes: 24 additions & 27 deletions examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ using chip::CharSpan;
using chip::Optional;
using chip::Span;
using chip::app::Clusters::OTAProviderDelegate;
using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands;

constexpr uint8_t kUpdateTokenLen = 32; // must be between 8 and 32
constexpr uint8_t kUpdateTokenStrLen = kUpdateTokenLen * 2 + 1; // Hex string needs 2 hex chars for every byte
Expand Down Expand Up @@ -94,17 +95,14 @@ void OTAProviderExample::SetOTAFilePath(const char * path)
}

EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath, uint16_t vendorId,
uint16_t productId, uint32_t softwareVersion, uint8_t protocolsSupported,
const Optional<uint16_t> & hardwareVersion, const Optional<CharSpan> & location,
const Optional<bool> & requestorCanConsent,
const Optional<ByteSpan> & metadataForProvider)
const chip::app::ConcreteCommandPath & commandPath,
const QueryImage::DecodableType & commandData)
{
// TODO: add confiuration for returning BUSY status

EmberAfOTAQueryStatus queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
uint32_t newSoftwareVersion = softwareVersion + 1; // This implementation will always indicate that an update is available
// (if the user provides a file).
uint32_t newSoftwareVersion = commandData.softwareVersion + 1; // This implementation will always indicate that an update is
// available (if the user provides a file).
constexpr char kExampleSoftwareString[] = "Example-Image-V0.1";
bool userConsentNeeded = false;
uint8_t updateToken[kUpdateTokenLen] = { 0 };
Expand Down Expand Up @@ -149,52 +147,51 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
queryStatus = EMBER_ZCL_OTA_QUERY_STATUS_NOT_AVAILABLE;
}

chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImageResponse::Type response;
response.status = queryStatus;
response.delayedActionTime = mDelayedActionTimeSec;
response.imageURI = chip::CharSpan(uriBuf, strlen(uriBuf));
response.softwareVersion = newSoftwareVersion;
response.softwareVersionString = chip::CharSpan(kExampleSoftwareString, strlen(kExampleSoftwareString));
response.updateToken = chip::ByteSpan(updateToken);
response.userConsentNeeded = userConsentNeeded;
// TODO: Once our client is using APIs that handle optional arguments
// correctly, update QueryImageResponse to have the right things optional.
// At that point we can decide whether to send metadataForRequestor as an
// empty ByteSpan or whether to not send it at all.
response.metadataForRequestor = chip::ByteSpan();
QueryImageResponse::Type response;
response.status = queryStatus;
response.delayedActionTime.Emplace(mDelayedActionTimeSec);
response.imageURI.Emplace(chip::CharSpan(uriBuf, strlen(uriBuf)));
response.softwareVersion.Emplace(newSoftwareVersion);
response.softwareVersionString.Emplace(chip::CharSpan(kExampleSoftwareString, strlen(kExampleSoftwareString)));
response.updateToken.Emplace(chip::ByteSpan(updateToken));
response.userConsentNeeded.Emplace(userConsentNeeded);
// Could also just not send metadataForRequestor at all.
response.metadataForRequestor.Emplace(chip::ByteSpan());

VerifyOrReturnError(commandObj->AddResponseData(commandPath, response) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus OTAProviderExample::HandleApplyUpdateRequest(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath,
const ByteSpan & updateToken, uint32_t newVersion)
const ApplyUpdateRequest::DecodableType & commandData)
{
// TODO: handle multiple transfers by tracking updateTokens

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

GetUpdateTokenString(updateToken, tokenBuf, kUpdateTokenStrLen);
ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, newVersion);
GetUpdateTokenString(commandData.updateToken, tokenBuf, kUpdateTokenStrLen);
ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, commandData.newVersion);

VerifyOrReturnError(commandObj != nullptr, EMBER_ZCL_STATUS_INVALID_VALUE);

chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::ApplyUpdateResponse::Type response;
ApplyUpdateResponse::Type response;
response.action = updateAction;
response.delayedActionTime = mDelayedActionTimeSec;
VerifyOrReturnError(commandObj->AddResponseData(commandPath, response) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);

return EMBER_ZCL_STATUS_SUCCESS;
}

EmberAfStatus OTAProviderExample::HandleNotifyUpdateApplied(const chip::ByteSpan & updateToken, uint32_t softwareVersion)
EmberAfStatus OTAProviderExample::HandleNotifyUpdateApplied(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath,
const NotifyUpdateApplied::DecodableType & commandData)
{
char tokenBuf[kUpdateTokenStrLen] = { 0 };

GetUpdateTokenString(updateToken, tokenBuf, kUpdateTokenStrLen);
ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, softwareVersion);
GetUpdateTokenString(commandData.updateToken, tokenBuf, kUpdateTokenStrLen);
ChipLogDetail(SoftwareUpdate, "%s: token: %s, version: %" PRIu32, __FUNCTION__, tokenBuf, commandData.softwareVersion);

emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS);

Expand Down
19 changes: 9 additions & 10 deletions examples/ota-provider-app/ota-provider-common/OTAProviderExample.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate
void SetOTAFilePath(const char * path);

// Inherited from OTAProviderDelegate
EmberAfStatus HandleQueryImage(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
uint16_t vendorId, uint16_t productId, uint32_t softwareVersion, uint8_t protocolsSupported,
const chip::Optional<uint16_t> & hardwareVersion,
const chip::Optional<chip::CharSpan> & location,
const chip::Optional<bool> & requestorCanConsent,
const chip::Optional<chip::ByteSpan> & metadataForServer) override;
EmberAfStatus HandleApplyUpdateRequest(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath, const chip::ByteSpan & updateToken,
uint32_t newVersion) override;
EmberAfStatus HandleNotifyUpdateApplied(const chip::ByteSpan & updateToken, uint32_t softwareVersion) override;
EmberAfStatus HandleQueryImage(
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) override;
EmberAfStatus HandleApplyUpdateRequest(
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) override;
EmberAfStatus HandleNotifyUpdateApplied(
chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData) override;

enum queryImageBehaviorType
{
Expand Down
46 changes: 23 additions & 23 deletions examples/ota-requestor-app/linux/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

#include <app-common/zap-generated/callback.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app/device/OperationalDeviceProxy.h>
#include <app/server/Server.h>
#include <credentials/examples/DeviceAttestationCredsExample.h>
Expand All @@ -43,11 +44,10 @@ using chip::Transport::PeerAddress;
using namespace chip::ArgParser;
using namespace chip::Messaging;
using namespace chip::app::device;
using namespace chip::app::Clusters::OtaSoftwareUpdateProvider::Commands;

void OnQueryImageResponse(void * context, uint8_t status, uint32_t delayedActionTime, CharSpan imageURI, uint32_t softwareVersion,
CharSpan softwareVersionString, ByteSpan updateToken, bool userConsentNeeded,
ByteSpan metadataForRequestor);
void OnQueryImageFailure(void * context, uint8_t status);
void OnQueryImageResponse(void * context, const QueryImageResponse::DecodableType & response);
void OnQueryImageFailure(void * context, EmberAfStatus status);
void OnConnected(void * context, OperationalDeviceProxy * operationalDeviceProxy);
void OnConnectionFailure(void * context, OperationalDeviceProxy * operationalDeviceProxy, CHIP_ERROR error);
bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier, const char * aName, const char * aValue);
Expand All @@ -56,8 +56,6 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
OperationalDeviceProxy gOperationalDeviceProxy;
ExchangeContext * exchangeCtx = nullptr;
BdxDownloader bdxDownloader;
Callback<OtaSoftwareUpdateProviderClusterQueryImageResponseCallback> mQueryImageResponseCallback(OnQueryImageResponse, nullptr);
Callback<DefaultFailureCallback> mOnQueryFailureCallback(OnQueryImageFailure, nullptr);
Callback<OnOperationalDeviceConnected> mOnConnectedCallback(OnConnected, nullptr);
Callback<OnOperationalDeviceConnectionFailure> mOnConnectionFailureCallback(OnConnectionFailure, nullptr);

Expand Down Expand Up @@ -108,11 +106,9 @@ HelpOptions helpOptions("ota-requestor-app", "Usage: ota-requestor-app [options]

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

void OnQueryImageResponse(void * context, uint8_t status, uint32_t delayedActionTime, CharSpan imageURI, uint32_t softwareVersion,
CharSpan softwareVersionString, ByteSpan updateToken, bool userConsentNeeded,
ByteSpan metadataForRequestor)
void OnQueryImageResponse(void * context, const QueryImageResponse::DecodableType & response)
{
ChipLogDetail(SoftwareUpdate, "QueryImageResponse responded with action %" PRIu8, status);
ChipLogDetail(SoftwareUpdate, "QueryImageResponse responded with action %" PRIu8, response.status);

TransferSession::TransferInitData initOptions;
initOptions.TransferCtlFlags = chip::bdx::TransferControlFlags::kReceiverDrive;
Expand Down Expand Up @@ -143,7 +139,7 @@ void OnQueryImageResponse(void * context, uint8_t status, uint32_t delayedAction
chip::System::Clock::Seconds16(20));
}

void OnQueryImageFailure(void * context, uint8_t status)
void OnQueryImageFailure(void * context, EmberAfStatus status)
{
ChipLogDetail(SoftwareUpdate, "QueryImage failure response %" PRIu8, status);
}
Expand All @@ -154,17 +150,13 @@ void OnConnected(void * context, OperationalDeviceProxy * operationalDeviceProxy
chip::Controller::OtaSoftwareUpdateProviderCluster cluster;
constexpr EndpointId kOtaProviderEndpoint = 0;

chip::Callback::Cancelable * successCallback = mQueryImageResponseCallback.Cancel();
chip::Callback::Cancelable * failureCallback = mOnQueryFailureCallback.Cancel();

// These QueryImage params have been chosen arbitrarily
constexpr VendorId kExampleVendorId = VendorId::Common;
constexpr uint16_t kExampleProductId = 77;
constexpr uint16_t kExampleHWVersion = 3;
constexpr uint16_t kExampleSoftwareVersion = 0;
constexpr uint8_t kExampleProtocolsSupported =
EMBER_ZCL_OTA_DOWNLOAD_PROTOCOL_BDX_SYNCHRONOUS; // TODO: support this as a list once ember adds list support
const char locationBuf[] = { 'U', 'S' };
constexpr VendorId kExampleVendorId = VendorId::Common;
constexpr uint16_t kExampleProductId = 77;
constexpr uint16_t kExampleHWVersion = 3;
constexpr uint16_t kExampleSoftwareVersion = 0;
constexpr EmberAfOTADownloadProtocol kExampleProtocolsSupported[] = { EMBER_ZCL_OTA_DOWNLOAD_PROTOCOL_BDX_SYNCHRONOUS };
const char locationBuf[] = { 'U', 'S' };
CharSpan exampleLocation(locationBuf);
constexpr bool kExampleClientCanConsent = false;
ByteSpan metadata;
Expand All @@ -175,8 +167,16 @@ void OnConnected(void * context, OperationalDeviceProxy * operationalDeviceProxy
ChipLogError(SoftwareUpdate, "Associate() failed: %" CHIP_ERROR_FORMAT, err.Format());
return;
}
err = cluster.QueryImage(successCallback, failureCallback, kExampleVendorId, kExampleProductId, kExampleSoftwareVersion,
kExampleProtocolsSupported, kExampleHWVersion, exampleLocation, kExampleClientCanConsent, metadata);
QueryImage::Type args;
args.vendorId = kExampleVendorId;
args.productId = kExampleProductId;
args.softwareVersion = kExampleSoftwareVersion;
args.protocolsSupported = kExampleProtocolsSupported;
args.hardwareVersion.Emplace(kExampleHWVersion);
args.location.Emplace(exampleLocation);
args.requestorCanConsent.Emplace(kExampleClientCanConsent);
args.metadataForProvider.Emplace(metadata);
err = cluster.InvokeCommand(args, /* context = */ nullptr, OnQueryImageResponse, OnQueryImageFailure);
if (err != CHIP_NO_ERROR)
{
ChipLogError(SoftwareUpdate, "QueryImage() failed: %" CHIP_ERROR_FORMAT, err.Format());
Expand Down
19 changes: 10 additions & 9 deletions src/app/clusters/ota-provider/ota-provider-delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@

#pragma once

#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/enums.h>
#include <app/CommandHandler.h>
#include <app/ConcreteCommandPath.h>
#include <app/data-model/DecodableList.h>
#include <app/util/af.h>
#include <lib/core/Optional.h>

Expand All @@ -35,17 +37,16 @@ namespace Clusters {
class OTAProviderDelegate
{
public:
// TODO(#8605): protocolsSupported should be list of OTADownloadProtocol enums, not uint8_t
virtual EmberAfStatus HandleQueryImage(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, uint16_t vendorId,
uint16_t productId, uint32_t softwareVersion, uint8_t protocolsSupported,
const Optional<uint16_t> & hardwareVersion, const Optional<CharSpan> & location,
const Optional<bool> & requestorCanConsent,
const Optional<ByteSpan> & metadataForProvider) = 0;
virtual EmberAfStatus HandleQueryImage(CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
const OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData) = 0;

virtual EmberAfStatus HandleApplyUpdateRequest(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const chip::ByteSpan & updateToken, uint32_t newVersion) = 0;
virtual EmberAfStatus
HandleApplyUpdateRequest(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) = 0;

virtual EmberAfStatus HandleNotifyUpdateApplied(const chip::ByteSpan & updateToken, uint32_t softwareVersion) = 0;
virtual EmberAfStatus
HandleNotifyUpdateApplied(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
const OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData) = 0;

virtual ~OTAProviderDelegate() = default;
};
Expand Down
21 changes: 13 additions & 8 deletions src/app/clusters/ota-provider/ota-provider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback(
const Commands::ApplyUpdateRequest::DecodableType & commandData)
{
auto & updateToken = commandData.updateToken;
auto & newVersion = commandData.newVersion;

EndpointId endpoint = commandPath.mEndpointId;

Expand All @@ -90,9 +89,10 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback(
{
ChipLogError(Zcl, "expected size %zu for UpdateToken, got %zu", kUpdateTokenMaxLength, updateToken.size());
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
return true;
}

status = delegate->HandleApplyUpdateRequest(commandObj, commandPath, updateToken, newVersion);
status = delegate->HandleApplyUpdateRequest(commandObj, commandPath, commandData);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfSendImmediateDefaultResponse(status);
Expand All @@ -114,8 +114,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterNotifyUpdateAppliedCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::NotifyUpdateApplied::DecodableType & commandData)
{
auto & updateToken = commandData.updateToken;
auto & softwareVersion = commandData.softwareVersion;
auto & updateToken = commandData.updateToken;

EndpointId endpoint = commandPath.mEndpointId;

Expand All @@ -133,9 +132,10 @@ bool emberAfOtaSoftwareUpdateProviderClusterNotifyUpdateAppliedCallback(
{
ChipLogError(Zcl, "expected size %zu for UpdateToken, got %zu", kUpdateTokenMaxLength, updateToken.size());
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_INVALID_ARGUMENT);
return true;
}

status = delegate->HandleNotifyUpdateApplied(updateToken, softwareVersion);
status = delegate->HandleNotifyUpdateApplied(commandObj, commandPath, commandData);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfSendImmediateDefaultResponse(status);
Expand Down Expand Up @@ -188,7 +188,13 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl
ChipLogDetail(Zcl, " VendorID: 0x%" PRIx16, vendorId);
ChipLogDetail(Zcl, " ProductID: %" PRIu16, productId);
ChipLogDetail(Zcl, " SoftwareVersion: %" PRIu32, softwareVersion);
ChipLogDetail(Zcl, " ProtocolsSupported: %" PRIu8, protocolsSupported);
ChipLogDetail(Zcl, " ProtocolsSupported: [");
auto protocolIter = protocolsSupported.begin();
while (protocolIter.Next())
{
ChipLogDetail(Zcl, " %" PRIu8, protocolIter.GetValue());
}
ChipLogDetail(Zcl, " ]");
if (hardwareVersion.HasValue())
{
ChipLogDetail(Zcl, " HardwareVersion: %" PRIu16, hardwareVersion.Value());
Expand Down Expand Up @@ -220,8 +226,7 @@ bool emberAfOtaSoftwareUpdateProviderClusterQueryImageCallback(app::CommandHandl
return true;
}

status = delegate->HandleQueryImage(commandObj, commandPath, vendorId, productId, softwareVersion, protocolsSupported,
hardwareVersion, location, requestorCanConsent, metadataForProvider);
status = delegate->HandleQueryImage(commandObj, commandPath, commandData);
if (status != EMBER_ZCL_STATUS_SUCCESS)
{
emberAfSendImmediateDefaultResponse(status);
Expand Down
Loading

0 comments on commit 2392136

Please sign in to comment.