From 4ee129e249353ec58bd05c1031217ad1de219a0e Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Mon, 10 Mar 2025 07:30:55 -0500 Subject: [PATCH] Added bypass in basic WriteProperty handler to accept writes of NULL to non-commandable properties (#919) --- apps/blinkt/device.c | 11 +++++++ ports/stm32f10x/device.c | 11 +++++++ ports/stm32f4xx/device.c | 40 +++++++++++++++++++++++ src/bacnet/bacdcode.c | 32 ++++++++++++++++++ src/bacnet/bacdcode.h | 2 ++ src/bacnet/basic/service/h_wp.c | 41 ++++++++++++++++++++++-- src/bacnet/wp.c | 57 +++++++++++++++++++++++++++++++++ src/bacnet/wp.h | 17 ++++++++++ test/bacnet/wp/src/main.c | 50 ++++++++++++++++++++++++++--- 9 files changed, 253 insertions(+), 8 deletions(-) diff --git a/apps/blinkt/device.c b/apps/blinkt/device.c index 207e9d6b..84f71ed7 100644 --- a/apps/blinkt/device.c +++ b/apps/blinkt/device.c @@ -384,6 +384,17 @@ static const int Device_Properties_Optional[] = { static const int Device_Properties_Proprietary[] = { -1 }; +/** + * @brief Returns the list of required, optional, and proprietary properties + * for the Device object. + * @param pRequired [out] Pointer to the list of required properties + * @param pOptional [out] Pointer to the list of optional properties + * @param pProprietary [out] Pointer to the list of proprietary properties + * @note The lists are terminated with -1. + * @note The lists are not allocated, so do not free them. + * @note The lists are static, so do not modify them. + * @ingroup ObjIntf + */ void Device_Property_Lists( const int **pRequired, const int **pOptional, const int **pProprietary) { diff --git a/ports/stm32f10x/device.c b/ports/stm32f10x/device.c index adfbac1c..8c508249 100644 --- a/ports/stm32f10x/device.c +++ b/ports/stm32f10x/device.c @@ -281,6 +281,17 @@ bool Device_Objects_Property_List_Member( return found; } +/** + * @brief Returns the list of required, optional, and proprietary properties + * for the Device object. + * @param pRequired [out] Pointer to the list of required properties + * @param pOptional [out] Pointer to the list of optional properties + * @param pProprietary [out] Pointer to the list of proprietary properties + * @note The lists are terminated with -1. + * @note The lists are not allocated, so do not free them. + * @note The lists are static, so do not modify them. + * @ingroup ObjIntf + */ void Device_Property_Lists( const int **pRequired, const int **pOptional, const int **pProprietary) { diff --git a/ports/stm32f4xx/device.c b/ports/stm32f4xx/device.c index 735a81dd..60b2e5c3 100644 --- a/ports/stm32f4xx/device.c +++ b/ports/stm32f4xx/device.c @@ -220,6 +220,46 @@ void Device_Objects_Property_List(BACNET_OBJECT_TYPE object_type, return; } +/** + * @brief Determine if the object property is a member of this object instance + * @param object_type - object type of the object + * @param object_instance - object-instance number of the object + * @param object_property - object-property to be checked + * @return true if the property is a member of this object instance + */ +bool Device_Objects_Property_List_Member( + BACNET_OBJECT_TYPE object_type, + uint32_t object_instance, + BACNET_PROPERTY_ID object_property) +{ + bool found = false; + struct special_property_list_t property_list = { 0 }; + + Device_Objects_Property_List(object_type, object_instance, &property_list); + found = property_list_member(property_list.Required.pList, object_property); + if (!found) { + found = + property_list_member(property_list.Optional.pList, object_property); + } + if (!found) { + found = property_list_member( + property_list.Proprietary.pList, object_property); + } + + return found; +} + +/** + * @brief Returns the list of required, optional, and proprietary properties + * for the Device object. + * @param pRequired [out] Pointer to the list of required properties + * @param pOptional [out] Pointer to the list of optional properties + * @param pProprietary [out] Pointer to the list of proprietary properties + * @note The lists are terminated with -1. + * @note The lists are not allocated, so do not free them. + * @note The lists are static, so do not modify them. + * @ingroup ObjIntf + */ void Device_Property_Lists( const int **pRequired, const int **pOptional, const int **pProprietary) { diff --git a/src/bacnet/bacdcode.c b/src/bacnet/bacdcode.c index c0e4a014..08d417d0 100644 --- a/src/bacnet/bacdcode.c +++ b/src/bacnet/bacdcode.c @@ -1360,6 +1360,38 @@ int encode_context_null(uint8_t *apdu, uint8_t tag_number) return encode_tag(apdu, tag_number, true, 0); } +/** + * @brief Decode the NULL value when application encoded + * From clause 20.2.2 Encoding of a Null Value + * and 20.2.1 General Rules for Encoding BACnet Tags + * + * @param apdu - buffer of data to be decoded + * @param apdu_size - number of bytes in the buffer + * + * @return number of bytes decoded, zero if tag mismatch, + * or #BACNET_STATUS_ERROR (-1) if malformed + */ +int bacnet_null_application_decode(const uint8_t *apdu, uint32_t apdu_size) +{ + int apdu_len = BACNET_STATUS_ERROR; + int len = 0; + BACNET_TAG tag = { 0 }; + + if (apdu_size == 0) { + return 0; + } + len = bacnet_tag_decode(apdu, apdu_size, &tag); + if (len > 0) { + if (tag.application && (tag.number == BACNET_APPLICATION_TAG_NULL)) { + apdu_len = len; + } else { + apdu_len = 0; + } + } + + return apdu_len; +} + /** * @brief Reverse the bits of the given byte. * diff --git a/src/bacnet/bacdcode.h b/src/bacnet/bacdcode.h index afc215b0..36f25e00 100644 --- a/src/bacnet/bacdcode.h +++ b/src/bacnet/bacdcode.h @@ -162,6 +162,8 @@ BACNET_STACK_EXPORT int encode_application_null(uint8_t *apdu); BACNET_STACK_EXPORT int encode_context_null(uint8_t *apdu, uint8_t tag_number); +BACNET_STACK_EXPORT +int bacnet_null_application_decode(const uint8_t *apdu, uint32_t apdu_size); /* from clause 20.2.3 Encoding of a Boolean Value */ BACNET_STACK_EXPORT diff --git a/src/bacnet/basic/service/h_wp.c b/src/bacnet/basic/service/h_wp.c index 65f7e5a9..311fa3cc 100644 --- a/src/bacnet/basic/service/h_wp.c +++ b/src/bacnet/basic/service/h_wp.c @@ -28,6 +28,34 @@ /** @file h_wp.c Handles Write Property requests. */ +/** + * @brief Handler for a WriteProperty Service request when the + * property is a NULL type and the property is not commandable + * + * 15.9.2 WriteProperty Service Procedure + * + * If an attempt is made to relinquish a property that is + * not commandable and for which Null is not a supported + * datatype, if no other error conditions exist, + * the property shall not be changed, and the write shall + * be considered successful. + * + * @param wp_data [in] The WriteProperty data structure + * @return true if the write shall be considered successful + */ +static bool +handler_write_property_relinquish_bypass(BACNET_WRITE_PROPERTY_DATA *wp_data) +{ + bool status = false; + +#if BACNET_PROTOCOL_REVISION >= 21 + status = write_property_relinquish_bypass( + wp_data, Device_Objects_Property_List_Member); +#endif + + return status; +} + /** Handler for a WriteProperty Service request. * @ingroup DSWP * This handler will be invoked by apdu_handler() if it has been enabled @@ -55,7 +83,7 @@ void handler_write_property( BACNET_WRITE_PROPERTY_DATA wp_data; int len = 0; bool bcontinue = true; - bool success; + bool success = false; int pdu_len = 0; BACNET_NPDU_DATA npdu_data; int bytes_sent = 0; @@ -104,9 +132,16 @@ void handler_write_property( bcontinue = false; } if (bcontinue) { - success = write_property_bacnet_array_valid(&wp_data); + success = handler_write_property_relinquish_bypass(&wp_data); if (success) { - success = Device_Write_Property(&wp_data); + /* this object property is not commandable, + and therefore, not able to be relinquished, + so it "shall not be changed, and + the write shall be considered successful." */ + } else { + if (write_property_bacnet_array_valid(&wp_data)) { + success = Device_Write_Property(&wp_data); + } } if (success) { len = encode_simple_ack( diff --git a/src/bacnet/wp.c b/src/bacnet/wp.c index f2ad2c5d..b7870abd 100644 --- a/src/bacnet/wp.c +++ b/src/bacnet/wp.c @@ -487,3 +487,60 @@ bool write_property_unsigned_decode( return status; } + +/** + * @brief Handler for a WriteProperty Service request when the + * property is a NULL type and the property is not commandable + * + * 15.9.2 WriteProperty Service Procedure + * + * If an attempt is made to relinquish a property that is + * not commandable and for which Null is not a supported + * datatype, if no other error conditions exist, + * the property shall not be changed, and the write shall + * be considered successful. + * + * @param wp_data [in] The WriteProperty data structure + * @param member_of_object [in] Function to check if a property is a member + of an object instance + * @return true if the write shall be considered successful + */ +bool write_property_relinquish_bypass( + BACNET_WRITE_PROPERTY_DATA *wp_data, + write_property_member_of_object member_of_object) +{ + bool bypass = false; + bool has_priority_array = false; + int len = 0; + + if (!wp_data) { + return false; + } + len = bacnet_null_application_decode( + wp_data->application_data, wp_data->application_data_len); + if ((len > 0) && (len == wp_data->application_data_len)) { + /* single NULL */ + /* check to see if this object property is commandable. + Does the property list contain a priority-array? */ + if (member_of_object) { + has_priority_array = member_of_object( + wp_data->object_type, wp_data->object_instance, + PROP_PRIORITY_ARRAY); + } + if (has_priority_array || (wp_data->object_type == OBJECT_CHANNEL)) { + if (wp_data->object_property != PROP_PRESENT_VALUE) { + /* this property is not commandable, + so it "shall not be changed, and + the write shall be considered successful." */ + bypass = true; + } + } else { + /* this object is not commandable, so any property + written with a NULL "shall not be changed, and + the write shall be considered successful." */ + bypass = true; + } + } + + return bypass; +} diff --git a/src/bacnet/wp.h b/src/bacnet/wp.h index 71df69d5..91628af9 100644 --- a/src/bacnet/wp.h +++ b/src/bacnet/wp.h @@ -55,6 +55,18 @@ typedef bool (*write_property_function)(BACNET_WRITE_PROPERTY_DATA *wp_data); typedef bool (*bacnet_property_unsigned_setter)( uint32_t object_instance, BACNET_UNSIGNED_INTEGER value); +/** + * @brief API to see if an object property is a member of this object instance + * @param object_type - object type of the object + * @param object_instance - object-instance number of the object + * @param object_property - object-property to be checked + * @return true if the property is a member of this object instance + */ +typedef bool (*write_property_member_of_object)( + BACNET_OBJECT_TYPE object_type, + uint32_t object_instance, + BACNET_PROPERTY_ID object_property); + #ifdef __cplusplus extern "C" { #endif /* __cplusplus */ @@ -104,6 +116,11 @@ bool write_property_unsigned_decode( bacnet_property_unsigned_setter setter, BACNET_UNSIGNED_INTEGER maximum); +BACNET_STACK_EXPORT +bool write_property_relinquish_bypass( + BACNET_WRITE_PROPERTY_DATA *wp_data, + write_property_member_of_object member_of_object); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/test/bacnet/wp/src/main.c b/test/bacnet/wp/src/main.c index 45dcb6f1..82bfb184 100644 --- a/test/bacnet/wp/src/main.c +++ b/test/bacnet/wp/src/main.c @@ -285,17 +285,57 @@ static void testWriteProperty(void) testWritePropertyTag(&value); } -/** - * @} - */ +static bool Is_Property_Member; +bool test_write_property_member_of_object( + BACNET_OBJECT_TYPE object_type, + uint32_t object_instance, + BACNET_PROPERTY_ID object_property) +{ + (void)object_type; + (void)object_instance; + (void)object_property; + return Is_Property_Member; +} + +#if defined(CONFIG_ZTEST_NEW_API) +ZTEST(wp_tests, testWritePropertyNull) +#else +static void testWritePropertyNull(void) +#endif +{ + bool bypass = false; + BACNET_WRITE_PROPERTY_DATA wp_data = { 0 }; + + bypass = write_property_relinquish_bypass(NULL, NULL); + zassert_equal(bypass, false, NULL); + wp_data.object_type = OBJECT_ANALOG_OUTPUT; + wp_data.object_instance = 0; + wp_data.object_property = PROP_PRESENT_VALUE; + wp_data.application_data_len = + encode_application_null(&wp_data.application_data[0]); + Is_Property_Member = true; + bypass = write_property_relinquish_bypass( + &wp_data, test_write_property_member_of_object); + zassert_equal(bypass, false, NULL); + Is_Property_Member = false; + bypass = write_property_relinquish_bypass( + &wp_data, test_write_property_member_of_object); + zassert_equal(bypass, true, NULL); + wp_data.object_property = PROP_OUT_OF_SERVICE; + Is_Property_Member = true; + bypass = write_property_relinquish_bypass( + &wp_data, test_write_property_member_of_object); + zassert_equal(bypass, true, NULL); +} #if defined(CONFIG_ZTEST_NEW_API) ZTEST_SUITE(wp_tests, NULL, NULL, NULL, NULL, NULL); #else void test_main(void) { - ztest_test_suite(wp_tests, ztest_unit_test(testWriteProperty)); - + ztest_test_suite( + wp_tests, ztest_unit_test(testWriteProperty), + ztest_unit_test(testWritePropertyNull)); ztest_run_test_suite(wp_tests); } #endif