From b1c6a0e74b5117f6e6db4267a8a3c6a3735e8c03 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Thu, 4 Dec 2025 20:28:43 -0600 Subject: [PATCH] Fixed writing to the Channel object when no member value coercion was required. (#1176) * Updated documentation for encode functions to accurately describe return values (0 on error instead of BACNET_STATUS_ERROR) * Expanded property support, removed INPUT object types, added fallback encoding for non-coerced data types, and updated error handling in ReadProperty of present-value to check for 0 instead of BACNET_STATUS_ERROR. * Expanded the validation tests and increased test code coverage. --- CHANGELOG.md | 7 +- src/bacnet/basic/object/channel.c | 97 +++++++++---------- src/bacnet/channel_value.c | 4 +- .../basic/object/channel/CMakeLists.txt | 1 + test/bacnet/basic/object/channel/src/main.c | 52 +++++++++- 5 files changed, 106 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd6c7b4..2175d312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ The git repositories are hosted at the following sites: ### Added +* Added properties to the Channel object write member value coercion + minimal properties supported. (#1176) * Added Send_x_Address() API to ReadPropertyMultiple, WritePropertyMultiple, and SubscribeSOV services primarily for interacting with MS/TP slaves (#1174) * Added npdu_set_i_am_router_to_network_handler() API. Fixed sending to @@ -54,9 +56,12 @@ The git repositories are hosted at the following sites: ### Fixed +* Fixed the Channel object to handle all data types that do not need + coercion when written. Fixed present-value when no value is able to + be encoded. (#1176) * Fixed the Loop object read/write references and manipulated variables update during timer loop by adding callbacks to device read/write property - in basic example device object. (##1175) + in basic example device object. (#1175) * Fixed library specific strcmp/stricmp functions match standard strcmp. (#1173) * Fixed compiler macro redefined warning when optional datatypes are defined globally. (#1172) diff --git a/src/bacnet/basic/object/channel.c b/src/bacnet/basic/object/channel.c index f75a6cd1..522c6ab5 100644 --- a/src/bacnet/basic/object/channel.c +++ b/src/bacnet/basic/object/channel.c @@ -581,10 +581,10 @@ bool Channel_Write_Member_Value( int apdu_len = 0; if (wp_data && value) { - if (((wp_data->object_type == OBJECT_ANALOG_INPUT) || - (wp_data->object_type == OBJECT_ANALOG_OUTPUT) || + if (((wp_data->object_type == OBJECT_ANALOG_OUTPUT) || (wp_data->object_type == OBJECT_ANALOG_VALUE)) && - (wp_data->object_property == PROP_PRESENT_VALUE) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_RELINQUISH_DEFAULT)) && (wp_data->array_index == BACNET_ARRAY_ALL)) { apdu_len = bacnet_channel_value_coerce_data_encode( wp_data->application_data, wp_data->application_data_len, value, @@ -594,10 +594,10 @@ bool Channel_Write_Member_Value( status = true; } } else if ( - ((wp_data->object_type == OBJECT_BINARY_INPUT) || - (wp_data->object_type == OBJECT_BINARY_OUTPUT) || + ((wp_data->object_type == OBJECT_BINARY_OUTPUT) || (wp_data->object_type == OBJECT_BINARY_VALUE)) && - (wp_data->object_property == PROP_PRESENT_VALUE) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_RELINQUISH_DEFAULT)) && (wp_data->array_index == BACNET_ARRAY_ALL)) { apdu_len = bacnet_channel_value_coerce_data_encode( wp_data->application_data, wp_data->application_data_len, value, @@ -607,10 +607,10 @@ bool Channel_Write_Member_Value( status = true; } } else if ( - ((wp_data->object_type == OBJECT_MULTI_STATE_INPUT) || - (wp_data->object_type == OBJECT_MULTI_STATE_OUTPUT) || + ((wp_data->object_type == OBJECT_MULTI_STATE_OUTPUT) || (wp_data->object_type == OBJECT_MULTI_STATE_VALUE)) && - (wp_data->object_property == PROP_PRESENT_VALUE) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_RELINQUISH_DEFAULT)) && (wp_data->array_index == BACNET_ARRAY_ALL)) { apdu_len = bacnet_channel_value_coerce_data_encode( wp_data->application_data, wp_data->application_data_len, value, @@ -619,49 +619,35 @@ bool Channel_Write_Member_Value( wp_data->application_data_len = apdu_len; status = true; } - } else if (wp_data->object_type == OBJECT_LIGHTING_OUTPUT) { - if ((wp_data->object_property == PROP_PRESENT_VALUE) && - (wp_data->array_index == BACNET_ARRAY_ALL)) { - apdu_len = bacnet_channel_value_coerce_data_encode( - wp_data->application_data, wp_data->application_data_len, - value, BACNET_APPLICATION_TAG_REAL); - if (apdu_len != BACNET_STATUS_ERROR) { - wp_data->application_data_len = apdu_len; - status = true; - } - } else if ( - (wp_data->object_property == PROP_LIGHTING_COMMAND) && - (wp_data->array_index == BACNET_ARRAY_ALL)) { - apdu_len = bacnet_channel_value_coerce_data_encode( - wp_data->application_data, wp_data->application_data_len, - value, BACNET_APPLICATION_TAG_LIGHTING_COMMAND); - if (apdu_len != BACNET_STATUS_ERROR) { - wp_data->application_data_len = apdu_len; - status = true; - } + } else if ( + (wp_data->object_type == OBJECT_LIGHTING_OUTPUT) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_RELINQUISH_DEFAULT)) && + (wp_data->array_index == BACNET_ARRAY_ALL)) { + apdu_len = bacnet_channel_value_coerce_data_encode( + wp_data->application_data, wp_data->application_data_len, value, + BACNET_APPLICATION_TAG_REAL); + if (apdu_len != BACNET_STATUS_ERROR) { + wp_data->application_data_len = apdu_len; + status = true; } - } else if (wp_data->object_type == OBJECT_COLOR) { - if ((wp_data->object_property == PROP_PRESENT_VALUE) && - (wp_data->array_index == BACNET_ARRAY_ALL)) { - apdu_len = bacnet_channel_value_coerce_data_encode( - wp_data->application_data, wp_data->application_data_len, - value, BACNET_APPLICATION_TAG_XY_COLOR); - if (apdu_len != BACNET_STATUS_ERROR) { - wp_data->application_data_len = apdu_len; - status = true; - } - } else if ( - (wp_data->object_property == PROP_COLOR_COMMAND) && - (wp_data->array_index == BACNET_ARRAY_ALL)) { - apdu_len = bacnet_channel_value_coerce_data_encode( - wp_data->application_data, wp_data->application_data_len, - value, BACNET_APPLICATION_TAG_COLOR_COMMAND); - if (apdu_len != BACNET_STATUS_ERROR) { - wp_data->application_data_len = apdu_len; - status = true; - } + } else if ( + (wp_data->object_type == OBJECT_COLOR) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_DEFAULT_COLOR)) && + (wp_data->array_index == BACNET_ARRAY_ALL)) { + apdu_len = bacnet_channel_value_coerce_data_encode( + wp_data->application_data, wp_data->application_data_len, value, + BACNET_APPLICATION_TAG_XY_COLOR); + if (apdu_len != BACNET_STATUS_ERROR) { + wp_data->application_data_len = apdu_len; + status = true; } - } else if (wp_data->object_type == OBJECT_COLOR_TEMPERATURE) { + } else if ( + (wp_data->object_type == OBJECT_COLOR_TEMPERATURE) && + ((wp_data->object_property == PROP_PRESENT_VALUE) || + (wp_data->object_property == PROP_DEFAULT_COLOR_TEMPERATURE)) && + (wp_data->array_index == BACNET_ARRAY_ALL)) { apdu_len = bacnet_channel_value_coerce_data_encode( wp_data->application_data, wp_data->application_data_len, value, BACNET_APPLICATION_TAG_UNSIGNED_INT); @@ -669,6 +655,15 @@ bool Channel_Write_Member_Value( wp_data->application_data_len = apdu_len; status = true; } + } else { + /* no coercion */ + apdu_len = bacnet_channel_value_encode( + wp_data->application_data, wp_data->application_data_len, + value); + if (apdu_len > 0) { + wp_data->application_data_len = apdu_len; + status = true; + } } } @@ -991,7 +986,7 @@ int Channel_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) case PROP_PRESENT_VALUE: cvalue = Channel_Present_Value(rpdata->object_instance); apdu_len = bacnet_channel_value_encode(apdu, apdu_size, cvalue); - if (apdu_len == BACNET_STATUS_ERROR) { + if (apdu_len == 0) { apdu_len = encode_application_null(apdu); } break; diff --git a/src/bacnet/channel_value.c b/src/bacnet/channel_value.c index 15c07621..336cf02c 100644 --- a/src/bacnet/channel_value.c +++ b/src/bacnet/channel_value.c @@ -24,7 +24,7 @@ * @brief Encode a given BACnetChanneValue * @param apdu - APDU buffer for storing the encoded data, or NULL for length * @param value - BACNET_CHANNEL_VALUE value - * @return number of bytes in the APDU + * @return number of bytes in the APDU, or zero if unable to encode */ int bacnet_channel_value_type_encode( uint8_t *apdu, const BACNET_CHANNEL_VALUE *value) @@ -245,7 +245,7 @@ int bacnet_channel_value_type_decode( * @param apdu - APDU buffer for storing the encoded data, or NULL for length * @param apdu_size - size of the APDU buffer * @param value - BACNET_CHANNEL_VALUE value - * @return number of bytes in the APDU, or BACNET_STATUS_ERROR + * @return number of bytes in the APDU, or zero if unable to encode */ int bacnet_channel_value_encode( uint8_t *apdu, size_t apdu_size, const BACNET_CHANNEL_VALUE *value) diff --git a/test/bacnet/basic/object/channel/CMakeLists.txt b/test/bacnet/basic/object/channel/CMakeLists.txt index 1cdbb814..6e85a752 100644 --- a/test/bacnet/basic/object/channel/CMakeLists.txt +++ b/test/bacnet/basic/object/channel/CMakeLists.txt @@ -23,6 +23,7 @@ set(ZTST_DIR "${TST_DIR}/ztest/src") add_compile_definitions( BIG_ENDIAN=0 CONFIG_ZTEST=1 + CHANNEL_MEMBERS_MAX=16 ) include_directories( diff --git a/test/bacnet/basic/object/channel/src/main.c b/test/bacnet/basic/object/channel/src/main.c index d9d9e38a..a2a5d169 100644 --- a/test/bacnet/basic/object/channel/src/main.c +++ b/test/bacnet/basic/object/channel/src/main.c @@ -16,6 +16,15 @@ * @addtogroup bacnet_tests * @{ */ +static BACNET_WRITE_PROPERTY_DATA Write_Property_Internal_Data; +static bool Write_Property_Internal(BACNET_WRITE_PROPERTY_DATA *wp_data) +{ + memcpy( + &Write_Property_Internal_Data, wp_data, + sizeof(BACNET_WRITE_PROPERTY_DATA)); + + return true; +} /** * @brief Test @@ -32,9 +41,12 @@ static void test_Channel_Property_Read_Write(void) const int32_t skip_fail_property_list[] = { -1 }; BACNET_CHANNEL_VALUE channel_value = { 0 }; BACNET_WRITE_PROPERTY_DATA wp_data = { 0 }; + BACNET_WRITE_GROUP_DATA wg_data = { 0 }; + BACNET_APPLICATION_DATA_VALUE value = { 0 }; BACNET_DEVICE_OBJECT_PROPERTY_REFERENCE member = { 0 }; + Channel_Write_Property_Internal_Callback_Set(Write_Property_Internal); Channel_Init(); Channel_Create(instance); status = Channel_Valid_Instance(instance); @@ -52,8 +64,8 @@ static void test_Channel_Property_Read_Write(void) member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_ANALOG_OUTPUT; member.objectIdentifier.instance = 1; - member.propertyIdentifier = PROP_PRESENT_VALUE; member.arrayIndex = BACNET_ARRAY_ALL; + member.propertyIdentifier = PROP_PRESENT_VALUE; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); status = @@ -61,6 +73,9 @@ static void test_Channel_Property_Read_Write(void) zassert_true(status, NULL); status = Channel_Control_Groups_Element_Set(instance, 1, 1); zassert_true(status, NULL); + member.propertyIdentifier = PROP_RELINQUISH_DEFAULT; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); member.deviceIdentifier.type = OBJECT_DEVICE; member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_BINARY_OUTPUT; @@ -69,6 +84,9 @@ static void test_Channel_Property_Read_Write(void) member.arrayIndex = BACNET_ARRAY_ALL; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_RELINQUISH_DEFAULT; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); member.deviceIdentifier.type = OBJECT_DEVICE; member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_MULTI_STATE_OUTPUT; @@ -77,6 +95,9 @@ static void test_Channel_Property_Read_Write(void) member.arrayIndex = BACNET_ARRAY_ALL; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_RELINQUISH_DEFAULT; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); member.deviceIdentifier.type = OBJECT_DEVICE; member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_LIGHTING_OUTPUT; @@ -85,6 +106,12 @@ static void test_Channel_Property_Read_Write(void) member.arrayIndex = BACNET_ARRAY_ALL; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_RELINQUISH_DEFAULT; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_HIGH_END_TRIM; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); member.deviceIdentifier.type = OBJECT_DEVICE; member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_COLOR; @@ -93,6 +120,9 @@ static void test_Channel_Property_Read_Write(void) member.arrayIndex = BACNET_ARRAY_ALL; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_DEFAULT_COLOR; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); member.deviceIdentifier.type = OBJECT_DEVICE; member.deviceIdentifier.instance = 0; member.objectIdentifier.type = OBJECT_COLOR_TEMPERATURE; @@ -101,6 +131,9 @@ static void test_Channel_Property_Read_Write(void) member.arrayIndex = BACNET_ARRAY_ALL; index = Channel_Reference_List_Member_Element_Add(instance, &member); zassert_not_equal(index, 0, NULL); + member.propertyIdentifier = PROP_DEFAULT_COLOR_TEMPERATURE; + index = Channel_Reference_List_Member_Element_Add(instance, &member); + zassert_not_equal(index, 0, NULL); /* perform a general test for RP/WP */ bacnet_object_properties_read_write_test( OBJECT_CHANNEL, instance, Channel_Property_Lists, Channel_Read_Property, @@ -129,6 +162,13 @@ static void test_Channel_Property_Read_Write(void) bacapp_encode_application_data(wp_data.application_data, &value); status = Channel_Write_Property(&wp_data); zassert_true(status, NULL); + value.type.Channel_Value.tag = BACNET_APPLICATION_TAG_XY_COLOR; + value.type.Channel_Value.type.XY_Color.x_coordinate = 0.4590f; + value.type.Channel_Value.type.XY_Color.y_coordinate = 0.4101f; + wp_data.application_data_len = + bacapp_encode_application_data(wp_data.application_data, &value); + status = Channel_Write_Property(&wp_data); + zassert_true(status, NULL); /* specific WriteProperty value */ wp_data.object_property = PROP_OUT_OF_SERVICE; value.tag = BACNET_APPLICATION_TAG_BOOLEAN; @@ -235,6 +275,16 @@ static void test_Channel_Property_Read_Write(void) channel_value.type.Real = 3.14159f; status = Channel_Present_Value_Set(instance, 1, &channel_value); zassert_true(status, NULL); + /* context API */ + Channel_Context_Set(instance, Channel_Context_Get(instance)); + /* WriteGroup API */ + wg_data.change_list.channel = 1; + wg_data.change_list.overriding_priority = BACNET_MAX_PRIORITY; + wg_data.change_list.value.tag = BACNET_APPLICATION_TAG_REAL; + wg_data.change_list.value.type.Real = 2.71828f; + wg_data.inhibit_delay = 0; + wg_data.write_priority = BACNET_MAX_PRIORITY; + Channel_Write_Group(&wg_data, 0, &wg_data.change_list); /* cleanup */ status = Channel_Delete(instance); zassert_true(status, NULL);