Update the preset handle field in the SetActivePresetRequest c… (proj…

…ect-chip#35105) * Update the preset handle field in the SetActivePresetRequestRequest command to be nullable - Update the deleagte API for getting the active preset handle to return a nullable handle * Return error if CopySpanToMutableSpan fails * Restyled by clang-format * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> --------- Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>
PeterC1965 · Aug 28, 2024 · 571e924 · 571e924
1 parent b9d5afe
commit 571e924
Show file tree

Hide file tree

Showing 26 changed files with 65 additions and 52 deletions.
diff --git a/examples/air-purifier-app/air-purifier-common/air-purifier-app.matter b/examples/air-purifier-app/air-purifier-common/air-purifier-app.matter
@@ -1856,7 +1856,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -5696,7 +5696,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter b/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter
@@ -4130,7 +4130,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/...irpurifier_airqualitysensor_temperaturesensor_humiditysensor_thermostat_56de3d5f45.matter b/...irpurifier_airqualitysensor_temperaturesensor_humiditysensor_thermostat_56de3d5f45.matter
@@ -1779,7 +1779,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/chef/devices/rootnode_heatingcoolingunit_ncdGai1E5a.matter b/examples/chef/devices/rootnode_heatingcoolingunit_ncdGai1E5a.matter
@@ -2136,7 +2136,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/chef/devices/rootnode_roomairconditioner_9cf3607804.matter b/examples/chef/devices/rootnode_roomairconditioner_9cf3607804.matter
@@ -1719,7 +1719,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/chef/devices/rootnode_thermostat_bm3fb8dhYi.matter b/examples/chef/devices/rootnode_thermostat_bm3fb8dhYi.matter
@@ -1939,7 +1939,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/placeholder/linux/apps/app1/config.matter b/examples/placeholder/linux/apps/app1/config.matter
@@ -5414,7 +5414,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {
@@ -5769,7 +5769,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/placeholder/linux/apps/app2/config.matter b/examples/placeholder/linux/apps/app2/config.matter
@@ -5371,7 +5371,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {
@@ -5726,7 +5726,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/thermostat/linux/include/thermostat-delegate-impl.h b/examples/thermostat/linux/include/thermostat-delegate-impl.h
@@ -54,7 +54,7 @@ class ThermostatDelegate : public Delegate
 
     CHIP_ERROR GetPresetAtIndex(size_t index, PresetStructWithOwnedMembers & preset) override;
 
-    CHIP_ERROR GetActivePresetHandle(MutableByteSpan & activePresetHandle) override;
+    CHIP_ERROR GetActivePresetHandle(DataModel::Nullable<MutableByteSpan> & activePresetHandle) override;
 
     CHIP_ERROR SetActivePresetHandle(const DataModel::Nullable<ByteSpan> & newActivePresetHandle) override;
 

diff --git a/examples/thermostat/linux/thermostat-delegate-impl.cpp b/examples/thermostat/linux/thermostat-delegate-impl.cpp
@@ -117,9 +117,19 @@ CHIP_ERROR ThermostatDelegate::GetPresetAtIndex(size_t index, PresetStructWithOw
     return CHIP_ERROR_PROVIDER_LIST_EXHAUSTED;
 }
 
-CHIP_ERROR ThermostatDelegate::GetActivePresetHandle(MutableByteSpan & activePresetHandle)
+CHIP_ERROR ThermostatDelegate::GetActivePresetHandle(DataModel::Nullable<MutableByteSpan> & activePresetHandle)
 {
-    return CopySpanToMutableSpan(ByteSpan(mActivePresetHandleData, mActivePresetHandleDataSize), activePresetHandle);
+    if (mActivePresetHandleDataSize != 0)
+    {
+        ReturnErrorOnFailure(
+            CopySpanToMutableSpan(ByteSpan(mActivePresetHandleData, mActivePresetHandleDataSize), activePresetHandle.Value()));
+        activePresetHandle.Value().reduce_size(mActivePresetHandleDataSize);
+    }
+    else
+    {
+        activePresetHandle.SetNull();
+    }
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR ThermostatDelegate::SetActivePresetHandle(const DataModel::Nullable<ByteSpan> & newActivePresetHandle)

diff --git a/examples/thermostat/nxp/zap/thermostat_matter_thread.matter b/examples/thermostat/nxp/zap/thermostat_matter_thread.matter
@@ -2530,7 +2530,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/thermostat/nxp/zap/thermostat_matter_wifi.matter b/examples/thermostat/nxp/zap/thermostat_matter_wifi.matter
@@ -2441,7 +2441,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/thermostat/qpg/zap/thermostaticRadiatorValve.matter b/examples/thermostat/qpg/zap/thermostaticRadiatorValve.matter
@@ -2138,7 +2138,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/examples/thermostat/thermostat-common/thermostat.matter b/examples/thermostat/thermostat-common/thermostat.matter
@@ -2318,7 +2318,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/src/app/clusters/thermostat-server/thermostat-delegate.h b/src/app/clusters/thermostat-server/thermostat-delegate.h
@@ -78,11 +78,10 @@ class Delegate
     /**
      * @brief Get the ActivePresetHandle attribute value.
      *
-     * @param[out] activePresetHandle The MutableByteSpan to copy the active preset handle into. On success,
-     *             the callee must update the length to the length of the copied data. If the value of
-     *             the attribute is null, the callee must set the MutableByteSpan to empty.
+     * @param[out] activePresetHandle The nullable MutableByteSpan to copy the active preset handle into. On success,
+     *             the size of the activePresetHandle is updated to the length of the copied data.
      */
-    virtual CHIP_ERROR GetActivePresetHandle(MutableByteSpan & activePresetHandle) = 0;
+    virtual CHIP_ERROR GetActivePresetHandle(DataModel::Nullable<MutableByteSpan> & activePresetHandle) = 0;
 
     /**
      * @brief Set the ActivePresetHandle attribute value.

diff --git a/src/app/clusters/thermostat-server/thermostat-server.cpp b/src/app/clusters/thermostat-server/thermostat-server.cpp
@@ -761,19 +761,13 @@ CHIP_ERROR ThermostatAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
         VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is null"));
 
         uint8_t buffer[kPresetHandleSize];
-        MutableByteSpan activePresetHandle(buffer);
+        MutableByteSpan activePresetHandleSpan(buffer);
+        auto activePresetHandle = DataModel::MakeNullable(activePresetHandleSpan);
 
         CHIP_ERROR err = delegate->GetActivePresetHandle(activePresetHandle);
         ReturnErrorOnFailure(err);
 
-        if (activePresetHandle.empty())
-        {
-            ReturnErrorOnFailure(aEncoder.EncodeNull());
-        }
-        else
-        {
-            ReturnErrorOnFailure(aEncoder.Encode(activePresetHandle));
-        }
+        ReturnErrorOnFailure(aEncoder.Encode(activePresetHandle));
     }
     break;
     case ScheduleTypes::Id: {
@@ -1299,16 +1293,16 @@ bool emberAfThermostatClusterSetActivePresetRequestCallback(
         return true;
     }
 
-    ByteSpan newPresetHandle = commandData.presetHandle;
+    DataModel::Nullable<ByteSpan> newPresetHandle = commandData.presetHandle;
 
     // If the preset handle passed in the command is not present in the Presets attribute, return INVALID_COMMAND.
-    if (!IsPresetHandlePresentInPresets(delegate, newPresetHandle))
+    if (!newPresetHandle.IsNull() && !IsPresetHandlePresentInPresets(delegate, newPresetHandle.Value()))
     {
         commandObj->AddStatus(commandPath, imcode::InvalidCommand);
         return true;
     }
 
-    CHIP_ERROR err = delegate->SetActivePresetHandle(DataModel::MakeNullable(newPresetHandle));
+    CHIP_ERROR err = delegate->SetActivePresetHandle(newPresetHandle);
 
     if (err != CHIP_NO_ERROR)
     {
@@ -1470,7 +1464,8 @@ imcode commitPresets(Delegate * delegate, EndpointId endpoint)
     // attribute. If a preset is not found with the same presetHandle, return INVALID_IN_STATE. If there is no ActivePresetHandle
     // attribute set, continue with other checks.
     uint8_t buffer[kPresetHandleSize];
-    MutableByteSpan activePresetHandle(buffer);
+    MutableByteSpan activePresetHandleSpan(buffer);
+    auto activePresetHandle = DataModel::MakeNullable(activePresetHandleSpan);
 
     err = delegate->GetActivePresetHandle(activePresetHandle);
 
@@ -1479,9 +1474,9 @@ imcode commitPresets(Delegate * delegate, EndpointId endpoint)
         return imcode::InvalidInState;
     }
 
-    if (!activePresetHandle.empty())
+    if (!activePresetHandle.IsNull())
     {
-        uint8_t count = CountPresetsInPendingListWithPresetHandle(delegate, activePresetHandle);
+        uint8_t count = CountPresetsInPendingListWithPresetHandle(delegate, activePresetHandle.Value());
         if (count == 0)
         {
             return imcode::InvalidInState;

diff --git a/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/thermostat-cluster.xml
@@ -481,7 +481,7 @@ limitations under the License.
 
     <command source="client" code="0x06" name="SetActivePresetRequest" optional="true">
       <description>ID</description>
-      <arg id="0" name="PresetHandle" type="octet_string" length="16"/>
+      <arg id="0" name="PresetHandle" type="octet_string" length="16" isNullable="true"/>
     </command>
 
     <!-- Server Commands/Responses -->

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -6994,7 +6994,7 @@ cluster Thermostat = 513 {
   }
 
   request struct SetActivePresetRequestRequest {
-    octet_string<16> presetHandle = 0;
+    nullable octet_string<16> presetHandle = 0;
   }
 
   response struct AtomicResponse = 253 {

diff --git a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java
@@ -40637,16 +40637,16 @@ public void onResponse(StructType invokeStructValue) {
         }}, commandId, commandArgs, timedInvokeTimeoutMs);
     }
 
-    public void setActivePresetRequest(DefaultClusterCallback callback, byte[] presetHandle) {
+    public void setActivePresetRequest(DefaultClusterCallback callback, @Nullable byte[] presetHandle) {
       setActivePresetRequest(callback, presetHandle, 0);
     }
 
-    public void setActivePresetRequest(DefaultClusterCallback callback, byte[] presetHandle, int timedInvokeTimeoutMs) {
+    public void setActivePresetRequest(DefaultClusterCallback callback, @Nullable byte[] presetHandle, int timedInvokeTimeoutMs) {
       final long commandId = 6L;
 
       ArrayList<StructElement> elements = new ArrayList<>();
       final long presetHandleFieldID = 0L;
-      BaseTLVType presetHandletlvValue = new ByteArrayType(presetHandle);
+      BaseTLVType presetHandletlvValue = presetHandle != null ? new ByteArrayType(presetHandle) : new NullType();
       elements.add(new StructElement(presetHandleFieldID, presetHandletlvValue));
 
       StructType commandArgs = new StructType(elements);

diff --git a/src/controller/java/generated/java/matter/controller/cluster/clusters/ThermostatCluster.kt b/src/controller/java/generated/java/matter/controller/cluster/clusters/ThermostatCluster.kt
@@ -504,7 +504,7 @@ class ThermostatCluster(private val controller: MatterController, private val en
   }
 
   suspend fun setActivePresetRequest(
-    presetHandle: ByteArray,
+    presetHandle: ByteArray?,
     timedInvokeTimeout: Duration? = null,
   ) {
     val commandId: UInt = 6u
@@ -513,7 +513,7 @@ class ThermostatCluster(private val controller: MatterController, private val en
     tlvWriter.startStructure(AnonymousTag)
 
     val TAG_PRESET_HANDLE_REQ: Int = 0
-    tlvWriter.put(ContextSpecificTag(TAG_PRESET_HANDLE_REQ), presetHandle)
+    presetHandle?.let { tlvWriter.put(ContextSpecificTag(TAG_PRESET_HANDLE_REQ), presetHandle) }
     tlvWriter.endStructure()
 
     val request: InvokeRequest =

diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py
diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.h
diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm b/src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm
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
diff --git a/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h