GH-24: Fix CC SWITCH_COLOR Duration param

applications/zpc/components/zpc_rust/src/rust_command_handlers/zwave_command_classes/zwave_command_class_switch_color.rs

@@ -327,9 +327,7 @@ fn switch_color_set(state_node: Attribute) -> Result<(sl_status_t, Vec<u8>), Att
             reserved: 0,
         },
         vg1: colors,
-        // FIXME: UIC-1926 This SwitchColorSetFrame wants a SwitchColorSetDurationEnum for
-        // the duration, which prevents me to use my duration found in the attribute store.
-        duration: SwitchColorSetDurationEnum::Instantly,
+        duration: _duration as u8,
     };
 
     Ok((SL_STATUS_OK, set_frame.into()))

applications/zpc/components/zwave_command_classes/assets/ZWave_custom_cmd_classes.xml

@@ -12970,7 +12970,10 @@
         <param key="0x00" name="Color Component ID" type="BYTE" />
         <param key="0x01" name="Value" type="BYTE" />
       </variant_group>
-      <param key="0x02" name="Duration" type="BYTE" />
+      <param key="0x02" name="Duration" type="BYTE">
+        <bitflag key="0x01" flagname="Instantly" flagmask="0x00" />
+        <bitflag key="0x02" flagname="Default" flagmask="0xFF" />
+      </param>
     </cmd>
     <cmd key="0x06" name="SWITCH_COLOR_START_LEVEL_CHANGE" help="Color Switch Start Level Change" support_mode="RX">
       <param key="0x00" name="Properties1" type="STRUCT_BYTE">
@@ -12998,10 +13001,10 @@
       <param key="0x00" name="Color Component ID" type="BYTE" />
       <param key="0x01" name="Current Value" type="BYTE" />
       <param key="0x02" name="Target Value" type="BYTE" />
-      <param key="0x03" name="Duration" type="CONST">
-        <const key="0x00" flagname="Already at the Target Value" flagmask="0x00" />
-        <const key="0x01" flagname="Unknown duration" flagmask="0xFE" />
-        <const key="0x02" flagname="Reserved" flagmask="0xFF" />
+      <param key="0x03" name="Duration" type="BYTE">
+        <bitflag key="0x00" flagname="Already at the Target Value" flagmask="0x00" />
+        <bitflag key="0x01" flagname="Unknown duration" flagmask="0xFE" />
+        <bitflag key="0x02" flagname="Reserved" flagmask="0xFF" />
       </param>
     </cmd>
     <cmd key="0x05" name="SWITCH_COLOR_SET" help="Color Switch Set" support_mode="RX">
@@ -13013,9 +13016,9 @@
         <param key="0x00" name="Color Component ID" type="BYTE" />
         <param key="0x01" name="Value" type="BYTE" />
       </variant_group>
-      <param key="0x02" name="Duration" type="CONST">
-        <const key="0x00" flagname="Instantly" flagmask="0x00" />
-        <const key="0x01" flagname="Default" flagmask="0xFF" />
+      <param key="0x02" name="Duration" type="BYTE">
+        <bitflag key="0x01" flagname="Instantly" flagmask="0x00" />
+        <bitflag key="0x02" flagname="Default" flagmask="0xFF" />
       </param>
     </cmd>
     <cmd key="0x06" name="SWITCH_COLOR_START_LEVEL_CHANGE" help="Color Switch Start Level Change" support_mode="RX">
@@ -13027,9 +13030,9 @@
       </param>
       <param key="0x01" name="Color Component ID" type="BYTE" />
       <param key="0x02" name="Start Level" type="BYTE" />
-      <param key="0x03" name="Duration" type="CONST">
-        <const key="0x00" flagname="Instantly" flagmask="0x00" />
-        <const key="0x01" flagname="Default" flagmask="0xFF" />
+      <param key="0x03" name="Duration" type="BYTE">
+        <bitflag key="0x01" flagname="Instantly" flagmask="0x00" />
+        <bitflag key="0x02" flagname="Default" flagmask="0xFF" />
       </param>
     </cmd>
     <cmd key="0x07" name="SWITCH_COLOR_STOP_LEVEL_CHANGE" help="Color Switch Stop Level Change" support_mode="RX">

applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c

@@ -24,6 +24,7 @@
 #include "attribute_store_defined_attribute_types.h"
 #include "ZW_classcmd.h"
 #include "zpc_attribute_resolver.h"
+#include "zwave_command_classes_utils.h"
 
 // Includes from other Unify Components
 #include "dotdot_mqtt.h"
@@ -34,6 +35,24 @@
 #include "attribute_timeouts.h"
 #include "sl_log.h"
 
+#define LOG_TAG "zwave_command_class_switch_color"
+
+static void set_duration(attribute_store_node_t state_node,
+                         attribute_store_node_value_state_t state,
+                         uint32_t duration)
+{
+  attribute_store_node_t duration_node
+    = attribute_store_get_first_child_by_type(
+      state_node,
+      ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION);
+
+  attribute_store_set_node_attribute_value(duration_node,
+                                           state,
+                                           (uint8_t *)&duration,
+                                           sizeof(duration));
+}
+
+[[maybe_unused]]
 static void
   set_all_color_switch_durations(attribute_store_node_t state_node,
                                  attribute_store_node_value_state_t value_state,
@@ -47,15 +66,7 @@ static void
   while (component_node != ATTRIBUTE_STORE_INVALID_NODE) {
     index += 1;
 
-    attribute_store_node_t duration_node
-      = attribute_store_get_first_child_by_type(
-        component_node,
-        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION);
-
-    attribute_store_set_node_attribute_value(duration_node,
-                                             value_state,
-                                             (uint8_t *)&duration,
-                                             sizeof(duration));
+    set_duration(component_node, value_state, duration);
 
     component_node = attribute_store_get_node_child_by_type(
       state_node,
@@ -64,6 +75,34 @@ static void
   }
 }
 
+static void switch_color_undefine_reported(attribute_store_node_t state_node)
+{
+  zwave_command_class_switch_color_invoke_on_all_attributes_with_return_value(
+    state_node,
+    ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+    attribute_stop_transition);
+
+  attribute_store_node_t duration_node
+      = attribute_store_get_first_child_by_type(
+        state_node,
+        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION);
+  color_component_id_duration_t duration = 0;
+  attribute_store_undefine_desired(duration_node);
+  attribute_store_set_reported(duration_node,
+                               &duration,
+                               sizeof(duration));
+
+  sl_log_debug(LOG_TAG, "Transition time expired, probe color");
+  zwave_command_class_switch_color_invoke_on_all_attributes(
+    state_node,
+    ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+    attribute_store_undefine_desired);
+  zwave_command_class_switch_color_invoke_on_all_attributes(
+    state_node,
+    ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+    attribute_store_undefine_reported);
+}
+
 // FIXME: UIC-1901 This function belongs to zwave_command_class_switch_color.rs, but this
 // component really cannot interact with the attribute resolver.
 static void
@@ -78,12 +117,22 @@ static void
     return;
   }
 
+  attribute_store_node_t duration_node
+      = attribute_store_get_first_child_by_type(
+        state_node,
+        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION);
+
+  color_component_id_duration_t duration = 0;
+  attribute_store_get_desired(duration_node,
+                              &duration,
+                              sizeof(duration));
+
+  clock_time_t zwave_desired_duration
+    = zwave_duration_to_time((uint8_t)duration);
+
   switch (event) {
     case FRAME_SENT_EVENT_OK_SUPERVISION_WORKING:
-      zwave_command_class_switch_color_invoke_on_all_attributes_with_return_value(
-        state_node,
-        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION,
-        &attribute_store_set_reported_as_desired);
+      attribute_store_set_reported_as_desired(duration_node);
       break;
 
     case FRAME_SENT_EVENT_OK_SUPERVISION_SUCCESS:
@@ -95,11 +144,44 @@ static void
         state_node,
         ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
         &attribute_store_undefine_desired);
