Fixed WriteProperty bypass which is only for present-value property of commandable objects. (#984)

doc/bacnet-implementation-guidelines.md

@@ -760,14 +760,23 @@ does not support the Priority\_Array property.
 
 ## 8.24: Ignore Priority and NULL for Non-Commandable object writes
 
-If a property is writable but not commandable, the device shall process
+Clause 19.2.1.1 Commandable Properties specifies the list of
-a WriteProperty, WriteGroup or WritePropertyMultiple request and ignore
+commandable object properties (only present-value for some objects),
-the priority parameter if it is included in the request.
+and this table defines the list of commandable objects.
+If the priority-array property for the object is optional
+and not implemented for an object instance, then the object
+is not commandable. The Channel object is commandable and does
+not include a priority-array.
 
-If an attempt is made to relinquish a property that is not commandable
+If any property is writable but the object is not commandable
-and for which Null is not a supported datatype and no other error conditions
+(does not contain a priority-array or is a Channel object),
-exist, the property shall not be changed, and the write shall be considered
+the device shall process a WriteProperty, WriteGroup or
-successful [PR 21+].
+WritePropertyMultiple request and ignore the priority parameter
+if it is included in the request.
+
+If an attempt is made to relinquish a present-value property but
+the object is not commandable, the property shall not be changed,
+and the write shall be considered successful [PR 21+].
 
 ## 8.25: Server devices are required to support all COV lifetime values up to 8 hours (28800 seconds)
 

src/bacnet/basic/service/h_wp.c

@@ -40,6 +40,12 @@
  *      the property shall not be changed, and the write shall
  *      be considered successful.
  *
+ * @note There was an interpretation request in April 2025 that clarifies
+ *  that the NULL bypass is only for present-value property of objects that
+ *  optionally support a priority array but don't implement it.
+ *  See 135-2024-19.2.1.1 Commandable Properties for the list of commandable
+ *  properties of specific objects.
+ *
  * @param wp_data [in] The WriteProperty data structure
  * @return true if the write shall be considered successful
  */

src/bacnet/proplist.c

@@ -534,3 +534,53 @@ bool property_list_bacnet_list_member(
 
     return property_list_member(Properties_BACnetLIST, object_property);
 }
+
+/**
+ * @brief Determine if the object property is a commandable member
+ *
+ * 19.2.1.1 Commandable Properties
+ * The prioritization scheme is applied to certain properties of objects.
+ * The standard commandable properties and objects are as follows.
+ *
+ * @param object_type - object-type to be checked
+ * @param object_property - object-property to be checked
+ * @return true if the property is a commandable member
+ */
+bool property_list_commandable_member(
+    BACNET_OBJECT_TYPE object_type, BACNET_PROPERTY_ID object_property)
+{
+    bool status = false;
+
+    switch (object_type) {
+        case OBJECT_ACCESS_DOOR:
+        case OBJECT_ANALOG_OUTPUT:
+        case OBJECT_ANALOG_VALUE:
+        case OBJECT_BINARY_LIGHTING_OUTPUT:
+        case OBJECT_BINARY_OUTPUT:
+        case OBJECT_BINARY_VALUE:
+        case OBJECT_BITSTRING_VALUE:
+        case OBJECT_CHANNEL:
+        case OBJECT_CHARACTERSTRING_VALUE:
+        case OBJECT_DATE_VALUE:
+        case OBJECT_DATE_PATTERN_VALUE:
+        case OBJECT_DATETIME_VALUE:
+        case OBJECT_DATETIME_PATTERN_VALUE:
+        case OBJECT_INTEGER_VALUE:
+        case OBJECT_LARGE_ANALOG_VALUE:
+        case OBJECT_LIGHTING_OUTPUT:
+        case OBJECT_MULTI_STATE_OUTPUT:
+        case OBJECT_MULTI_STATE_VALUE:
+        case OBJECT_OCTETSTRING_VALUE:
+        case OBJECT_POSITIVE_INTEGER_VALUE:
+        case OBJECT_TIME_VALUE:
+        case OBJECT_TIME_PATTERN_VALUE:
+            if (object_property == PROP_PRESENT_VALUE) {
+                status = true;
+            }
+            break;
+        default:
+            break;
+    }
+
+    return status;
+}

src/bacnet/proplist.h

@@ -62,6 +62,9 @@ const int *property_list_bacnet_list(void);
 BACNET_STACK_EXPORT
 bool property_list_bacnet_list_member(
     BACNET_OBJECT_TYPE object_type, BACNET_PROPERTY_ID object_property);
+BACNET_STACK_EXPORT
+bool property_list_commandable_member(
+    BACNET_OBJECT_TYPE object_type, BACNET_PROPERTY_ID object_property);
 
 #ifdef __cplusplus
 }

