Bugfix/read property multiple client errors (#765)

* Fixed variable data type for boolean in RPM structure.

* Fixed RPM error handling to use callback.

* Fixed bacrpm app example when not enough command line parameters are used

* Fixed empty-list EPICS printing.

* Fixed RPM-Ack processing for end of list-of-results

* Added minimal handling for segmentation-not-supported during RPM of object properties.

apps/readpropm/main.c

@@ -496,7 +496,10 @@ int main(int argc, char *argv[])
             }
         }
     }
-    if (tag_value_arg != 0) {
+    if (!Read_Access_Data) {
+        print_usage(filename);
+        return 1;
+    } else if (tag_value_arg != 0) {
         fprintf(stderr, "Error: not enough object property triples.\n");
         return 1;
     }

src/bacnet/bacapp.c

@@ -3278,6 +3278,9 @@ int bacapp_snprintf_value(
                     str, str_len, &value->type.Shed_Level);
                 break;
 #endif
+            case BACNET_APPLICATION_TAG_EMPTYLIST:
+                ret_val = bacapp_snprintf(str, str_len, "{}");
+                break;
             default:
                 ret_val = bacapp_snprintf(
                     str, str_len, "UnknownType(tag=%d)", value->tag);
@@ -3922,7 +3925,7 @@ void bacapp_value_list_init(BACNET_APPLICATION_DATA_VALUE *value, size_t count)
     if (value && count) {
         for (i = 0; i < count; i++) {
             value->tag = BACNET_APPLICATION_TAG_NULL;
-            value->context_specific = 0;
+            value->context_specific = false;
             value->context_tag = 0;
             if ((i + 1) < count) {
                 value->next = value + 1;

src/bacnet/basic/client/bac-discover.c

@@ -677,16 +677,20 @@ static void bacnet_device_object_property_add(
 /**
  * @brief Handle the error from a ReadProperty or ReadPropertyMultiple
  * @param device_id - device instance number where data originated
- * @param error_code - BACnet Error code
+ * @param rp_data - ReadProperty data structure
+ * @param device_data - Pointer to the device data structure
  */
 static void Device_Error_Handler(
     uint32_t device_id,
-    BACNET_ERROR_CODE error_code,
+    BACNET_READ_PROPERTY_DATA *rp_data,
     BACNET_DEVICE_DATA *device_data)
 {
+    bool status = false;
+
     if (device_data) {
         debug_printf(
-            "%u - %s\n", device_id, bactext_error_code_name((int)error_code));
+            "%u - %s\n", device_id,
+            bactext_error_code_name((int)rp_data->error_code));
         switch (device_data->Discovery_State) {
             case BACNET_DISCOVER_STATE_OBJECT_LIST_REQUEST:
                 /* resend request */
@@ -697,12 +701,38 @@ static void Device_Error_Handler(
                     BACNET_DISCOVER_STATE_OBJECT_LIST_RESPONSE;
                 break;
             case BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_REQUEST:
-                /* resend request */
+                if (rp_data->error_code == ERROR_CODE_TIMEOUT) {
-                if (device_data->Object_List_Index != 0) {
+                    /* resend request */
-                    device_data->Object_List_Index--;
+                    if (device_data->Object_List_Index != 0) {
+                        device_data->Object_List_Index--;
+                    }
+                    device_data->Discovery_State =
+                        BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_RESPONSE;
+                } else if (
+                    (rp_data->error_code ==
+                     ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED) &&
+                    (rp_data->object_property == PROP_ALL)) {
+                    /* fallback to ReadProperty required properties */
+                    /* FIXME: fill a property-list with properties
+                       and use FSM to ReadProperty of each.
+                       For now, read the object-name property */
+                    status = bacnet_read_property_queue(
+                        device_id, rp_data->object_type,
+                        rp_data->object_instance, PROP_OBJECT_NAME,
+                        BACNET_ARRAY_ALL);
+                    if (status) {
+                        device_data->Discovery_State =
+                            BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_REQUEST;
+                    } else {
+                        /* skip */
+                        device_data->Discovery_State =
+                            BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_RESPONSE;
+                    }
+                } else {
+                    /* skip */
+                    device_data->Discovery_State =
+                        BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_RESPONSE;
                 }
-                device_data->Discovery_State =
-                    BACNET_DISCOVER_STATE_OBJECT_GET_PROPERTY_RESPONSE;
                 break;
             default:
                 break;
@@ -733,7 +763,7 @@ static void bacnet_read_property_reply(
         return;
     }
     if (rp_data->error_code != ERROR_CODE_SUCCESS) {
-        Device_Error_Handler(device_id, rp_data->error_code, device_data);
+        Device_Error_Handler(device_id, rp_data, device_data);
     } else if (value) {
         bacnet_device_object_property_add(
             device_id, rp_data, value, device_data);

src/bacnet/basic/client/bac-rw.c

@@ -211,15 +211,29 @@ static void bacnet_read_property_ack_process(
     BACNET_ARRAY_INDEX array_index = 0;
 
     if (rp_data) {
+        value = &Target_Decoded_Property_Value;
+        /* check for property error */
         if (rp_data->error_code != ERROR_CODE_SUCCESS) {
             if (bacnet_read_write_value_callback) {
                 bacnet_read_write_value_callback(device_id, rp_data, NULL);
             }
+            return;
+        }
+        /* check for empty list */
+        if (rp_data->application_data_len == 0) {
+            bacapp_value_list_init(value, 1);
+            value->tag = BACNET_APPLICATION_TAG_EMPTYLIST;
+            rp_data->error_class = ERROR_CLASS_SERVICES;
+            rp_data->error_code = ERROR_CODE_SUCCESS;
+            if (bacnet_read_write_value_callback) {
+                bacnet_read_write_value_callback(device_id, rp_data, value);
+            }
+            return;
         }
         apdu = rp_data->application_data;
         apdu_len = rp_data->application_data_len;
         while (apdu_len) {
-            value = &Target_Decoded_Property_Value;
+            bacapp_value_list_init(value, 1);
             len = bacapp_decode_known_property(
                 apdu, (unsigned)apdu_len, value, rp_data->object_type,
                 rp_data->object_property);
@@ -251,7 +265,11 @@ static void bacnet_read_property_ack_process(
                 }
             } else {
                 rp_data->error_class = ERROR_CLASS_SERVICES;
-                rp_data->error_code = ERROR_CODE_SUCCESS;
+                if (len < 0) {
+                    rp_data->error_code = ERROR_CODE_OTHER;
+                } else {
+                    rp_data->error_code = ERROR_CODE_SUCCESS;
+                }
                 if (bacnet_read_write_value_callback) {
                     bacnet_read_write_value_callback(device_id, rp_data, NULL);
                 }

src/bacnet/basic/service/h_rpm_a.c

@@ -103,18 +103,19 @@ int rpm_ack_decode_service_request(
                 decoded_len++;
                 apdu_len--;
                 apdu++;
-                /* note: if this is an array, there will be
-                   more than one element to decode */
                 value = calloc(1, sizeof(BACNET_APPLICATION_DATA_VALUE));
                 rpm_property->value = value;
-
-                /* Special case for an empty array - we decode it as null */
                 if (apdu_len && decode_is_closing_tag_number(apdu, 4)) {
-                    bacapp_value_list_init(value, 1);
+                    /* Special case for an empty array or list */
+                    if (value) {
+                        bacapp_value_list_init(value, 1);
+                        value->tag = BACNET_APPLICATION_TAG_EMPTYLIST;
+                    }
                     decoded_len++;
                     apdu_len--;
                     apdu++;
                 } else {
+                    /* one or more (array or list) elements to decode */
                     while (value && (apdu_len > 0)) {
                         len = bacapp_decode_known_property(
                             apdu, (unsigned)apdu_len, value,

src/bacnet/rpm.c

@@ -663,7 +663,7 @@ void rpm_ack_object_property_process(
     read_property_ack_process callback)
 {
     int len = 0;
-    uint16_t application_data_len;
+    int application_data_len;
     uint32_t error_value = 0; /* decoded error value */
 
     if (!apdu) {
@@ -674,21 +674,45 @@ void rpm_ack_object_property_process(
     }
     while (apdu_len) {
         /*  object-identifier [0] BACnetObjectIdentifier */
-        /*      list-of-results [1] SEQUENCE OF SEQUENCE */
+        /*  list-of-results [1] SEQUENCE OF SEQUENCE */
         len = rpm_ack_decode_object_id(
             apdu, apdu_len, &rp_data->object_type, &rp_data->object_instance);
         if (len <= 0) {
             /* malformed */
+            rp_data->error_class = ERROR_CLASS_SERVICES;
+            rp_data->error_code = ERROR_CODE_INVALID_TAG;
+            if (callback) {
+                callback(device_id, rp_data);
+            }
             return;
         }
         apdu_len -= len;
         apdu += len;
         while (apdu_len) {
+            if (bacnet_is_closing_tag_number(apdu, apdu_len, 1, &len)) {
+                /*  end of list-of-results [1] SEQUENCE OF SEQUENCE */
+                apdu_len -= len;
+                if (apdu_len > 0) {
+                    /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
+                    return;
+                }
+                break;
+            }
             len = rpm_ack_decode_object_property(
                 apdu, apdu_len, &rp_data->object_property,
                 &rp_data->array_index);
             if (len <= 0) {
                 /* malformed */
+                rp_data->error_class = ERROR_CLASS_SERVICES;
+                rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                if (callback) {
+                    callback(device_id, rp_data);
+                }
                 return;
             }
             apdu_len -= len;
@@ -696,20 +720,33 @@ void rpm_ack_object_property_process(
             if (bacnet_is_opening_tag_number(apdu, apdu_len, 4, &len)) {
                 application_data_len =
                     bacnet_enclosed_data_length(apdu, apdu_len);
+                if (application_data_len < 0) {
+                    /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
+                    return;
+                }
                 /* propertyValue */
                 apdu_len -= len;
                 apdu += len;
-                if (application_data_len) {
+                /* fill the RP application data */
-                    rp_data->application_data_len = application_data_len;
+                rp_data->application_data_len = application_data_len;
-                    rp_data->application_data = apdu;
+                rp_data->application_data = apdu;
-                    apdu_len -= application_data_len;
+                apdu_len -= application_data_len;
-                    apdu += application_data_len;
+                apdu += application_data_len;
-                }
                 if (bacnet_is_closing_tag_number(apdu, apdu_len, 4, &len)) {
                     apdu_len -= len;
                     apdu += len;
                 } else {
                     /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
                     return;
                 }
                 rp_data->error_class = ERROR_CLASS_PROPERTY;
@@ -729,6 +766,11 @@ void rpm_ack_object_property_process(
                     apdu += len;
                 } else {
                     /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
                     return;
                 }
                 len = bacnet_enumerated_application_decode(
@@ -739,6 +781,11 @@ void rpm_ack_object_property_process(
                     apdu += len;
                 } else {
                     /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
                     return;
                 }
                 if (bacnet_is_closing_tag_number(apdu, apdu_len, 5, &len)) {
@@ -746,6 +793,11 @@ void rpm_ack_object_property_process(
                     apdu += len;
                 } else {
                     /* malformed */
+                    rp_data->error_class = ERROR_CLASS_SERVICES;
+                    rp_data->error_code = ERROR_CODE_INVALID_TAG;
+                    if (callback) {
+                        callback(device_id, rp_data);
+                    }
                     return;
                 }
                 if (callback) {
@@ -753,11 +805,6 @@ void rpm_ack_object_property_process(
                 }
             }
         }
-        len = rpm_decode_object_end(apdu, apdu_len);
-        if (len) {
-            apdu_len -= len;
-            apdu += len;
-        }
     }
 }
 #endif