From 75be48b54d906bc7f5d8a04eb6d1f4ba622fdeb2 Mon Sep 17 00:00:00 2001 From: Nenad Kljajic Date: Wed, 29 Nov 2023 15:20:17 +0100 Subject: [PATCH 1/4] GH-24: Fix CC SWITCH_COLOR Duration param Duration can be any value in the range 0x01..0xFD. For reference, see: CC:0000.00.00.11.015 CC:0033.02.05.11.007 Signed-off-by: Nenad Kljajic --- .../zwave_command_class_switch_color.rs | 4 +- .../assets/ZWave_custom_cmd_classes.xml | 25 ++-- .../src/zwave_command_class_switch_color.c | 130 +++++++++++++++--- 3 files changed, 129 insertions(+), 30 deletions(-) diff --git a/applications/zpc/components/zpc_rust/src/rust_command_handlers/zwave_command_classes/zwave_command_class_switch_color.rs b/applications/zpc/components/zpc_rust/src/rust_command_handlers/zwave_command_classes/zwave_command_class_switch_color.rs index ca801e371a..c0c57b6c7c 100644 --- a/applications/zpc/components/zpc_rust/src/rust_command_handlers/zwave_command_classes/zwave_command_class_switch_color.rs +++ b/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), 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())) diff --git a/applications/zpc/components/zwave_command_classes/assets/ZWave_custom_cmd_classes.xml b/applications/zpc/components/zwave_command_classes/assets/ZWave_custom_cmd_classes.xml index dd2578fc6c..8481cafc21 100644 --- a/applications/zpc/components/zwave_command_classes/assets/ZWave_custom_cmd_classes.xml +++ b/applications/zpc/components/zwave_command_classes/assets/ZWave_custom_cmd_classes.xml @@ -12970,7 +12970,10 @@ - + + + + @@ -12998,10 +13001,10 @@ - - - - + + + + @@ -13013,9 +13016,9 @@ - - - + + + @@ -13027,9 +13030,9 @@ - - - + + + diff --git a/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c b/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c index 878c8e95f2..a18aa0d66f 100644 --- a/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c +++ b/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,38 @@ #include "attribute_timeouts.h" #include "sl_log.h" +#define LOG_TAG "zwave_command_class_switch_color" + +[[maybe_unused]] +static void set_desired_duration(attribute_store_node_t state_node, + 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, + DESIRED_ATTRIBUTE, + (uint8_t *)&duration, + sizeof(duration)); +} + +static void set_reported_duration(attribute_store_node_t state_node, + 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, + REPORTED_ATTRIBUTE, + (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, @@ -64,6 +97,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 +139,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 +166,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_reported_duration(state_node, 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 +219,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 +288,4 @@ void zwave_command_class_switch_color_invoke_on_all_attributes_with_return_value ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_COLOR_COMPONENT_ID, index); } -} \ No newline at end of file +} From 779fc6f071ee66267f4dcaadf2cbedf2a28d3b11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Labb=C3=A9?= Date: Tue, 23 Jan 2024 11:30:28 +0100 Subject: [PATCH 2/4] GH-24: zpc: Code refactoring of switch color Bug-SiliconLabs: UIC-3082 Relate-to: https://github.com/SiliconLabs/UnifySDK/pull/24 Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-24/develop Origin: https://github.com/nkljajic/UnifySDK/pull/5 Signed-off-by: Philippe Coval --- .../src/zwave_command_class_switch_color.c | 34 ++++--------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c b/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c index a18aa0d66f..6493072455 100644 --- a/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c +++ b/applications/zpc/components/zwave_command_classes/src/zwave_command_class_switch_color.c @@ -37,9 +37,9 @@ #define LOG_TAG "zwave_command_class_switch_color" -[[maybe_unused]] -static void set_desired_duration(attribute_store_node_t state_node, - uint32_t duration) +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( @@ -47,21 +47,7 @@ static void set_desired_duration(attribute_store_node_t state_node, ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_DURATION); attribute_store_set_node_attribute_value(duration_node, - DESIRED_ATTRIBUTE, - (uint8_t *)&duration, - sizeof(duration)); -} - -static void set_reported_duration(attribute_store_node_t state_node, - 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, - REPORTED_ATTRIBUTE, + state, (uint8_t *)&duration, sizeof(duration)); } @@ -80,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, @@ -166,7 +144,7 @@ static void state_node, ATTRIBUTE_COMMAND_CLASS_SWITCH_COLOR_VALUE, &attribute_store_undefine_desired); - set_reported_duration(state_node, 0); + set_duration(state_node, REPORTED_ATTRIBUTE, 0); attribute_store_undefine_desired(duration_node); break; From 5dd5084b2028674f93e0b9798fc4f52e98220f52 Mon Sep 17 00:00:00 2001 From: Philippe Coval Date: Mon, 22 Jan 2024 14:52:45 +0100 Subject: [PATCH 3/4] GH-24: zpc: Update test for switch color Signed-off-by: Philippe Coval (cherry picked from commit 7cf96bbd299634ed7bc393eae1d017bf5bf07d21) Bug-SiliconLabs: UIC-3082 Relate-to: https://github.com/SiliconLabs/UnifySDK/pull/24 Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-24/develop Origin: https://github.com/nkljajic/UnifySDK/pull/5 Signed-off-by: Philippe Coval --- .../zwave_command_class_switch_color_test.c | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/applications/zpc/components/zwave_command_classes/test/zwave_command_class_switch_color_test.c b/applications/zpc/components/zwave_command_classes/test/zwave_command_class_switch_color_test.c index b768a9c9da..5a69252ac8 100644 --- a/applications/zpc/components/zwave_command_classes/test/zwave_command_class_switch_color_test.c +++ b/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,6 +170,60 @@ 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); @@ -162,3 +231,32 @@ void test_zwave_command_class_on_send_complete() 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))); + } +} \ No newline at end of file From 3bcf29f25cac58be208b052242e0831a62c04e82 Mon Sep 17 00:00:00 2001 From: Philippe Coval Date: Thu, 9 Nov 2023 11:09:02 +0100 Subject: [PATCH 4/4] GH-14: zpc: Enable testing by default More to be tested Bug-SiliconLabs: UIC-3082 Bug-GitHub: https://github.com/rzr/UnifySDK/issues/3 Forwarded: https://github.com/SiliconLabs/UnifySDK/pull/26 Origin: https://github.com/nkljajic/UnifySDK/pull/5 Signed-off-by: Philippe Coval --- helper.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper.mk b/helper.mk index 6487374ba0..2c930c08c7 100755 --- a/helper.mk +++ b/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