-      set_all_color_switch_durations(state_node, REPORTED_ATTRIBUTE, 0);
-      set_all_color_switch_durations(state_node, DESIRED_ATTRIBUTE, 0);
+      set_duration(state_node, REPORTED_ATTRIBUTE, 0);
+      attribute_store_undefine_desired(duration_node);
+      break;
+
+    case FRAME_SENT_EVENT_OK_NO_SUPERVISION:
+      zwave_command_class_switch_color_invoke_on_all_attributes_with_return_value(
+        state_node,
+        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+        attribute_stop_transition);
+      if(zwave_desired_duration > 0) {
+        // Should we estimate reported color values during transition
+        // and publish them as reported, like we did for level cluster?
+
+        zwave_command_class_switch_color_invoke_on_all_attributes(
+          state_node,
+          ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+          attribute_store_undefine_desired);
+
+        attribute_store_set_reported_as_desired(duration_node);
+        attribute_store_undefine_desired(duration_node);
+
+        // Probe again after this duration
+        attribute_timeout_set_callback(state_node,
+                                       zwave_desired_duration + PROBE_BACK_OFF,
+                                       &switch_color_undefine_reported);
+      } else {
+        zwave_command_class_switch_color_invoke_on_all_attributes(
+          state_node,
+          ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+          attribute_store_undefine_desired);
+        zwave_command_class_switch_color_invoke_on_all_attributes(
+          state_node,
+          ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+          attribute_store_undefine_reported);
+        attribute_store_undefine_desired(duration_node);
+      }
       break;
 
-    // FRAME_SENT_EVENT_OK_NO_SUPERVISION:
     // FRAME_SENT_EVENT_OK_SUPERVISION_NO_SUPPORT:
     // FRAME_SENT_EVENT_OK_SUPERVISION_FAIL:
     default:
@@ -115,14 +197,8 @@ static void
         state_node,
         ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
         &attribute_store_undefine_reported);
-      zwave_command_class_switch_color_invoke_on_all_attributes(
-        state_node,
-        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION,
-        &attribute_store_undefine_desired);
-      zwave_command_class_switch_color_invoke_on_all_attributes(
-        state_node,
-        ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION,
-        &attribute_store_undefine_reported);
+      attribute_store_undefine_desired(duration_node);
+      attribute_store_undefine_reported(duration_node);
       break;
   }
 
@@ -190,4 +266,4 @@ void zwave_command_class_switch_color_invoke_on_all_attributes_with_return_value
       ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_COLOR_COMPONENT_ID,
       index);
   }
-}
+}

applications/zpc/components/zwave_command_classes/test/zwave_command_class_switch_color_test.c

@@ -31,6 +31,7 @@
 #include "ZW_classcmd.h"
 #include "zwave_utils.h"
 #include "zwave_controller_types.h"
+#include "zwave_command_class_color_switch_types.h"
 
 // Test helpers
 #include "zpc_attribute_store_test_helper.h"
@@ -47,6 +48,7 @@
 
 // Static variables
 static zpc_resolver_event_notification_function_t on_send_complete = NULL;
