Fully align the XML for QueryImage and QueryImageResponse with the spec.

examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp

@@ -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
@@ -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 };
@@ -149,39 +147,36 @@ 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);

examples/ota-provider-app/ota-provider-common/OTAProviderExample.h

@@ -32,15 +32,12 @@ 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 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(const chip::ByteSpan & updateToken, uint32_t softwareVersion) override;
 
     enum queryImageBehaviorType

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;

src/app/clusters/ota-provider/ota-provider-delegate.h

@@ -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>
 
@@ -35,15 +37,12 @@ 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 HandleApplyUpdateRequest(CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
-                                                   const chip::ByteSpan & updateToken, uint32_t newVersion) = 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 OtaSoftwareUpdateProvider::Commands::ApplyUpdateRequest::DecodableType & commandData) = 0;
 
     virtual EmberAfStatus HandleNotifyUpdateApplied(const chip::ByteSpan & updateToken, uint32_t softwareVersion) = 0;
 

src/app/clusters/ota-provider/ota-provider.cpp

@@ -72,7 +72,6 @@ bool emberAfOtaSoftwareUpdateProviderClusterApplyUpdateRequestCallback(
     const Commands::ApplyUpdateRequest::DecodableType & commandData)
 {
     auto & updateToken = commandData.updateToken;
-    auto & newVersion  = commandData.newVersion;
 
     EndpointId endpoint = commandPath.mEndpointId;
 
@@ -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);
@@ -133,6 +133,7 @@ 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);
@@ -188,7 +189,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());
@@ -220,8 +227,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);

src/app/zap-templates/zcl/data-model/chip/chip-ota.xml

@@ -48,7 +48,7 @@ limitations under the License.
             <arg name="vendorId" type="vendor_id"/>
             <arg name="productId" type="INT16U"/>
             <arg name="softwareVersion" type="INT32U"/>
-            <arg name="protocolsSupported" type="OTADownloadProtocol"/> <!-- TODO(#8605): add array="true" when lists are supported -->
+            <arg name="protocolsSupported" type="OTADownloadProtocol" array="true"/>
             <arg name="hardwareVersion" type="INT16U" optional="true"/>
             <arg name="location" type="CHAR_STRING" length="2" optional="true"/>
             <arg name="requestorCanConsent" type="BOOLEAN" optional="true"/>
@@ -67,13 +67,13 @@ limitations under the License.
         <command source="server" code="0x03" name="QueryImageResponse" optional="false" cli="chip ota queryimageresponse">
             <description>Response to QueryImage command</description>
             <arg name="status" type="OTAQueryStatus"/>
-            <arg name="delayedActionTime" type="INT32U"/>
-            <arg name="imageURI" type="CHAR_STRING"/>
-            <arg name="softwareVersion" type="INT32U"/>
-            <arg name="softwareVersionString" type="CHAR_STRING"/>
-            <arg name="updateToken" type="OCTET_STRING" length="32"/>
-            <arg name="userConsentNeeded" type="BOOLEAN" default="false"/>
-            <arg name="metadataForRequestor" type="OCTET_STRING" length="512"/>
+            <arg name="delayedActionTime" type="INT32U" optional="true"/>
+            <arg name="imageURI" type="CHAR_STRING" optional="true"/>
+            <arg name="softwareVersion" type="INT32U" optional="true"/>
+            <arg name="softwareVersionString" type="CHAR_STRING" optional="true"/>
+            <arg name="updateToken" type="OCTET_STRING" length="32" optional="true"/>
+            <arg name="userConsentNeeded" type="BOOLEAN" default="false" optional="true"/>
+            <arg name="metadataForRequestor" type="OCTET_STRING" length="512" optional="true"/>
         </command>
         <command source="server" code="0x04" name="ApplyUpdateResponse" optional="false" cli="chip ota applyupdateresponse">
             <description>Reponse to ApplyUpdateRequest command</description>