Bugfix/property list consistency testing (#910)

* Added unit test while reading all object properties to flag properties not in the property-list.

* Added enumeration for last-property used in unit testing.

* Added missing reliability property in the basic multistate output object example.

* Removed polarity property in binary value object as it is not standard.

ports/at91sam7s/bv.c

@@ -123,7 +123,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     BACNET_BIT_STRING bit_string;
     BACNET_CHARACTER_STRING char_string;
     BACNET_BINARY_PV present_value = BINARY_INACTIVE;
-    BACNET_POLARITY polarity = POLARITY_NORMAL;
     uint8_t *apdu = NULL;
 
     if ((rpdata == NULL) || (rpdata->application_data == NULL) ||
@@ -169,10 +168,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
         case PROP_OUT_OF_SERVICE:
             apdu_len = encode_application_boolean(&apdu[0], false);
             break;
-        case PROP_POLARITY:
-            /* FIXME: figure out the polarity */
-            apdu_len = encode_application_enumerated(&apdu[0], polarity);
-            break;
         default:
             rpdata->error_class = ERROR_CLASS_PROPERTY;
             rpdata->error_code = ERROR_CODE_UNKNOWN_PROPERTY;

ports/atmega328/bv.c

@@ -219,7 +219,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
     BACNET_BIT_STRING bit_string;
     BACNET_CHARACTER_STRING char_string;
     BACNET_BINARY_PV present_value = BINARY_INACTIVE;
-    BACNET_POLARITY polarity = POLARITY_NORMAL;
     uint8_t *apdu;
 
     apdu = rpdata->application_data;
@@ -261,10 +260,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
         case PROP_OUT_OF_SERVICE:
             apdu_len = encode_application_boolean(&apdu[0], false);
             break;
-        case PROP_POLARITY:
-            /* FIXME: figure out the polarity */
-            apdu_len = encode_application_enumerated(&apdu[0], polarity);
-            break;
         default:
             rpdata->error_class = ERROR_CLASS_PROPERTY;
             rpdata->error_code = ERROR_CODE_UNKNOWN_PROPERTY;
@@ -337,7 +332,6 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
         case PROP_OBJECT_TYPE:
         case PROP_STATUS_FLAGS:
         case PROP_EVENT_STATE:
-        case PROP_POLARITY:
             wp_data->error_class = ERROR_CLASS_PROPERTY;
             wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED;
             break;

src/bacnet/bacenum.h

@@ -568,6 +568,8 @@ typedef enum {
     PROP_HIGH_END_TRIM = 4194335,
     PROP_LOW_END_TRIM = 4194336,
     PROP_TRIM_FADE_TIME = 4194337,
+    /* update this value which is used in testing */
+    PROP_RESERVED_RANGE_LAST = 4194337,
     /* Enumerated values 4194303-16777215 are reserved
        for definition by ASHRAE.  */
     /* do the max range inside of enum so that

src/bacnet/basic/object/bv.c

@@ -37,7 +37,6 @@ struct object_data {
     bool Change_Of_Value : 1;
     bool Present_Value : 1;
     bool Write_Enabled : 1;
-    bool Polarity : 1;
     unsigned Event_State : 3;
     uint8_t Reliability;
     const char *Object_Name;
@@ -221,38 +220,6 @@ static bool Binary_Present_Value_Boolean(BACNET_BINARY_PV binary_value)
     return boolean_value;
 }
 
-/**
- * @brief Convert from boolean to BACNET_POLARITY enumeration
- * @param  value - boolean value
- * @return  BACNET_POLARITY enumeration
- */
-static BACNET_POLARITY Binary_Polarity(bool value)
-{
-    BACNET_POLARITY polarity = POLARITY_NORMAL;
-
-    if (value) {
-        polarity = POLARITY_REVERSE;
-    }
-
-    return polarity;
-}
-
-/**
- * @brief Convert from BACNET_POLARITY enumeration to boolean
- * @param binary_value BACNET_POLARITY enumeration
- * @return boolean value
- */
-static bool Binary_Polarity_Boolean(BACNET_POLARITY polarity)
-{
-    bool boolean_value = false;
-
-    if (polarity == POLARITY_REVERSE) {
-        boolean_value = true;
-    }
-
-    return boolean_value;
-}
-
 /**
  * For a given object instance-number, return the present value.
  *
@@ -268,13 +235,6 @@ BACNET_BINARY_PV Binary_Value_Present_Value(uint32_t object_instance)
     pObject = Binary_Value_Object(object_instance);
     if (pObject) {
         value = Binary_Present_Value(pObject->Present_Value);
-        if (Binary_Polarity(pObject->Polarity) != POLARITY_NORMAL) {
-            if (value == BINARY_INACTIVE) {
-                value = BINARY_ACTIVE;
-            } else {
-                value = BINARY_INACTIVE;
-            }
-        }
     }
 
     return value;
@@ -502,14 +462,6 @@ bool Binary_Value_Present_Value_Set(
     pObject = Binary_Value_Object(object_instance);
     if (pObject) {
         if (value <= MAX_BINARY_PV) {
-            /* de-polarize */
-            if (Binary_Polarity(pObject->Polarity) != POLARITY_NORMAL) {
-                if (value == BINARY_INACTIVE) {
-                    value = BINARY_ACTIVE;
-                } else {
-                    value = BINARY_INACTIVE;
-                }
-            }
             Binary_Value_Present_Value_COV_Detect(pObject, value);
             pObject->Present_Value = Binary_Present_Value_Boolean(value);
             status = true;
@@ -640,44 +592,6 @@ const char *Binary_Value_Name_ASCII(uint32_t object_instance)
     return name;
 }
 
-/**
- * @brief For a given object instance-number, returns the polarity property.
- * @param  object_instance - object-instance number of the object
- * @return  the polarity property of the object.
- */
-BACNET_POLARITY Binary_Value_Polarity(uint32_t object_instance)
-{
-    BACNET_POLARITY polarity = POLARITY_NORMAL;
-    struct object_data *pObject;
-
-    pObject = Binary_Value_Object(object_instance);
-    if (pObject) {
-        polarity = Binary_Polarity(pObject->Polarity);
-    }
-
-    return polarity;
-}
-
-/**
- * @brief For a given object instance-number, sets the polarity property
- * @param  object_instance - object-instance number of the object
- * @param  polarity - polarity property value
- * @return  true if polarity was set
- */
-bool Binary_Value_Polarity_Set(
-    uint32_t object_instance, BACNET_POLARITY polarity)
-{
-    bool status = false;
-    struct object_data *pObject;
-
-    pObject = Binary_Value_Object(object_instance);
-    if (pObject) {
-        pObject->Polarity = Binary_Polarity_Boolean(polarity);
-    }
-
-    return status;
-}
-
 /**
  * @brief For a given object instance-number, returns the description
  * @param  object_instance - object-instance number of the object
@@ -931,10 +845,6 @@ int Binary_Value_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata)
             state = Binary_Value_Out_Of_Service(rpdata->object_instance);
             apdu_len = encode_application_boolean(&apdu[0], state);
             break;
-        case PROP_POLARITY:
-            apdu_len = encode_application_enumerated(
-                &apdu[0], Binary_Value_Polarity(rpdata->object_instance));
-            break;
         case PROP_RELIABILITY:
             apdu_len = encode_application_enumerated(
                 &apdu[0], Binary_Value_Reliability(rpdata->object_instance));
@@ -1098,21 +1008,6 @@ bool Binary_Value_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data)
                     wp_data->object_instance, value.type.Boolean);
             }
             break;
-        case PROP_POLARITY:
-            status = write_property_type_valid(
-                wp_data, &value, BACNET_APPLICATION_TAG_ENUMERATED);
-            if (status) {
-                if (value.type.Enumerated < MAX_POLARITY) {
-                    Binary_Value_Polarity_Set(
-                        wp_data->object_instance,
-                        (BACNET_POLARITY)value.type.Enumerated);
-                } else {
-                    status = false;
-                    wp_data->error_class = ERROR_CLASS_PROPERTY;
-                    wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE;
-                }
-            }
-            break;
 #if defined(INTRINSIC_REPORTING) && (BINARY_VALUE_INTRINSIC_REPORTING)
         case PROP_TIME_DELAY:
             status = write_property_type_valid(
@@ -1291,7 +1186,6 @@ uint32_t Binary_Value_Create(uint32_t object_instance)
             pObject->Inactive_Text = Default_Inactive_Text;
             pObject->Change_Of_Value = false;
             pObject->Write_Enabled = false;
-            pObject->Polarity = false;
 #if defined(INTRINSIC_REPORTING) && (BINARY_VALUE_INTRINSIC_REPORTING)
             pObject->Event_State = EVENT_STATE_NORMAL;
             pObject->Event_Detection_Enable = true;
@@ -1886,10 +1780,6 @@ bool Binary_Value_Alarm_Value_Set(
     struct object_data *pObject = Binary_Value_Object(object_instance);
 
     if (pObject) {
-        if (pObject->Polarity != POLARITY_NORMAL) {
-            value =
-                (value == BINARY_INACTIVE) ? BINARY_ACTIVE : BINARY_INACTIVE;
-        }
         pObject->Alarm_Value = value;
         status = true;
     }

src/bacnet/basic/object/bv.h

@@ -120,11 +120,6 @@ const char *Binary_Value_Active_Text(uint32_t instance);
 BACNET_STACK_EXPORT
 bool Binary_Value_Active_Text_Set(uint32_t instance, const char *new_name);
 
-BACNET_STACK_EXPORT
-BACNET_POLARITY Binary_Value_Polarity(uint32_t instance);
-BACNET_STACK_EXPORT
-bool Binary_Value_Polarity_Set(
-    uint32_t object_instance, BACNET_POLARITY polarity);
 BACNET_STACK_EXPORT
 uint32_t Binary_Value_Create(uint32_t object_instance);
 BACNET_STACK_EXPORT

src/bacnet/basic/object/mso.c

@@ -55,23 +55,28 @@ static const char *Default_State_Text = "State 1\0"
                                         "State 2\0"
                                         "State 3\0";
 /* These three arrays are used by the ReadPropertyMultiple handler */
-static const int Properties_Required[] = { PROP_OBJECT_IDENTIFIER,
+static const int Properties_Required[] = {
-                                           PROP_OBJECT_NAME,
+    /* list of required properties in the object */
-                                           PROP_OBJECT_TYPE,
+    PROP_OBJECT_IDENTIFIER,
-                                           PROP_PRESENT_VALUE,
+    PROP_OBJECT_NAME,
-                                           PROP_STATUS_FLAGS,
+    PROP_OBJECT_TYPE,
-                                           PROP_EVENT_STATE,
+    PROP_PRESENT_VALUE,
-                                           PROP_OUT_OF_SERVICE,
+    PROP_STATUS_FLAGS,
-                                           PROP_NUMBER_OF_STATES,
+    PROP_EVENT_STATE,
-                                           PROP_PRIORITY_ARRAY,
+    PROP_OUT_OF_SERVICE,
-                                           PROP_RELINQUISH_DEFAULT,
+    PROP_NUMBER_OF_STATES,
+    PROP_PRIORITY_ARRAY,
+    PROP_RELINQUISH_DEFAULT,
 #if (BACNET_PROTOCOL_REVISION >= 17)
-                                           PROP_CURRENT_COMMAND_PRIORITY,
+    PROP_CURRENT_COMMAND_PRIORITY,
 #endif
-                                           -1 };
+    -1
+};
 
-static const int Properties_Optional[] = { PROP_STATE_TEXT, PROP_DESCRIPTION,
+static const int Properties_Optional[] = {
-                                           -1 };
+    /* list of required properties in the object */
+    PROP_STATE_TEXT, PROP_DESCRIPTION, PROP_RELIABILITY, -1
+};
 
 static const int Properties_Proprietary[] = { -1 };
 

test/bacnet/basic/object/test/property_test.c

@@ -213,6 +213,7 @@ void bacnet_object_properties_read_write_test(
     const int *pRequired = NULL;
     const int *pOptional = NULL;
     const int *pProprietary = NULL;
+    BACNET_PROPERTY_ID property;
     int len = 0;
     bool status = false;
 
@@ -222,6 +223,32 @@ void bacnet_object_properties_read_write_test(
     rpdata.object_type = object_type;
     rpdata.object_instance = object_instance;
     property_list(&pRequired, &pOptional, &pProprietary);
+    /* detect properties that are not in the property lists */
+    for (property = 0; property < MAX_BACNET_PROPERTY_ID; property++) {
+        if (property_lists_member(
+                pRequired, pOptional, pProprietary, property)) {
+            continue;
+        }
+        if ((property == PROP_ALL) || (property == PROP_REQUIRED) ||
+            (property == PROP_OPTIONAL)) {
+            continue;
+        }
+        rpdata.object_property = property;
+        rpdata.array_index = BACNET_ARRAY_ALL;
+        len = read_property(&rpdata);
+        zassert_equal(
+            len, BACNET_STATUS_ERROR,
+            "property '%s' array_index=ALL: Missing in property list.\n",
+            bactext_property_name(rpdata.object_property));
+        /* shrink the number space and skip proprietary range values */
+        if (property == PROP_RESERVED_RANGE_MAX) {
+            property = PROP_RESERVED_RANGE_MIN2 - 1;
+        }
+        /* shrink the number space to known values */
+        if (property == PROP_RESERVED_RANGE_LAST) {
+            break;
+        }
+    }
     while ((*pRequired) != -1) {
         rpdata.object_property = *pRequired;
         rpdata.array_index = BACNET_ARRAY_ALL;