+static attribute_timeout_callback_t switch_color_undefine_reported = NULL;
 
 // Stub functions
 static sl_status_t register_send_event_handler_stub(
@@ -62,10 +64,23 @@ static sl_status_t register_send_event_handler_stub(
   return SL_STATUS_OK;
 }
 
+static sl_status_t attribute_timeout_set_callback_stub(
+  attribute_store_node_t node,
+  clock_time_t duration,
+  attribute_timeout_callback_t callback_function,
+  int cmock_num_calls)
+{
+  TEST_ASSERT_EQUAL(ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_STATE, attribute_store_get_node_type(node));
+  switch_color_undefine_reported = callback_function;
+  return SL_STATUS_OK;
+}
+
+
 /// Setup the test suite (called once before all test_xxx functions are called)
 void suiteSetUp()
 {
   on_send_complete = NULL;
+  switch_color_undefine_reported = NULL;
 
   datastore_init(":memory:");
   attribute_store_init();
@@ -155,10 +170,93 @@ void test_zwave_command_class_on_send_complete()
   TEST_ASSERT_TRUE(
     attribute_store_is_value_defined(value_1_node, REPORTED_ATTRIBUTE));
 
+  // Test without duration
+  on_send_complete(state_node,
+                   RESOLVER_SET_RULE,
+                   FRAME_SENT_EVENT_OK_NO_SUPERVISION);
+
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_2_node, REPORTED_ATTRIBUTE));
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_2_node, DESIRED_ATTRIBUTE));
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_1_node, REPORTED_ATTRIBUTE));
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_1_node, DESIRED_ATTRIBUTE));
+
+  // Test with duration
+  value = 0;
+  attribute_store_set_reported(value_1_node, &value, sizeof(value));
+  value = 1;
+  attribute_store_set_desired(value_1_node, &value, sizeof(value));
+  value = 2;
+  attribute_store_set_reported(value_2_node, &value, sizeof(value));
+  value = 3;
+  attribute_store_set_desired(value_2_node, &value, sizeof(value));
+
+  color_component_id_duration_t duration_value = 20;
+  attribute_store_node_t duration_node
+    = attribute_store_add_node(ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION,
+                               state_node);
+  attribute_store_set_desired(duration_node, &duration_value, sizeof(duration_value));
+
+  attribute_timeout_set_callback_ExpectAndReturn(
+    state_node,
+    zwave_duration_to_time(duration_value) + PROBE_BACK_OFF,
+    NULL,
+    SL_STATUS_OK);
+  attribute_timeout_set_callback_IgnoreArg_callback_function();
+  attribute_timeout_set_callback_Stub(&attribute_timeout_set_callback_stub);
+
+  on_send_complete(state_node,
+                   RESOLVER_SET_RULE,
+                   FRAME_SENT_EVENT_OK_NO_SUPERVISION);
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_2_node, DESIRED_ATTRIBUTE));
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(value_1_node, DESIRED_ATTRIBUTE));
+  TEST_ASSERT_FALSE(
+    attribute_store_is_value_defined(duration_node, DESIRED_ATTRIBUTE));
+  TEST_ASSERT_TRUE(
+    attribute_store_is_value_defined(duration_node, REPORTED_ATTRIBUTE));
+  TEST_ASSERT_TRUE(
+    attribute_store_is_value_defined(value_1_node, REPORTED_ATTRIBUTE));
+  TEST_ASSERT_TRUE(
+    attribute_store_is_value_defined(value_2_node, REPORTED_ATTRIBUTE));
+
   on_send_complete(state_node,
                    RESOLVER_SET_RULE,
                    FRAME_SENT_EVENT_OK_SUPERVISION_NO_SUPPORT);
 
   TEST_ASSERT_FALSE(
     attribute_store_is_value_defined(value_1_node, REPORTED_ATTRIBUTE));
 }
+
+void test_switch_color_undefine_reported_happy_case()
+{
+  //
+  TEST_ASSERT_NOT_NULL(switch_color_undefine_reported);
+
+  const int COLOR_VALUE_COUNT = 3;
+
+  for (int i = 0; i < COLOR_VALUE_COUNT; i++) {
+    attribute_store_node_t color_value_node
+      = attribute_store_add_node(ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+                                 endpoint_id_node);
+
+    attribute_store_set_reported(color_value_node, &i, sizeof(i));
+  }
+
+  switch_color_undefine_reported(endpoint_id_node);
+
+  for (int i = 0; i < COLOR_VALUE_COUNT; i++) {
+    int j;
+    attribute_store_node_t color_value_node
+      = attribute_store_add_node(ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE,
+                                 endpoint_id_node);
+
+    TEST_ASSERT_EQUAL(
+      SL_STATUS_FAIL,
+      attribute_store_get_reported(color_value_node, &j, sizeof(j)));
+  }
+}

helper.mk

@@ -156,7 +156,7 @@ zpc/build: zpc/configure build
 zpc/test: ${build_dir}/applications/zpc/components/zwave_command_classes/test/
 	ctest --test-dir ${<}
 
-zpc/default: zpc/configure zpc/build
+zpc/default: zpc/configure zpc/build zpc/test
 	@date -u
 
 ### @rootfs is faster than docker for env check