From ae135cd368c77ef779c1d02635a490fb4fc31dd0 Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Wed, 12 Feb 2025 12:14:01 -0600 Subject: [PATCH] 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 | 5 - ports/atmega328/bv.c | 6 - src/bacnet/bacenum.h | 2 + src/bacnet/basic/object/bv.c | 110 ------------------ src/bacnet/basic/object/bv.h | 5 - src/bacnet/basic/object/mso.c | 33 +++--- test/bacnet/basic/object/test/property_test.c | 27 +++++ 7 files changed, 48 insertions(+), 140 deletions(-) diff --git a/ports/at91sam7s/bv.c b/ports/at91sam7s/bv.c index 46ec5717..e11cedcd 100644 --- a/ports/at91sam7s/bv.c +++ b/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; diff --git a/ports/atmega328/bv.c b/ports/atmega328/bv.c index 18d4d25a..461abca5 100644 --- a/ports/atmega328/bv.c +++ b/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; diff --git a/src/bacnet/bacenum.h b/src/bacnet/bacenum.h index 0332cbcc..4a194d3a 100644 --- a/src/bacnet/bacenum.h +++ b/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 diff --git a/src/bacnet/basic/object/bv.c b/src/bacnet/basic/object/bv.c index 792528f8..4a75ae48 100644 --- a/src/bacnet/basic/object/bv.c +++ b/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; } diff --git a/src/bacnet/basic/object/bv.h b/src/bacnet/basic/object/bv.h index 811e6f4e..3940e1c9 100644 --- a/src/bacnet/basic/object/bv.h +++ b/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 diff --git a/src/bacnet/basic/object/mso.c b/src/bacnet/basic/object/mso.c index e06b0ee6..53c135b9 100644 --- a/src/bacnet/basic/object/mso.c +++ b/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, - PROP_OBJECT_NAME, - PROP_OBJECT_TYPE, - PROP_PRESENT_VALUE, - PROP_STATUS_FLAGS, - PROP_EVENT_STATE, - PROP_OUT_OF_SERVICE, - PROP_NUMBER_OF_STATES, - PROP_PRIORITY_ARRAY, - PROP_RELINQUISH_DEFAULT, +static const int Properties_Required[] = { + /* list of required properties in the object */ + PROP_OBJECT_IDENTIFIER, + PROP_OBJECT_NAME, + PROP_OBJECT_TYPE, + PROP_PRESENT_VALUE, + PROP_STATUS_FLAGS, + PROP_EVENT_STATE, + PROP_OUT_OF_SERVICE, + 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, - -1 }; +static const int Properties_Optional[] = { + /* list of required properties in the object */ + PROP_STATE_TEXT, PROP_DESCRIPTION, PROP_RELIABILITY, -1 +}; static const int Properties_Proprietary[] = { -1 }; diff --git a/test/bacnet/basic/object/test/property_test.c b/test/bacnet/basic/object/test/property_test.c index 2947d4c2..66ed54dd 100644 --- a/test/bacnet/basic/object/test/property_test.c +++ b/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;