From 5a9abde528124964334af6e839c7c894a5fd4af7 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Thu, 18 Dec 2025 11:08:28 -0600 Subject: [PATCH] Fixed channel-value encoding in the channel object when no-coercian is required for lighting-command, color-command, and xy-color. (#1190) --- CHANGELOG.md | 2 + src/bacnet/basic/object/channel.c | 2 +- src/bacnet/channel_value.c | 200 +++++++++++++++++++++++++++ src/bacnet/channel_value.h | 12 ++ test/bacnet/channel_value/src/main.c | 29 +++- 5 files changed, 243 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30365edb..5027dfe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,8 @@ The git repositories are hosted at the following sites: ### Fixed +* Fixed channel-value encoding in the channel object when no-coercian + is required for lighting-command, color-command, and xy-color. (#1190) * Fixed NULL handling in CharacterString sprintf which caused an endless loop. (#1189) * Fixed a regression in the rpm_ack_object_property_process() function diff --git a/src/bacnet/basic/object/channel.c b/src/bacnet/basic/object/channel.c index 522c6ab5..df28b69c 100644 --- a/src/bacnet/basic/object/channel.c +++ b/src/bacnet/basic/object/channel.c @@ -657,7 +657,7 @@ bool Channel_Write_Member_Value( } } else { /* no coercion */ - apdu_len = bacnet_channel_value_encode( + apdu_len = bacnet_channel_value_no_coerce_encode( wp_data->application_data, wp_data->application_data_len, value); if (apdu_len > 0) { diff --git a/src/bacnet/channel_value.c b/src/bacnet/channel_value.c index af818a2f..dcf17f25 100644 --- a/src/bacnet/channel_value.c +++ b/src/bacnet/channel_value.c @@ -1164,3 +1164,203 @@ int bacnet_channel_value_coerce_data_encode( return len; } + +/** + * @brief Encode a given BACnetChannelValue to use for internal writes + * This function encodes the complex data types without context tags + * normally used for BACnetChannelValue. + * @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, or zero if unable to encode + */ +int bacnet_channel_value_no_coerce_type_encode( + uint8_t *apdu, const BACNET_CHANNEL_VALUE *value) +{ + int apdu_len = 0; + + if (!value) { + return 0; + } + switch (value->tag) { +#if defined(CHANNEL_LIGHTING_COMMAND) + case BACNET_APPLICATION_TAG_LIGHTING_COMMAND: + apdu_len = + lighting_command_encode(apdu, &value->type.Lighting_Command); + break; +#endif +#if defined(CHANNEL_COLOR_COMMAND) + case BACNET_APPLICATION_TAG_COLOR_COMMAND: + apdu_len = color_command_encode(apdu, &value->type.Color_Command); + break; +#endif +#if defined(CHANNEL_XY_COLOR) + case BACNET_APPLICATION_TAG_XY_COLOR: + apdu_len = xy_color_encode(apdu, &value->type.XY_Color); + break; +#endif + default: + apdu_len = bacnet_channel_value_type_encode(apdu, value); + break; + } + + return apdu_len; +} + +/** + * @brief Encode a given BACnetChannelValue to use for internal writes + * This function encodes the complex data types without context tags + * normally used for BACnetChannelValue. + * @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 zero if unable to encode + */ +int bacnet_channel_value_no_coerce_encode( + uint8_t *apdu, size_t apdu_size, const BACNET_CHANNEL_VALUE *value) +{ + size_t apdu_len = 0; /* total length of the apdu, return value */ + + apdu_len = bacnet_channel_value_no_coerce_type_encode(NULL, value); + if (apdu_len > apdu_size) { + apdu_len = 0; + } else { + apdu_len = bacnet_channel_value_no_coerce_type_encode(apdu, value); + } + + return apdu_len; +} + +/** + * @brief Decode a given BACnetChannelValue to use for internal writes + * This function decodes the complex data types without context tags + * normally used for BACnetChannelValue. + * @param apdu - APDU buffer for decoding + * @param apdu_size - Count of valid bytes in the buffer + * @param tag - application tag of the data to decode + * @param value - BACNET_CHANNEL_VALUE value to store the decoded data + * @return number of bytes decoded or BACNET_STATUS_ERROR on error + */ +int bacnet_channel_value_no_coerce_decode( + const uint8_t *apdu, + size_t apdu_size, + uint8_t tag, + BACNET_CHANNEL_VALUE *value) +{ + int len = 0; + + if (!apdu) { + return BACNET_STATUS_ERROR; + } + if (!value) { + return BACNET_STATUS_ERROR; + } + switch (tag) { + case BACNET_APPLICATION_TAG_NULL: + /* nothing else to do */ + break; +#if defined(CHANNEL_BOOLEAN) + case BACNET_APPLICATION_TAG_BOOLEAN: + len = bacnet_boolean_application_decode( + apdu, apdu_size, &value->type.Boolean); + break; +#endif +#if defined(CHANNEL_UNSIGNED) + case BACNET_APPLICATION_TAG_UNSIGNED_INT: + len = bacnet_unsigned_application_decode( + apdu, apdu_size, &value->type.Unsigned_Int); + break; +#endif +#if defined(CHANNEL_SIGNED) + case BACNET_APPLICATION_TAG_SIGNED_INT: + len = bacnet_signed_application_decode( + apdu, apdu_size, &value->type.Signed_Int); + break; +#endif +#if defined(CHANNEL_REAL) + case BACNET_APPLICATION_TAG_REAL: + len = bacnet_real_application_decode( + apdu, apdu_size, &value->type.Real); + break; +#endif +#if defined(CHANNEL_DOUBLE) + case BACNET_APPLICATION_TAG_DOUBLE: + len = bacnet_double_application_decode( + apdu, apdu_size, &value->type.Double); + break; +#endif +#if defined(CHANNEL_OCTET_STRING) + case BACNET_APPLICATION_TAG_OCTET_STRING: + len = bacnet_octet_string_application_decode( + apdu, apdu_size, &value->type.Octet_String); + break; +#endif +#if defined(CHANNEL_CHARACTER_STRING) + case BACNET_APPLICATION_TAG_CHARACTER_STRING: + len = bacnet_character_string_application_decode( + apdu, apdu_size, &value->type.Character_String); + break; +#endif +#if defined(CHANNEL_BIT_STRING) + case BACNET_APPLICATION_TAG_BIT_STRING: + len = bacnet_bitstring_application_decode( + apdu, apdu_size, &value->type.Bit_String); + break; +#endif +#if defined(CHANNEL_ENUMERATED) + case BACNET_APPLICATION_TAG_ENUMERATED: + len = bacnet_enumerated_application_decode( + apdu, apdu_size, &value->type.Enumerated); + break; +#endif +#if defined(CHANNEL_DATE) + case BACNET_APPLICATION_TAG_DATE: + len = bacnet_date_application_decode( + apdu, apdu_size, &value->type.Date); + break; +#endif +#if defined(CHANNEL_TIME) + case BACNET_APPLICATION_TAG_TIME: + len = bacnet_time_application_decode( + apdu, apdu_size, &value->type.Time); + break; +#endif +#if defined(CHANNEL_OBJECT_ID) + case BACNET_APPLICATION_TAG_OBJECT_ID: + len = bacnet_object_id_application_decode( + apdu, apdu_size, &value->type.Object_Id.type, + &value->type.Object_Id.instance); + break; +#endif +#if defined(CHANNEL_LIGHTING_COMMAND) + case BACNET_APPLICATION_TAG_LIGHTING_COMMAND: + len = lighting_command_decode( + apdu, apdu_size, &value->type.Lighting_Command); + break; +#endif +#if defined(CHANNEL_COLOR_COMMAND) + case BACNET_APPLICATION_TAG_COLOR_COMMAND: + len = color_command_decode( + apdu, apdu_size, NULL, &value->type.Color_Command); + break; +#endif +#if defined(CHANNEL_XY_COLOR) + case BACNET_APPLICATION_TAG_XY_COLOR: + len = xy_color_decode(apdu, apdu_size, &value->type.XY_Color); + break; +#endif + default: + len = BACNET_STATUS_ERROR; + break; + } + if ((len == 0) && (tag != BACNET_APPLICATION_TAG_NULL) && + (tag != BACNET_APPLICATION_TAG_BOOLEAN) && + (tag != BACNET_APPLICATION_TAG_OCTET_STRING)) { + /* indicate that we were not able to decode the value */ + len = BACNET_STATUS_ERROR; + } + if (len != BACNET_STATUS_ERROR) { + value->tag = tag; + } + + return len; +} diff --git a/src/bacnet/channel_value.h b/src/bacnet/channel_value.h index c166bfab..67b7a8fd 100644 --- a/src/bacnet/channel_value.h +++ b/src/bacnet/channel_value.h @@ -165,6 +165,18 @@ int bacnet_channel_value_coerce_data_encode( size_t apdu_size, const BACNET_CHANNEL_VALUE *value, BACNET_APPLICATION_TAG tag); +BACNET_STACK_EXPORT +int bacnet_channel_value_no_coerce_type_encode( + uint8_t *apdu, const BACNET_CHANNEL_VALUE *value); +BACNET_STACK_EXPORT +int bacnet_channel_value_no_coerce_encode( + uint8_t *apdu, size_t apdu_size, const BACNET_CHANNEL_VALUE *value); +BACNET_STACK_EXPORT +int bacnet_channel_value_no_coerce_decode( + const uint8_t *apdu, + size_t apdu_size, + uint8_t tag, + BACNET_CHANNEL_VALUE *value); #ifdef __cplusplus } diff --git a/test/bacnet/channel_value/src/main.c b/test/bacnet/channel_value/src/main.c index eba597b5..5e3c5eda 100644 --- a/test/bacnet/channel_value/src/main.c +++ b/test/bacnet/channel_value/src/main.c @@ -402,7 +402,7 @@ static void test_BACnetChannelValue(void) bacnet_channel_value_link_array(case_value, ARRAY_SIZE(case_value)); value = &case_value[0]; while (value) { - /* no coercion path */ + /* write-group encode/decode path */ null_len = bacnet_channel_value_encode(NULL, sizeof(apdu), value); if (value->tag != BACNET_APPLICATION_TAG_NULL) { zassert_not_equal(null_len, 0, NULL); @@ -464,6 +464,33 @@ static void test_BACnetChannelValue(void) zassert_true( status, "decode: different: %s", bactext_application_tag_name(value->tag)); + /* no-coercion path */ + null_len = + bacnet_channel_value_no_coerce_encode(NULL, sizeof(apdu), value); + if (value->tag != BACNET_APPLICATION_TAG_NULL) { + zassert_not_equal(null_len, 0, NULL); + } + apdu_len = + bacnet_channel_value_no_coerce_encode(apdu, sizeof(apdu), value); + zassert_equal( + apdu_len, null_len, "value->tag: %s len=%d null_len=%d", + bactext_application_tag_name(value->tag), apdu_len, null_len); + test_len = bacnet_channel_value_no_coerce_decode( + apdu, apdu_len, test_value.tag, NULL); + zassert_equal(test_len, BACNET_STATUS_ERROR, NULL); + test_len = bacnet_channel_value_no_coerce_decode( + NULL, apdu_len, test_value.tag, &test_value); + zassert_equal(test_len, BACNET_STATUS_ERROR, NULL); + test_len = bacnet_channel_value_no_coerce_decode( + apdu, apdu_len, test_value.tag, &test_value); + zassert_not_equal( + test_len, BACNET_STATUS_ERROR, "value->tag: %s test_len=%d", + bactext_application_tag_name(value->tag), test_len); + if (value->tag != BACNET_APPLICATION_TAG_NULL) { + zassert_equal( + test_len, apdu_len, "value->tag: %s test_len=%d", + bactext_application_tag_name(value->tag), test_len); + } /* next test case */ value = value->next; }