From c9a85a128212cc71cdbc58e410e4f381896e460b Mon Sep 17 00:00:00 2001 From: Steve Karg Date: Thu, 28 Sep 2023 15:30:28 -0500 Subject: [PATCH] Refactor/write property object name (#504) * refactor WriteProperty of object-name property rules into device object. * remove dependence on device object inside some dynamic objects * improve device object unit test coverage --------- Co-authored-by: Steve Karg --- apps/blinkt/device.c | 81 +++++++++++++++++++-- apps/blinkt/main.c | 1 - apps/piface/device.c | 80 ++++++++++++++++++-- src/bacnet/bacstr.h | 4 - src/bacnet/basic/object/ao.c | 24 +----- src/bacnet/basic/object/bacfile.c | 23 +----- src/bacnet/basic/object/bo.c | 23 +----- src/bacnet/basic/object/channel.c | 3 - src/bacnet/basic/object/color_object.c | 27 +------ src/bacnet/basic/object/color_temperature.c | 24 +----- src/bacnet/basic/object/device.c | 78 +++++++++++++++++++- src/bacnet/basic/object/lo.c | 24 +----- src/bacnet/basic/object/mso.c | 23 +----- src/bacnet/wp.c | 20 +++-- src/bacnet/wp.h | 4 +- test/bacnet/basic/object/device/src/main.c | 65 ++++++++++++----- 16 files changed, 304 insertions(+), 200 deletions(-) diff --git a/apps/blinkt/device.c b/apps/blinkt/device.c index 262f914a..1bdaba20 100644 --- a/apps/blinkt/device.c +++ b/apps/blinkt/device.c @@ -62,9 +62,6 @@ /* local forward (semi-private) and external prototypes */ int Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata); bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data); -extern int Routed_Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata); -extern bool Routed_Device_Write_Property_Local( - BACNET_WRITE_PROPERTY_DATA *wp_data); /* may be overridden by outside table */ static object_functions_t *Object_Table; @@ -1541,6 +1538,73 @@ bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data) return status; } +/** + * @brief Handles the writing of the object name property + * @param wp_data [in,out] WriteProperty data structure + * @param Object_Write_Property object specific function to write the property + * @return True on success, else False if there is an error. + */ +static bool Device_Write_Property_Object_Name( + BACNET_WRITE_PROPERTY_DATA *wp_data, + write_property_function Object_Write_Property) +{ + bool status = false; /* return value */ + int len = 0; + BACNET_CHARACTER_STRING value; + BACNET_OBJECT_TYPE object_type = OBJECT_NONE; + uint32_t object_instance = 0; + int apdu_size = 0; + uint8_t *apdu = NULL; + + if (!wp_data) { + return false; + } + if (wp_data->array_index != BACNET_ARRAY_ALL) { + /* only array properties can have array options */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY; + return false; + } + apdu = wp_data->application_data; + apdu_size = wp_data->application_data_len; + len = bacnet_character_string_application_decode(apdu, apdu_size, &value); + if (len > 0) { + if ((characterstring_encoding(&value) != CHARACTER_ANSI_X34) || + (characterstring_length(&value) == 0) || + (!characterstring_printable(&value))) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } else { + status = true; + } + } else if (len == 0) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE; + } else { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } + if (status) { + /* All the object names in a device must be unique */ + if (Device_Valid_Object_Name(&value, &object_type, &object_instance)) { + if ((object_type == wp_data->object_type) && + (object_instance == wp_data->object_instance)) { + /* writing same name to same object */ + status = true; + } else { + /* name already exists in some object */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_DUPLICATE_NAME; + status = false; + } + } else { + status = Object_Write_Property(wp_data); + } + } + + return status; +} + /** Looks up the requested Object and Property, and set the new Value in it, * if allowed. * If the Object or Property can't be found, sets the error class and code. @@ -1567,9 +1631,13 @@ bool Device_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) if (wp_data->object_property == PROP_PROPERTY_LIST) { wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; - } else + return false; + } #endif - { + if (wp_data->object_property == PROP_OBJECT_NAME) { + status = Device_Write_Property_Object_Name(wp_data, + pObject->Object_Write_Property); + } else { status = pObject->Object_Write_Property(wp_data); } } else { @@ -1881,6 +1949,9 @@ void Device_Init(object_functions_t *object_table) } pObject++; } +#if (BACNET_PROTOCOL_REVISION >= 14) + Channel_Write_Property_Internal_Callback_Set(Device_Write_Property); +#endif } bool DeviceGetRRInfo(BACNET_READ_RANGE_DATA *pRequest, /* Info on the request */ diff --git a/apps/blinkt/main.c b/apps/blinkt/main.c index dcbd3ae4..1fe6ab5f 100644 --- a/apps/blinkt/main.c +++ b/apps/blinkt/main.c @@ -304,7 +304,6 @@ static void bacnet_output_init(void) Color_Temperature_Write_Value_Handler); Lighting_Output_Write_Present_Value_Callback_Set( Lighting_Output_Write_Value_Handler); - Channel_Write_Property_Internal_Callback_Set(Device_Write_Property); } /** diff --git a/apps/piface/device.c b/apps/piface/device.c index 4ab7c0a4..29a9b7d6 100644 --- a/apps/piface/device.c +++ b/apps/piface/device.c @@ -55,9 +55,6 @@ /* local forward (semi-private) and external prototypes */ int Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata); bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data); -extern int Routed_Device_Read_Property_Local(BACNET_READ_PROPERTY_DATA *rpdata); -extern bool Routed_Device_Write_Property_Local( - BACNET_WRITE_PROPERTY_DATA *wp_data); /* may be overridden by outside table */ static object_functions_t *Object_Table; @@ -1508,6 +1505,73 @@ bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data) return status; } +/** + * @brief Handles the writing of the object name property + * @param wp_data [in,out] WriteProperty data structure + * @param Object_Write_Property object specific function to write the property + * @return True on success, else False if there is an error. + */ +static bool Device_Write_Property_Object_Name( + BACNET_WRITE_PROPERTY_DATA *wp_data, + write_property_function Object_Write_Property) +{ + bool status = false; /* return value */ + int len = 0; + BACNET_CHARACTER_STRING value; + BACNET_OBJECT_TYPE object_type = OBJECT_NONE; + uint32_t object_instance = 0; + int apdu_size = 0; + uint8_t *apdu = NULL; + + if (!wp_data) { + return false; + } + if (wp_data->array_index != BACNET_ARRAY_ALL) { + /* only array properties can have array options */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY; + return false; + } + apdu = wp_data->application_data; + apdu_size = wp_data->application_data_len; + len = bacnet_character_string_application_decode(apdu, apdu_size, &value); + if (len > 0) { + if ((characterstring_encoding(&value) != CHARACTER_ANSI_X34) || + (characterstring_length(&value) == 0) || + (!characterstring_printable(&value))) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } else { + status = true; + } + } else if (len == 0) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE; + } else { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } + if (status) { + /* All the object names in a device must be unique */ + if (Device_Valid_Object_Name(&value, &object_type, &object_instance)) { + if ((object_type == wp_data->object_type) && + (object_instance == wp_data->object_instance)) { + /* writing same name to same object */ + status = true; + } else { + /* name already exists in some object */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_DUPLICATE_NAME; + status = false; + } + } else { + status = Object_Write_Property(wp_data); + } + } + + return status; +} + /** Looks up the requested Object and Property, and set the new Value in it, * if allowed. * If the Object or Property can't be found, sets the error class and code. @@ -1534,10 +1598,14 @@ bool Device_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) if (wp_data->object_property == PROP_PROPERTY_LIST) { wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; - } else + return false; + } #endif - { - status = pObject->Object_Write_Property(wp_data); + if (wp_data->object_property == PROP_OBJECT_NAME) { + status = Device_Write_Property_Object_Name(wp_data, + pObject->Object_Write_Property); + } else { + status = pObject->Object_Write_Property(wp_data); } } else { wp_data->error_class = ERROR_CLASS_PROPERTY; diff --git a/src/bacnet/bacstr.h b/src/bacnet/bacstr.h index 464c2bdd..0eef4721 100644 --- a/src/bacnet/bacstr.h +++ b/src/bacnet/bacstr.h @@ -41,15 +41,11 @@ typedef struct BACnet_Bit_String { typedef struct BACnet_Character_String { size_t length; uint8_t encoding; - /* limit - 6 octets is the most our tag and type could be */ char value[MAX_CHARACTER_STRING_BYTES]; } BACNET_CHARACTER_STRING; -/* FIXME: convert the bacdcode library to use BACNET_OCTET_STRING - for APDU buffer to prevent buffer overflows */ typedef struct BACnet_Octet_String { size_t length; - /* limit - 6 octets is the most our tag and type could be */ uint8_t value[MAX_OCTET_STRING_BYTES]; } BACNET_OCTET_STRING; diff --git a/src/bacnet/basic/object/ao.c b/src/bacnet/basic/object/ao.c index 2db64b87..1de3e1b6 100644 --- a/src/bacnet/basic/object/ao.c +++ b/src/bacnet/basic/object/ao.c @@ -50,7 +50,6 @@ #include "bacnet/reject.h" #include "bacnet/rp.h" #include "bacnet/wp.h" -#include "bacnet/basic/object/device.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/keylist.h" /* me! */ @@ -521,7 +520,6 @@ bool Analog_Output_Object_Name( /** * For a given object instance-number, sets the object-name - * Note that the object name must be unique within this device. * * @param object_instance - object-instance number of the object * @param new_name - holds the object-name to be set @@ -531,30 +529,12 @@ bool Analog_Output_Object_Name( bool Analog_Output_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = 0; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == Object_Type) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/basic/object/bacfile.c b/src/bacnet/basic/object/bacfile.c index 6945dbfb..8f0a08b6 100644 --- a/src/bacnet/basic/object/bacfile.c +++ b/src/bacnet/basic/object/bacfile.c @@ -37,7 +37,6 @@ #include "bacnet/npdu.h" #include "bacnet/apdu.h" #include "bacnet/basic/tsm/tsm.h" -#include "bacnet/basic/object/device.h" #include "bacnet/arf.h" #include "bacnet/awf.h" #include "bacnet/rp.h" @@ -218,30 +217,12 @@ bool bacfile_object_name( bool bacfile_object_name_set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = 0; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == Object_Type) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/basic/object/bo.c b/src/bacnet/basic/object/bo.c index f5ae225b..58946afe 100644 --- a/src/bacnet/basic/object/bo.c +++ b/src/bacnet/basic/object/bo.c @@ -49,7 +49,6 @@ #include "bacnet/reject.h" #include "bacnet/rp.h" #include "bacnet/wp.h" -#include "bacnet/basic/object/device.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/keylist.h" /* me! */ @@ -531,30 +530,12 @@ bool Binary_Output_Object_Name( bool Binary_Output_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = OBJECT_NONE; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == Object_Type) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/basic/object/channel.c b/src/bacnet/basic/object/channel.c index 37c52760..6a3e6ca7 100644 --- a/src/bacnet/basic/object/channel.c +++ b/src/bacnet/basic/object/channel.c @@ -45,9 +45,6 @@ #include "bacnet/basic/services.h" #include "bacnet/proplist.h" #include "bacnet/basic/sys/keylist.h" -#if defined(CHANNEL_LIGHTING_COMMAND) -#include "bacnet/basic/object/lo.h" -#endif #if defined(CHANNEL_LIGHTING_COMMAND) || defined(CHANNEL_COLOR_COMMAND) #include "bacnet/lighting.h" #endif diff --git a/src/bacnet/basic/object/color_object.c b/src/bacnet/basic/object/color_object.c index e576c1fe..8b7ce2b7 100644 --- a/src/bacnet/basic/object/color_object.c +++ b/src/bacnet/basic/object/color_object.c @@ -690,7 +690,7 @@ bool Color_Object_Name( { bool status = false; struct object_data *pObject; - char name_text[16] = "COLOR-4194303"; + char name_text[24] = "COLOR-4194303"; pObject = Keylist_Data(Object_List, object_instance); if (pObject) { @@ -708,8 +708,7 @@ bool Color_Object_Name( /** * For a given object instance-number, sets the object-name - * Note that the object name must be unique within this device. - * + * * @param object_instance - object-instance number of the object * @param new_name - holds the object-name to be set * @@ -718,30 +717,12 @@ bool Color_Object_Name( bool Color_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = 0; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == OBJECT_COLOR) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + pObject->Object_Name = new_name; + status = true; } return status; diff --git a/src/bacnet/basic/object/color_temperature.c b/src/bacnet/basic/object/color_temperature.c index 8e7b7fa3..09914b0f 100644 --- a/src/bacnet/basic/object/color_temperature.c +++ b/src/bacnet/basic/object/color_temperature.c @@ -36,7 +36,6 @@ #include "bacnet/reject.h" #include "bacnet/rp.h" #include "bacnet/wp.h" -#include "bacnet/basic/object/device.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/keylist.h" #include "bacnet/basic/sys/linear.h" @@ -946,7 +945,6 @@ bool Color_Temperature_Object_Name( /** * For a given object instance-number, sets the object-name - * Note that the object name must be unique within this device. * * @param object_instance - object-instance number of the object * @param new_name - holds the object-name to be set @@ -956,30 +954,12 @@ bool Color_Temperature_Object_Name( bool Color_Temperature_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = 0; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == OBJECT_COLOR) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/basic/object/device.c b/src/bacnet/basic/object/device.c index 1e125069..39b361b2 100644 --- a/src/bacnet/basic/object/device.c +++ b/src/bacnet/basic/object/device.c @@ -1741,6 +1741,73 @@ bool Device_Write_Property_Local(BACNET_WRITE_PROPERTY_DATA *wp_data) return status; } +/** + * @brief Handles the writing of the object name property + * @param wp_data [in,out] WriteProperty data structure + * @param Object_Write_Property object specific function to write the property + * @return True on success, else False if there is an error. + */ +static bool Device_Write_Property_Object_Name( + BACNET_WRITE_PROPERTY_DATA *wp_data, + write_property_function Object_Write_Property) +{ + bool status = false; /* return value */ + int len = 0; + BACNET_CHARACTER_STRING value; + BACNET_OBJECT_TYPE object_type = OBJECT_NONE; + uint32_t object_instance = 0; + int apdu_size = 0; + uint8_t *apdu = NULL; + + if (!wp_data) { + return false; + } + if (wp_data->array_index != BACNET_ARRAY_ALL) { + /* only array properties can have array options */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_PROPERTY_IS_NOT_AN_ARRAY; + return false; + } + apdu = wp_data->application_data; + apdu_size = wp_data->application_data_len; + len = bacnet_character_string_application_decode(apdu, apdu_size, &value); + if (len > 0) { + if ((characterstring_encoding(&value) != CHARACTER_ANSI_X34) || + (characterstring_length(&value) == 0) || + (!characterstring_printable(&value))) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } else { + status = true; + } + } else if (len == 0) { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_INVALID_DATA_TYPE; + } else { + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; + } + if (status) { + /* All the object names in a device must be unique */ + if (Device_Valid_Object_Name(&value, &object_type, &object_instance)) { + if ((object_type == wp_data->object_type) && + (object_instance == wp_data->object_instance)) { + /* writing same name to same object */ + status = true; + } else { + /* name already exists in some object */ + wp_data->error_class = ERROR_CLASS_PROPERTY; + wp_data->error_code = ERROR_CODE_DUPLICATE_NAME; + status = false; + } + } else { + status = Object_Write_Property(wp_data); + } + } + + return status; +} + /** Looks up the requested Object and Property, and set the new Value in it, * if allowed. * If the Object or Property can't be found, sets the error class and code. @@ -1767,9 +1834,13 @@ bool Device_Write_Property(BACNET_WRITE_PROPERTY_DATA *wp_data) if (wp_data->object_property == PROP_PROPERTY_LIST) { wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_WRITE_ACCESS_DENIED; - } else + return status; + } #endif - { + if (wp_data->object_property == PROP_OBJECT_NAME) { + status = Device_Write_Property_Object_Name(wp_data, + pObject->Object_Write_Property); + } else { status = pObject->Object_Write_Property(wp_data); } } else { @@ -2117,6 +2188,9 @@ void Device_Init(object_functions_t *object_table) } pObject++; } +#if (BACNET_PROTOCOL_REVISION >= 14) + Channel_Write_Property_Internal_Callback_Set(Device_Write_Property); +#endif } bool DeviceGetRRInfo(BACNET_READ_RANGE_DATA *pRequest, /* Info on the request */ diff --git a/src/bacnet/basic/object/lo.c b/src/bacnet/basic/object/lo.c index 783338de..70f1309b 100644 --- a/src/bacnet/basic/object/lo.c +++ b/src/bacnet/basic/object/lo.c @@ -41,7 +41,6 @@ #include "bacnet/rp.h" #include "bacnet/wp.h" #include "bacnet/lighting.h" -#include "bacnet/basic/object/device.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/keylist.h" #include "bacnet/basic/sys/linear.h" @@ -719,7 +718,6 @@ bool Lighting_Output_Object_Name( /** * For a given object instance-number, sets the object-name - * Note that the object name must be unique within this device. * * @param object_instance - object-instance number of the object * @param new_name - holds the object-name to be set @@ -729,30 +727,12 @@ bool Lighting_Output_Object_Name( bool Lighting_Output_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = 0; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == OBJECT_LIGHTING_OUTPUT) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/basic/object/mso.c b/src/bacnet/basic/object/mso.c index 089e24d8..d60d3168 100644 --- a/src/bacnet/basic/object/mso.c +++ b/src/bacnet/basic/object/mso.c @@ -49,7 +49,6 @@ #include "bacnet/reject.h" #include "bacnet/rp.h" #include "bacnet/wp.h" -#include "bacnet/basic/object/device.h" #include "bacnet/basic/services.h" #include "bacnet/basic/sys/keylist.h" /* me! */ @@ -623,30 +622,12 @@ bool Multistate_Output_Object_Name( bool Multistate_Output_Name_Set(uint32_t object_instance, char *new_name) { bool status = false; /* return value */ - BACNET_CHARACTER_STRING object_name; - BACNET_OBJECT_TYPE found_type = OBJECT_NONE; - uint32_t found_instance = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject && new_name) { - /* All the object names in a device must be unique */ - characterstring_init_ansi(&object_name, new_name); - if (Device_Valid_Object_Name( - &object_name, &found_type, &found_instance)) { - if ((found_type == Object_Type) && - (found_instance == object_instance)) { - /* writing same name to same object */ - status = true; - } else { - /* duplicate name! */ - status = false; - } - } else { - status = true; - pObject->Object_Name = new_name; - Device_Inc_Database_Revision(); - } + status = true; + pObject->Object_Name = new_name; } return status; diff --git a/src/bacnet/wp.c b/src/bacnet/wp.c index 6cfc742c..51e81553 100644 --- a/src/bacnet/wp.c +++ b/src/bacnet/wp.c @@ -31,6 +31,7 @@ License. ------------------------------------------- ####COPYRIGHTEND####*/ +#include #include #include "bacnet/bacenum.h" #include "bacnet/bacdcode.h" @@ -268,6 +269,7 @@ int wp_decode_service_request( * requested data and space for the reply, or error response. * @param value - #BACNET_APPLICATION_DATA_VALUE data, for the tag * @param expected_tag - the tag that is expected for this property value + * @return true if the expected tag matches the value tag */ bool write_property_type_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, BACNET_APPLICATION_DATA_VALUE *value, @@ -292,11 +294,12 @@ bool write_property_type_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, * @param wp_data - #BACNET_WRITE_PROPERTY_DATA data, including * requested data and space for the reply, or error response. * @param value - #BACNET_APPLICATION_DATA_VALUE data, for the tag - * @param expected_tag - the tag that is expected for this property value + * @param len_max - max length accepted for a character string, or 0=unchecked + * @return true if the character string value is valid */ bool write_property_string_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, BACNET_APPLICATION_DATA_VALUE *value, - int len_max) + size_t len_max) { bool valid = false; @@ -315,8 +318,8 @@ bool write_property_string_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, wp_data->error_class = ERROR_CLASS_PROPERTY; wp_data->error_code = ERROR_CODE_VALUE_OUT_OF_RANGE; } - } else if (characterstring_length(&value->type.Character_String) > - (uint16_t)len_max) { + } else if ((len_max > 0) && (characterstring_length( + &value->type.Character_String) > len_max)) { if (wp_data) { wp_data->error_class = ERROR_CLASS_RESOURCES; wp_data->error_code = ERROR_CODE_NO_SPACE_TO_WRITE_PROPERTY; @@ -347,19 +350,20 @@ bool write_property_string_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, * @param wp_data - #BACNET_WRITE_PROPERTY_DATA data, including * requested data and space for the reply, or error response. * @param value - #BACNET_APPLICATION_DATA_VALUE data, for the tag - * @param expected_tag - the tag that is expected for this property value + * @param len_max - max length accepted for a character string, or 0=unchecked + * @return true if the character string value is valid */ bool write_property_empty_string_valid(BACNET_WRITE_PROPERTY_DATA *wp_data, BACNET_APPLICATION_DATA_VALUE *value, - int len_max) + size_t len_max) { bool valid = false; if (value && (value->tag == BACNET_APPLICATION_TAG_CHARACTER_STRING)) { if (characterstring_encoding(&value->type.Character_String) == CHARACTER_ANSI_X34) { - if (characterstring_length(&value->type.Character_String) > - (uint16_t)len_max) { + if ((len_max > 0) && (characterstring_length( + &value->type.Character_String) > len_max)) { if (wp_data) { wp_data->error_class = ERROR_CLASS_RESOURCES; wp_data->error_code = ERROR_CODE_NO_SPACE_TO_WRITE_PROPERTY; diff --git a/src/bacnet/wp.h b/src/bacnet/wp.h index 4a134637..38c19dc6 100644 --- a/src/bacnet/wp.h +++ b/src/bacnet/wp.h @@ -89,12 +89,12 @@ extern "C" { bool write_property_string_valid( BACNET_WRITE_PROPERTY_DATA * wp_data, BACNET_APPLICATION_DATA_VALUE * value, - int len_max); + size_t len_max); BACNET_STACK_EXPORT bool write_property_empty_string_valid( BACNET_WRITE_PROPERTY_DATA * wp_data, BACNET_APPLICATION_DATA_VALUE * value, - int len_max); + size_t len_max); #ifdef __cplusplus } diff --git a/test/bacnet/basic/object/device/src/main.c b/test/bacnet/basic/object/device/src/main.c index 5d741131..332f0215 100644 --- a/test/bacnet/basic/object/device/src/main.c +++ b/test/bacnet/basic/object/device/src/main.c @@ -20,18 +20,20 @@ /** * @brief Test ReadProperty API */ -static void test_Device_ReadProperty(void) +static void test_Device_Data_Sharing(void) { uint8_t apdu[MAX_APDU] = { 0 }; int len = 0; int test_len = 0; - BACNET_READ_PROPERTY_DATA rpdata; + BACNET_READ_PROPERTY_DATA rpdata = { 0 }; + BACNET_WRITE_PROPERTY_DATA wpdata = { 0 }; /* for decode value data */ - BACNET_APPLICATION_DATA_VALUE value; + BACNET_APPLICATION_DATA_VALUE value = { 0 }; const int *pRequired = NULL; const int *pOptional = NULL; const int *pProprietary = NULL; unsigned count = 0; + bool status = false; Device_Init(NULL); count = Device_Count(); @@ -45,22 +47,35 @@ static void test_Device_ReadProperty(void) rpdata.object_property = *pRequired; rpdata.array_index = BACNET_ARRAY_ALL; len = Device_Read_Property(&rpdata); - zassert_not_equal(len, BACNET_STATUS_ERROR, NULL); + zassert_not_equal(len, BACNET_STATUS_ERROR, + "property '%s': failed to ReadProperty!\n", + bactext_property_name(rpdata.object_property)); if (len > 0) { test_len = bacapp_decode_application_data(rpdata.application_data, (uint8_t)rpdata.application_data_len, &value); - if (len != test_len) { - printf("property '%s': failed to decode!\n", - bactext_property_name(rpdata.object_property)); - } - if (rpdata.object_property == PROP_PRIORITY_ARRAY) { + if ((rpdata.object_property == PROP_PRIORITY_ARRAY) || + (rpdata.object_property == PROP_OBJECT_LIST)) { /* FIXME: known fail to decode */ len = test_len; } - zassert_true(test_len >= 0, NULL); - } else { - printf("property '%s': failed to read!\n", + zassert_equal(test_len, len, "property '%s': failed to decode!\n", bactext_property_name(rpdata.object_property)); + /* check WriteProperty properties */ + wpdata.object_type = rpdata.object_type; + wpdata.object_instance = rpdata.object_instance; + wpdata.object_property = rpdata.object_property; + wpdata.array_index = rpdata.array_index; + memcpy(&wpdata.application_data, rpdata.application_data, MAX_APDU); + wpdata.application_data_len = len; + wpdata.error_code = ERROR_CODE_SUCCESS; + status = Device_Write_Property(&wpdata); + if (!status) { + /* verify WriteProperty property is known */ + zassert_not_equal(wpdata.error_code, + ERROR_CODE_UNKNOWN_PROPERTY, + "property '%s': WriteProperty Unknown!\n", + bactext_property_name(rpdata.object_property)); + } } pRequired++; } @@ -68,7 +83,9 @@ static void test_Device_ReadProperty(void) rpdata.object_property = *pOptional; rpdata.array_index = BACNET_ARRAY_ALL; len = Device_Read_Property(&rpdata); - zassert_not_equal(len, BACNET_STATUS_ERROR, NULL); + zassert_not_equal(len, BACNET_STATUS_ERROR, + "property '%s': failed to ReadProperty!\n", + bactext_property_name(rpdata.object_property)); if (len > 0) { test_len = bacapp_decode_application_data(rpdata.application_data, (uint8_t)rpdata.application_data_len, &value); @@ -76,10 +93,24 @@ static void test_Device_ReadProperty(void) printf("property '%s': failed to decode!\n", bactext_property_name(rpdata.object_property)); } - zassert_true(test_len >= 0, NULL); - } else { - printf("property '%s': failed to read!\n", + zassert_equal(test_len, len, "property '%s': failed to decode!\n", bactext_property_name(rpdata.object_property)); + /* check WriteProperty properties */ + wpdata.object_type = rpdata.object_type; + wpdata.object_instance = rpdata.object_instance; + wpdata.object_property = rpdata.object_property; + wpdata.array_index = rpdata.array_index; + memcpy(&wpdata.application_data, rpdata.application_data, MAX_APDU); + wpdata.application_data_len = len; + wpdata.error_code = ERROR_CODE_SUCCESS; + status = Device_Write_Property(&wpdata); + if (!status) { + /* verify WriteProperty property is known */ + zassert_not_equal(wpdata.error_code, + ERROR_CODE_UNKNOWN_PROPERTY, + "property '%s': WriteProperty Unknown!\n", + bactext_property_name(rpdata.object_property)); + } } pOptional++; } @@ -134,7 +165,7 @@ void test_main(void) { ztest_test_suite(device_tests, ztest_unit_test(testDevice), - ztest_unit_test(test_Device_ReadProperty) + ztest_unit_test(test_Device_Data_Sharing) ); ztest_run_test_suite(device_tests);