From 27a0643b8eec8a2ee383a5cb6c31079e90f1f59b Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 2 Nov 2021 14:17:04 -0400 Subject: [PATCH] Fully align the XML for QueryImage and QueryImageResponse with the spec. --- .../OTAProviderExample.cpp | 33 +++++++++---------- .../ota-provider-common/OTAProviderExample.h | 3 +- examples/ota-requestor-app/linux/main.cpp | 13 ++++---- .../ota-provider/ota-provider-delegate.h | 5 +-- .../clusters/ota-provider/ota-provider.cpp | 8 ++++- .../zcl/data-model/chip/chip-ota.xml | 16 ++++----- .../python/chip/clusters/Objects.py | 4 +-- .../zap-generated/cluster-objects.h | 32 +++++++++--------- .../zap-generated/cluster/Commands.h | 19 +++++------ 9 files changed, 67 insertions(+), 66 deletions(-) diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp index 26c7b363a154be..5add7c1a2f672f 100644 --- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp +++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp @@ -93,12 +93,12 @@ 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 & hardwareVersion, const Optional & location, - const Optional & requestorCanConsent, - const Optional & metadataForProvider) +EmberAfStatus +OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, + uint16_t vendorId, uint16_t productId, uint32_t softwareVersion, + const chip::app::DataModel::DecodableList & protocolsSupported, + const Optional & hardwareVersion, const Optional & location, + const Optional & requestorCanConsent, const Optional & metadataForProvider) { // TODO: add confiuration for returning BUSY status @@ -150,18 +150,15 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c } 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(); + 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; diff --git a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h index 854fc0f3fb0db8..a113d84770e9d1 100644 --- a/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h +++ b/examples/ota-provider-app/ota-provider-common/OTAProviderExample.h @@ -33,7 +33,8 @@ class OTAProviderExample : public chip::app::Clusters::OTAProviderDelegate // 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, + uint16_t vendorId, uint16_t productId, uint32_t softwareVersion, + const chip::app::DataModel::DecodableList & protocolsSupported, const chip::Optional & hardwareVersion, const chip::Optional & location, const chip::Optional & requestorCanConsent, diff --git a/examples/ota-requestor-app/linux/main.cpp b/examples/ota-requestor-app/linux/main.cpp index 7ee0084159f1ec..ae16b33848f535 100644 --- a/examples/ota-requestor-app/linux/main.cpp +++ b/examples/ota-requestor-app/linux/main.cpp @@ -151,13 +151,12 @@ void OnConnected(void * context, OperationalDeviceProxy * operationalDeviceProxy constexpr EndpointId kOtaProviderEndpoint = 0; // 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 EmberAfOTADownloadProtocol 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; diff --git a/src/app/clusters/ota-provider/ota-provider-delegate.h b/src/app/clusters/ota-provider/ota-provider-delegate.h index cc18a79afb9bdd..20ce0f95b9f1e0 100644 --- a/src/app/clusters/ota-provider/ota-provider-delegate.h +++ b/src/app/clusters/ota-provider/ota-provider-delegate.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -35,9 +36,9 @@ 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, + uint16_t productId, uint32_t softwareVersion, + const DataModel::DecodableList & protocolsSupported, const Optional & hardwareVersion, const Optional & location, const Optional & requestorCanConsent, const Optional & metadataForProvider) = 0; diff --git a/src/app/clusters/ota-provider/ota-provider.cpp b/src/app/clusters/ota-provider/ota-provider.cpp index a2da67b9521e1b..49451b7645a1eb 100644 --- a/src/app/clusters/ota-provider/ota-provider.cpp +++ b/src/app/clusters/ota-provider/ota-provider.cpp @@ -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()); diff --git a/src/app/zap-templates/zcl/data-model/chip/chip-ota.xml b/src/app/zap-templates/zcl/data-model/chip/chip-ota.xml index 37f71a164e82eb..9d311bd50beed1 100644 --- a/src/app/zap-templates/zcl/data-model/chip/chip-ota.xml +++ b/src/app/zap-templates/zcl/data-model/chip/chip-ota.xml @@ -48,7 +48,7 @@ limitations under the License. - + @@ -67,13 +67,13 @@ limitations under the License. Response to QueryImage command - - - - - - - + + + + + + + Reponse to ApplyUpdateRequest command diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py index bc6b81ad0507e7..92851987639f81 100644 --- a/src/controller/python/chip/clusters/Objects.py +++ b/src/controller/python/chip/clusters/Objects.py @@ -4973,7 +4973,7 @@ def descriptor(cls) -> ClusterObjectDescriptor: ClusterObjectFieldDescriptor( Label="softwareVersion", Tag=2, Type=uint), ClusterObjectFieldDescriptor( - Label="protocolsSupported", Tag=3, Type=OtaSoftwareUpdateProvider.Enums.OTADownloadProtocol), + Label="protocolsSupported", Tag=3, Type=OtaSoftwareUpdateProvider.Enums.OTADownloadProtocol, IsArray=True), ClusterObjectFieldDescriptor( Label="hardwareVersion", Tag=4, Type=uint), ClusterObjectFieldDescriptor( @@ -4987,7 +4987,7 @@ def descriptor(cls) -> ClusterObjectDescriptor: vendorId: 'uint' = None productId: 'uint' = None softwareVersion: 'uint' = None - protocolsSupported: 'OtaSoftwareUpdateProvider.Enums.OTADownloadProtocol' = None + protocolsSupported: typing.List['OtaSoftwareUpdateProvider.Enums.OTADownloadProtocol'] = None hardwareVersion: 'uint' = None location: 'str' = None requestorCanConsent: 'bool' = None diff --git a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h index 0fc32cbd401e24..b8524f6d05da37 100644 --- a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h +++ b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h @@ -5904,7 +5904,7 @@ struct Type chip::VendorId vendorId; uint16_t productId; uint32_t softwareVersion; - OTADownloadProtocol protocolsSupported; + DataModel::List protocolsSupported; Optional hardwareVersion; Optional location; Optional requestorCanConsent; @@ -5922,7 +5922,7 @@ struct DecodableType chip::VendorId vendorId; uint16_t productId; uint32_t softwareVersion; - OTADownloadProtocol protocolsSupported; + DataModel::DecodableList protocolsSupported; Optional hardwareVersion; Optional location; Optional requestorCanConsent; @@ -6013,13 +6013,13 @@ struct Type static constexpr ClusterId GetClusterId() { return Clusters::OtaSoftwareUpdateProvider::Id; } OTAQueryStatus status; - uint32_t delayedActionTime; - chip::CharSpan imageURI; - uint32_t softwareVersion; - chip::CharSpan softwareVersionString; - chip::ByteSpan updateToken; - bool userConsentNeeded; - chip::ByteSpan metadataForRequestor; + Optional delayedActionTime; + Optional imageURI; + Optional softwareVersion; + Optional softwareVersionString; + Optional updateToken; + Optional userConsentNeeded; + Optional metadataForRequestor; CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag) const; }; @@ -6031,13 +6031,13 @@ struct DecodableType static constexpr ClusterId GetClusterId() { return Clusters::OtaSoftwareUpdateProvider::Id; } OTAQueryStatus status; - uint32_t delayedActionTime; - chip::CharSpan imageURI; - uint32_t softwareVersion; - chip::CharSpan softwareVersionString; - chip::ByteSpan updateToken; - bool userConsentNeeded; - chip::ByteSpan metadataForRequestor; + Optional delayedActionTime; + Optional imageURI; + Optional softwareVersion; + Optional softwareVersionString; + Optional updateToken; + Optional userConsentNeeded; + Optional metadataForRequestor; CHIP_ERROR Decode(TLV::TLVReader & reader); }; }; // namespace QueryImageResponse diff --git a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h index 93a55d60662dd1..2ce548b54219f4 100644 --- a/zzz_generated/chip-tool/zap-generated/cluster/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/cluster/Commands.h @@ -1889,14 +1889,13 @@ static void OnOtaSoftwareUpdateProviderQueryImageResponseSuccess( { ChipLogProgress(Zcl, "Received QueryImageResponse:"); ChipLogProgress(Zcl, " status: %" PRIu8 "", data.status); - ChipLogProgress(Zcl, " delayedActionTime: %" PRIu32 "", data.delayedActionTime); - ChipLogProgress(Zcl, " imageURI: %.*s", static_cast(data.imageURI.size()), data.imageURI.data()); - ChipLogProgress(Zcl, " softwareVersion: %" PRIu32 "", data.softwareVersion); - ChipLogProgress(Zcl, " softwareVersionString: %.*s", static_cast(data.softwareVersionString.size()), - data.softwareVersionString.data()); - ChipLogProgress(Zcl, " updateToken: %zu", data.updateToken.size()); - ChipLogProgress(Zcl, " userConsentNeeded: %d", data.userConsentNeeded); - ChipLogProgress(Zcl, " metadataForRequestor: %zu", data.metadataForRequestor.size()); + ChipLogProgress(Zcl, " delayedActionTime: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " imageURI: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " softwareVersion: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " softwareVersionString: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " updateToken: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " userConsentNeeded: Optional printing is not implemented yet."); + ChipLogProgress(Zcl, " metadataForRequestor: Optional printing is not implemented yet."); ModelCommand * command = static_cast(context); command->SetCommandExitStatus(CHIP_NO_ERROR); @@ -14959,9 +14958,7 @@ class OtaSoftwareUpdateProviderQueryImage : public ModelCommand AddArgument("VendorId", 0, UINT16_MAX, &mRequest.vendorId); AddArgument("ProductId", 0, UINT16_MAX, &mRequest.productId); AddArgument("SoftwareVersion", 0, UINT32_MAX, &mRequest.softwareVersion); - AddArgument( - "ProtocolsSupported", 0, UINT8_MAX, - reinterpret_cast *>(&mRequest.protocolsSupported)); + // protocolsSupported Array parsing is not supported yet AddArgument("HardwareVersion", 0, UINT16_MAX, &mRequest.hardwareVersion); AddArgument("Location", &mRequest.location); AddArgument("RequestorCanConsent", 0, 1, &mRequest.requestorCanConsent);