src/bacnet/wp.c

@@ -500,6 +500,12 @@ bool write_property_unsigned_decode(
  *      the property shall not be changed, and the write shall
  *      be considered successful.
  *
+ * @note There was an interpretation request in April 2025 that clarifies
+ *  that the NULL bypass is only for present-value property of objects that
+ *  optionally support a priority array but don't implement it.
+ *  See 135-2024-19.2.1.1 Commandable Properties for the list of commandable
+ *  properties of specific objects.
+ *
  * @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
@@ -520,25 +526,25 @@ bool write_property_relinquish_bypass(
         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.
+        if (property_list_commandable_member(
-           Does the property list contain a priority-array? */
+                wp_data->object_type, wp_data->object_property)) {
-        if (member_of_object) {
+            if (member_of_object) {
-            has_priority_array = member_of_object(
+                /* check to see if this object property is commandable.
-                wp_data->object_type, wp_data->object_instance,
+                Does the property list contain a priority-array? */
-                PROP_PRIORITY_ARRAY);
+                has_priority_array = member_of_object(
-        }
+                    wp_data->object_type, wp_data->object_instance,
-        if (has_priority_array || (wp_data->object_type == OBJECT_CHANNEL)) {
+                    PROP_PRIORITY_ARRAY);
-            if (wp_data->object_property != PROP_PRESENT_VALUE) {
+            }
-                /* this property is not commandable,
+            if (has_priority_array ||
+                (wp_data->object_type == OBJECT_CHANNEL)) {
+                /* this property is commandable and shall not be bypassed */
+            } else {
+                /* this property that is optionally commandable
+                   is not commandable for this object instance,
                    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;
         }
     }
 

test/bacnet/property/src/main.c

@@ -69,6 +69,26 @@ static void testPropList(void)
             property_list_member(
                 property_list.Required.pList, PROP_OBJECT_NAME),
             NULL);
+        /* validate commandable object property list */
+        if ((i == OBJECT_CHANNEL) ||
+            (property_list_member(
+                 property_list.Required.pList, PROP_PRESENT_VALUE) &&
+             (property_list_member(
+                  property_list.Required.pList, PROP_PRIORITY_ARRAY) ||
+              property_list_member(
+                  property_list.Optional.pList, PROP_PRIORITY_ARRAY)))) {
+            zassert_true(
+                property_list_commandable_member(
+                    (BACNET_OBJECT_TYPE)i, PROP_PRESENT_VALUE),
+                "Object %s present-value is not listed as commandable",
+                bactext_object_type_name((BACNET_OBJECT_TYPE)i));
+        } else {
+            zassert_false(
+                property_list_commandable_member(
+                    (BACNET_OBJECT_TYPE)i, PROP_PRESENT_VALUE),
+                "Object %s present-value is listed as commandable",
+                bactext_object_type_name((BACNET_OBJECT_TYPE)i));
+        }
     }
     /* property is a BACnetARRAY */
     for (i = 0; i < OBJECT_PROPRIETARY_MIN; i++) {

test/bacnet/wp/src/main.c

@@ -308,7 +308,7 @@ static void testWritePropertyNull(void)
 
     bypass = write_property_relinquish_bypass(NULL, NULL);
     zassert_equal(bypass, false, NULL);
-    wp_data.object_type = OBJECT_ANALOG_OUTPUT;
+    wp_data.object_type = OBJECT_ANALOG_VALUE;
     wp_data.object_instance = 0;
     wp_data.object_property = PROP_PRESENT_VALUE;
     wp_data.application_data_len =
@@ -321,11 +321,16 @@ static void testWritePropertyNull(void)
     bypass = write_property_relinquish_bypass(
         &wp_data, test_write_property_member_of_object);
     zassert_equal(bypass, true, NULL);
+    wp_data.object_type = OBJECT_CHANNEL;
+    Is_Property_Member = false;
+    bypass = write_property_relinquish_bypass(
+        &wp_data, test_write_property_member_of_object);
+    zassert_equal(bypass, false, 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);
+    zassert_equal(bypass, false, NULL);
 }
 
 #if defined(CONFIG_ZTEST_NEW_API)