Added bypass in basic WriteProperty handler to accept writes of NULL to non-commandable properties (#919)

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)
 {

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)
 {

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)
 {

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.
  *

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

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,10 +132,17 @@ 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) {
+                /* 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(
                     &Handler_Transmit_Buffer[pdu_len], service_data->invoke_id,

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;
+}

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 */

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