Bugfix/secure read range codec (#957)

* Secured ReadRange service codecs. Added ReadRange unit testing. Secured ReadRange-ACK handler to enable APDU size checking.

.github/workflows/bsc-tests-linux.yml

@@ -12,70 +12,81 @@ jobs:
   job_bsc_tests_linux:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+      - name: Checkout BACnet Stack
-    - name: Create Build Environment
+        uses: actions/checkout@v4
-      run: |
+      - name: Checkout libwebsockets
-        sudo apt-get update -qq
+        uses: actions/checkout@v4
-        sudo apt-get install -qq libconfig-dev
+        with:
-        sudo apt-get install -qq libcap-dev
+          repository: warmcat/libwebsockets
-        sudo apt-get install -qq libssl-dev
+          ref: v4.3-stable
-        sudo apt-get install -qq libuv1-dev
+          path: libwebsockets
-        git clone --branch v4.3-stable https://github.com/warmcat/libwebsockets.git
+      - name: Create Build Environment
-        bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
+        run: |
-        sudo bash -c 'cd libwebsockets;cd build;make install'
+          sudo apt-get update -qq
-    - name: Build and run bsc_event test
+          sudo apt-get install -qq libconfig-dev
-      run: |
+          sudo apt-get install -qq libcap-dev
-        cd ./test/ports/linux/bsc_event
+          sudo apt-get install -qq libssl-dev
-        mkdir build
+          sudo apt-get install -qq libuv1-dev
-        cd build
+          bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
-        cmake ..
+          sudo bash -c 'cd libwebsockets;cd build;make install'
-        make
+      - name: Build and run bsc_event test
-        ./test_bsc_event
+        run: |
-    - name: Build and run bvlc-sc test
+          pwd
-      run: |
+          cd ./test/ports/linux/bsc_event
-        cd ./test/bacnet/datalink/bvlc-sc
+          mkdir build
-        mkdir build
+          cd build
-        cd build
+          cmake ..
-        cmake ..
+          make
-        make
+          ./test_bsc_event
-        ./test_bvlc-sc
+      - name: Build and run bvlc-sc test
-    - name: Build and run websockets test
+        run: |
-      run: |
+          pwd
-        cd ./test/bacnet/datalink/websockets
+          cd ./test/bacnet/datalink/bvlc-sc
-        mkdir build
+          mkdir build
-        cd build
+          cd build
-        cmake ..
+          cmake ..
-        make
+          make
-        ./test_websockets
+          ./test_bvlc-sc
-    - name: Build and run bsc-socket test
+      - name: Build and run websockets test
-      run: |
+        run: |
-        cd ./test/bacnet/datalink/bsc-socket
+          pwd
-        mkdir build
+          cd ./test/bacnet/datalink/websockets
-        cd build
+          mkdir build
-        cmake ..
+          cd build
-        make
+          cmake ..
-        ./test_bsc-socket
+          make
-    - name: Build and run hub-sc test
+          ./test_websockets
-      run: |
+      - name: Build and run bsc-socket test
-        cd ./test/bacnet/datalink/hub-sc
+        run: |
-        mkdir build
+          pwd
-        cd build
+          cd ./test/bacnet/datalink/bsc-socket
-        cmake ..
+          mkdir build
-        make
+          cd build
-        ./test_hub-sc
+          cmake ..
-    - name: Build and run bsc-node test
+          make
-      run: |
+          ./test_bsc-socket
-        cd ./test/bacnet/datalink/bsc-node
+      - name: Build and run hub-sc test
-        mkdir build
+        run: |
-        cd build
+          pwd
-        cmake ..
+          cd ./test/bacnet/datalink/hub-sc
-        make
+          mkdir build
-        ./test_bsc-node
+          cd build
-    - name: Build and run bsc-datalink test
+          cmake ..
-      run: |
+          make
-        cd ./test/bacnet/datalink/bsc-datalink
+          ./test_hub-sc
-        mkdir build
+      - name: Build and run bsc-node test
-        cd build
+        run: |
-        cmake ..
+          cd ./test/bacnet/datalink/bsc-node
-        make
+          mkdir build
-        ./test_bsc-datalink
+          cd build
+          cmake ..
+          make
+          ./test_bsc-node
+      - name: Build and run bsc-datalink test
+        run: |
+          cd ./test/bacnet/datalink/bsc-datalink
+          mkdir build
+          cd build
+          cmake ..
+          make
+          ./test_bsc-datalink

.github/workflows/bsc-tests-windows.yml

@@ -1,12 +1,7 @@
 name: BACNet/SC windows tests
 
 on:
-  push:
+  workflow_dispatch:
-    branches:
-      - master
-  pull_request:
-    branches:
-      - '*'
 
 jobs:
   job_bsc_tests_windows:
@@ -35,7 +30,7 @@ jobs:
         Copy-Item -Path .\pkg\lib -Destination c:\vcpkg\installed\x64-windows-custom -Recurse -Force
         Copy-Item -Path .\pkg\share -Destination c:\vcpkg\installed\x64-windows-custom -Recurse -Force
         cd ../../
-    - name: Setup MSBuild 
+    - name: Setup MSBuild
       uses: microsoft/setup-msbuild@v2
       with:
         msbuild-architecture: x64
@@ -63,7 +58,7 @@ jobs:
         cmake -B build -S . -DCMAKE_TOOLCHAIN_FILE=c:\vcpkg\scripts\buildsystems\vcpkg.cmake -DVCPKG_TARGET_TRIPLET=x64-windows-custom
         msbuild build/test_bsc-socket.sln /Property:Configuration=Release
         ./build/Release/test_bsc-socket.exe
-    # (Link target) -> 
+    # (Link target) ->
     #  bacreal.obj : error LNK2019: unresolved external symbol big_endian referenced in function decode_double [D:\a\bacnet-stack-upstream\bacnet-stack-upstream\test\bacnet\datalink\hub-sc\build\test_hub-sc.vcxproj]
     # - name: Build and run hub-sc test
     #   run: |

.github/workflows/gcc.yml

@@ -80,7 +80,14 @@ jobs:
   bacnet-sc-hub:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - name: Checkout BACnet Stack
+      uses: actions/checkout@v4
+    - name: Checkout libwebsockets
+      uses: actions/checkout@v4
+      with:
+        repository: warmcat/libwebsockets
+        ref: v4.3-stable
+        path: libwebsockets
     - name: Create BACnet/SC Build Environment
       run: |
         sudo apt-get update -qq
@@ -88,19 +95,27 @@ jobs:
         sudo apt-get install -qq libcap-dev
         sudo apt-get install -qq libssl-dev
         sudo apt-get install -qq libuv1-dev
-        git clone  --branch v4.3-stable https://github.com/warmcat/libwebsockets.git
         bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
         sudo bash -c 'cd libwebsockets;cd build;make install'
     - name: Build BACnet/SC Hub Demo
       run: |
         gcc --version
+        pwd
+        ls -al
         make clean
         make sc-hub
 
   router:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - name: Checkout BACnet Stack
+      uses: actions/checkout@v4
+    - name: Checkout libwebsockets
+      uses: actions/checkout@v4
+      with:
+        repository: warmcat/libwebsockets
+        ref: v4.3-stable
+        path: libwebsockets
     - name: Create Build Environment
       run: |
         sudo apt-get update -qq
@@ -108,12 +123,13 @@ jobs:
         sudo apt-get install -qq libcap-dev
         sudo apt-get install -qq libssl-dev
         sudo apt-get install -qq libuv1-dev
-        git clone  --branch v4.3-stable https://github.com/warmcat/libwebsockets.git
         bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
         sudo bash -c 'cd libwebsockets;cd build;make install'
     - name: Build Router Demo
       run: |
         gcc --version
+        pwd
+        ls -al
         make clean
         make LEGACY=true router
 

.github/workflows/lint.yml

@@ -85,14 +85,34 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Create Build Environment
+      run: |
+        sudo apt-get update -qq
+        sudo apt-get install -qq lcov
+    - name: Run Unit Test with Code Coverage
+      run: make test
+
+  unittest-secure-connect:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout BACnet Stack
+      uses: actions/checkout@v4
+    - name: Checkout libwebsockets
+      uses: actions/checkout@v4
+      with:
+        repository: warmcat/libwebsockets
+        ref: v4.3-stable
+        path: libwebsockets
+    - name: Create BACnet/SC Build Environment
       run: |
         sudo apt-get update -qq
         sudo apt-get install -qq lcov
         sudo apt-get install -qq libssl-dev
         sudo apt-get install -qq libcap-dev
         sudo apt-get install -qq libuv1-dev
-        git clone  --branch v4.3-stable https://github.com/warmcat/libwebsockets.git
         bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
         sudo bash -c 'cd libwebsockets;cd build;make install'
-    - name: Run Unit Test with Code Coverage
+    - name: Run BACnet/SC Unit Test with Code Coverage
-      run: make test
+      run: |
+        pwd
+        ls -al
+        echo "make test-bsc"

Makefile

@@ -519,6 +519,11 @@ test:
 retest:
 	$(MAKE) -s -j -C test retest
 
+.PHONY: test-bsc
+test-bsc:
+	$(MAKE) -s -C test clean
+	$(MAKE) -s -j -C test test-bsc
+
 # Zephyr unit testing with twister
 # expects zephyr to be installed in ../zephyr in Workspace
 # expects ZEPHYR_BASE to be set. E.g. source ../zephyr/zephyr-env.sh

src/bacnet/bacdcode.c

@@ -1517,8 +1517,7 @@ int decode_bitstring(
  * @param apdu - buffer to hold the bytes
  * @param apdu_size - number of bytes in the buffer to decode
  * @param len_value - number of bytes in the unsigned value encoding
- * @param value - value to decode into
+ * @param value - value to decode into, or NULL for length checking
- *
  * @return  number of bytes decoded, or zero if errors occur
  */
 int bacnet_bitstring_decode(
@@ -1533,7 +1532,7 @@ int bacnet_bitstring_decode(
     uint32_t bytes_used;
 
     /* check to see if the APDU is long enough */
-    if (apdu && value && (len_value <= apdu_size)) {
+    if (apdu && (len_value <= apdu_size)) {
         /* Init/empty the string. */
         bitstring_init(value);
         if (len_value > 0) {
@@ -1544,7 +1543,8 @@ int bacnet_bitstring_decode(
                 /* Copy the bytes in reversed bit order. */
                 for (i = 0; i < bytes_used; i++) {
                     bitstring_set_octet(
-                        value, (uint8_t)i, byte_reverse_bits(apdu[len++]));
+                        value, (uint8_t)i, byte_reverse_bits(apdu[len]));
+                    len++;
                 }
                 /* Erase the remaining unused bits. */
                 unused_bits = (uint8_t)(apdu[0] & 0x07);

src/bacnet/basic/service/h_rr.c

@@ -154,14 +154,16 @@ void handler_read_range(
             len = Encode_RR_payload(&Temp_Buf[0], &data);
             if (len >= 0) {
                 /* encode the APDU portion of the packet */
-                data.application_data = &Temp_Buf[0];
+                len = rr_ack_encode_apdu(NULL, service_data->invoke_id, &data);
-                data.application_data_len = len;
+                if (len < sizeof(Handler_Transmit_Buffer) - pdu_len) {
-                /* FIXME: probably need a length limitation sent with encode */
+                    len = rr_ack_encode_apdu(
-                len = rr_ack_encode_apdu(
+                        &Handler_Transmit_Buffer[pdu_len],
-                    &Handler_Transmit_Buffer[pdu_len], service_data->invoke_id,
+                        service_data->invoke_id, &data);
-                    &data);
+                    debug_print("RR: Sending Ack!\n");
-                debug_print("RR: Sending Ack!\n");
+                    error = false;
-                error = false;
+                } else {
+                    len = -2; /* too big */
+                }
             }
             if (error) {
                 if (len == -2) {
@@ -182,7 +184,6 @@ void handler_read_range(
             }
         }
     }
-
     pdu_len += len;
     bytes_sent = datalink_send_pdu(
         src, &npdu_data, &Handler_Transmit_Buffer[0], pdu_len);

src/bacnet/readrange.c

@@ -205,188 +205,197 @@ int rr_encode_apdu(
 /**
  * Decode the received ReadRange request
  *
- *  @param apdu  Pointer to the APDU buffer.
+ *  @param apdu Pointer to the APDU buffer.
- *  @param apdu_len  Bytes valid in the APDU buffer.
+ *  @param apdu_size number of bytes in the APDU buffer.
- *  @param rrdata  Pointer to the data used for encoding.
+ *  @param data Pointer to the data filled while decoding.
- *
+ *  @return Bytes decoded, or #BACNET_STATUS_ERROR
- *  @return Bytes encoded.
  */
 int rr_decode_service_request(
-    const uint8_t *apdu, unsigned apdu_len, BACNET_READ_RANGE_DATA *rrdata)
+    const uint8_t *apdu, unsigned apdu_size, BACNET_READ_RANGE_DATA *data)
 {
-    unsigned len = 0;
+    int len = 0, apdu_len = 0;
-    unsigned TagLen = 0;
+    uint32_t value32 = 0;
-    uint8_t tag_number = 0;
+    int32_t signed_value = 0;
-    uint32_t len_value_type = 0;
+    BACNET_OBJECT_TYPE object_type = OBJECT_NONE;
-    BACNET_OBJECT_TYPE type = OBJECT_NONE; /* for decoding */
+    uint32_t enum_value = 0;
-    uint32_t enum_value;
+    BACNET_UNSIGNED_INTEGER unsigned_value = 0;
-    BACNET_UNSIGNED_INTEGER unsigned_value;
+    BACNET_DATE *bdate = NULL;
+    BACNET_TIME *btime = NULL;
 
     /* check for value pointers */
-    if ((apdu_len >= 5) && apdu && rrdata) {
+    if (!apdu) {
-        /* Tag 0: Object ID */
+        return BACNET_STATUS_ERROR;
-        if (!decode_is_context_tag(&apdu[len++], 0)) {
+    }
-            return -1;
+    /* objectIdentifier   [0] BACnetObjectIdentifier */
-        }
+    len = bacnet_object_id_context_decode(
-        len += decode_object_id(&apdu[len], &type, &rrdata->object_instance);
+        &apdu[apdu_len], apdu_size - apdu_len, 0, &object_type, &value32);
-        rrdata->object_type = type;
+    if (len > 0) {
-        /* Tag 1: Property ID */
+        apdu_len += len;
-        if (len >= apdu_len) {
+        if (data) {
-            return (-1);
+            data->object_type = object_type;
-        }
+            data->object_instance = value32;
-        len += decode_tag_number_and_value(
-            &apdu[len], &tag_number, &len_value_type);
-        if (tag_number != 1) {
-            return -1;
-        }
-        len += decode_enumerated(&apdu[len], len_value_type, &enum_value);
-        rrdata->object_property = (BACNET_PROPERTY_ID)enum_value;
-        rrdata->Overhead = RR_OVERHEAD; /* Start with the fixed overhead */
-
-        /* Tag 2: Optional Array Index - set to ALL if not present */
-        rrdata->array_index = BACNET_ARRAY_ALL; /* Assuming this is the most
-                                                   common outcome... */
-        if (len < apdu_len) {
-            TagLen = (unsigned)decode_tag_number_and_value(
-                &apdu[len], &tag_number, &len_value_type);
-            if (tag_number == 2) {
-                len += TagLen;
-                len += decode_unsigned(
-                    &apdu[len], len_value_type, &unsigned_value);
-                rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
-                rrdata->Overhead +=
-                    RR_INDEX_OVERHEAD; /* Allow for this in the response */
-            }
-        }
-        /* And/or optional range selection- Tags 3, 6 and 7 */
-        rrdata->RequestType = RR_READ_ALL; /* Assume the worst to cut out
-                                              explicit checking later */
-        if (len < apdu_len) {
-            /*
-             * Note: We pick up the opening tag and then decode the
-             * parameter types we recognise. We deal with the count and the
-             * closing tag in each case statement even though it might
-             * appear that we could do them after the switch statement as
-             * common elements. This is so that if we receive a tag we don't
-             * recognise, we don't try to decode it blindly and make a mess
-             * of it.
-             */
-            len += decode_tag_number_and_value(
-                &apdu[len], &tag_number, &len_value_type);
-            switch (tag_number) {
-                case 3: /* ReadRange by position */
-                    rrdata->RequestType = RR_BY_POSITION;
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_unsigned(
-                        &apdu[len], len_value_type, &unsigned_value);
-                    rrdata->Range.RefIndex = (uint32_t)unsigned_value;
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_signed(
-                        &apdu[len], len_value_type, &rrdata->Count);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    break;
-
-                case 6: /* ReadRange by sequence number */
-                    rrdata->RequestType = RR_BY_SEQUENCE;
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_unsigned(
-                        &apdu[len], len_value_type, &unsigned_value);
-                    rrdata->Range.RefSeqNum = (uint32_t)unsigned_value;
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_signed(
-                        &apdu[len], len_value_type, &rrdata->Count);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    /* Allow for this in the response */
-                    rrdata->Overhead += RR_1ST_SEQ_OVERHEAD;
-                    break;
-
-                case 7: /* ReadRange by time stamp */
-                    rrdata->RequestType = RR_BY_TIME;
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_date(&apdu[len], &rrdata->Range.RefTime.date);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_bacnet_time(
-                        &apdu[len], &rrdata->Range.RefTime.time);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_signed(
-                        &apdu[len], len_value_type, &rrdata->Count);
-                    if (len >= apdu_len) {
-                        break;
-                    }
-                    len += decode_tag_number_and_value(
-                        &apdu[len], &tag_number, &len_value_type);
-                    /* Allow for this in the response */
-                    rrdata->Overhead += RR_1ST_SEQ_OVERHEAD;
-                    break;
-
-                default: /* If we don't recognise the tag then we do nothing
-                          * here and try to return all elements of the array
-                          */
-                    break;
-            }
         }
     } else {
-        return (-1);
+        return BACNET_STATUS_ERROR;
+    }
+    /* propertyIdentifier [1] BACnetPropertyIdentifier */
+    len = bacnet_enumerated_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 1, &enum_value);
+    if (len > 0) {
+        apdu_len += len;
+        if (data) {
+            data->object_property = (BACNET_PROPERTY_ID)enum_value;
+            data->Overhead = RR_OVERHEAD; /* Start with the fixed overhead */
+        }
+    } else {
+        return BACNET_STATUS_ERROR;
+    }
+    /* propertyArrayIndex [2] Unsigned OPTIONAL */
+    len = bacnet_unsigned_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 2, &unsigned_value);
+    if (len > 0) {
+        apdu_len += len;
+        if (data) {
+            data->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
+            data->Overhead += RR_INDEX_OVERHEAD;
+        }
+    } else if (len == 0) {
+        /* OPTIONAL missing - skip adding len */
+        if (data) {
+            data->array_index = BACNET_ARRAY_ALL;
+        }
+    } else {
+        return BACNET_STATUS_ERROR;
+    }
+    if (bacnet_is_opening_tag_number(
+            &apdu[apdu_len], apdu_size - apdu_len, 3, &len)) {
+        /*
+            byPosition [3] SEQUENCE {
+                referenceIndex  Unsigned,
+                count           INTEGER
+            }
+        */
+        apdu_len += len;
+        if (data) {
+            data->RequestType = RR_BY_POSITION;
+        }
+        len = bacnet_unsigned_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, &unsigned_value);
+        if (len > 0) {
+            apdu_len += len;
+            if (data) {
+                data->Range.RefIndex = (uint32_t)unsigned_value;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        len = bacnet_signed_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, &signed_value);
+        if (len > 0) {
+            apdu_len += len;
+            if (data) {
+                data->Count = signed_value;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        if (bacnet_is_closing_tag_number(
+                &apdu[apdu_len], apdu_size - apdu_len, 3, &len)) {
+            apdu_len += len;
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+    } else if (bacnet_is_opening_tag_number(
+                   &apdu[apdu_len], apdu_size - apdu_len, 6, &len)) {
+        /*
+            bySequenceNumber [6] SEQUENCE {
+                referenceIndex  Unsigned,
+                count           INTEGER
+            }
+        */
+        apdu_len += len;
+        if (data) {
+            data->RequestType = RR_BY_SEQUENCE;
+        }
+        len = bacnet_unsigned_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, &unsigned_value);
+        if (len > 0) {
+            apdu_len += len;
+            if (data) {
+                data->Range.RefSeqNum = (uint32_t)unsigned_value;
+                data->Overhead += RR_1ST_SEQ_OVERHEAD;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        len = bacnet_signed_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, &signed_value);
+        if (len > 0) {
+            apdu_len += len;
+            if (data) {
+                data->Count = signed_value;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        if (bacnet_is_closing_tag_number(
+                &apdu[apdu_len], apdu_size - apdu_len, 6, &len)) {
+            apdu_len += len;
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+    } else if (bacnet_is_opening_tag_number(
+                   &apdu[apdu_len], apdu_size - apdu_len, 7, &len)) {
+        /*
+            byTime [7] SEQUENCE {
+                referenceTime   BACnetDateTime,
+                count           INTEGER
+            }
+        */
+        apdu_len += len;
+        if (data) {
+            data->RequestType = RR_BY_TIME;
+            bdate = &data->Range.RefTime.date;
+            btime = &data->Range.RefTime.time;
+        }
+        len = bacnet_date_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, bdate);
+        if (len > 0) {
+            apdu_len += len;
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        len = bacnet_time_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, btime);
+        if (len > 0) {
+            apdu_len += len;
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        len = bacnet_signed_application_decode(
+            &apdu[apdu_len], apdu_size - apdu_len, &signed_value);
+        if (len > 0) {
+            apdu_len += len;
+            if (data) {
+                data->Count = signed_value;
+            }
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+        if (bacnet_is_closing_tag_number(
+                &apdu[apdu_len], apdu_size - apdu_len, 7, &len)) {
+            apdu_len += len;
+        } else {
+            return BACNET_STATUS_ERROR;
+        }
+    } else {
+        /* OPTIONAL range missing - skip adding len */
+        if (data) {
+            data->RequestType = RR_READ_ALL;
+        }
     }
 
-    return (int)len;
+    return apdu_len;
 }
 
 /*
@@ -404,70 +413,134 @@ int rr_decode_service_request(
  * }
  */
 
+/**
+ * @brief Encode ReadRange-ACK service APDU
+ * @param apdu  Pointer to the buffer, or NULL for length
+ * @param data  Pointer to the data to encode.
+ * @return number of bytes encoded, or zero on error.
+ */
+int readrange_ack_encode(uint8_t *apdu, const BACNET_READ_RANGE_DATA *data)
+{
+    int apdu_len = 0; /* total length of the apdu, return value */
+    int len = 0;
+
+    if (!data) {
+        return 0;
+    }
+    len = encode_context_object_id(
+        apdu, 0, data->object_type, data->object_instance);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    len = encode_context_enumerated(apdu, 1, data->object_property);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    /* context 2 array index is optional */
+    if (data->array_index != BACNET_ARRAY_ALL) {
+        len = encode_context_unsigned(apdu, 2, data->array_index);
+        apdu_len += len;
+        if (apdu) {
+            apdu += len;
+        }
+    }
+    /* Context 3 BACnet Result Flags */
+    len = encode_context_bitstring(apdu, 3, &data->ResultFlags);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    /* Context 4 Item Count */
+    len = encode_context_unsigned(apdu, 4, data->ItemCount);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    /* Context 5 Property list - reading the standard it looks like an
+     * empty list still requires an opening and closing tag as the
+     * tagged parameter is not optional
+     */
+    len = encode_opening_tag(apdu, 5);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    if (data->application_data_len > 0) {
+        for (len = 0; len < data->application_data_len; len++) {
+            if (apdu) {
+                apdu[len] = data->application_data[len];
+            }
+        }
+        apdu_len += len;
+        if (apdu) {
+            apdu += len;
+        }
+    }
+    len = encode_closing_tag(apdu, 5);
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    if ((data->ItemCount != 0) && (data->RequestType != RR_BY_POSITION) &&
+        (data->RequestType != RR_READ_ALL)) {
+        /* Context 6 Sequence number of first item */
+        len = encode_context_unsigned(apdu, 6, data->FirstSequence);
+        apdu_len += len;
+    }
+
+    return apdu_len;
+}
+
+/**
+ * @brief Encode the ReadRange-ACK service
+ * @param apdu  Pointer to the buffer for encoding into, or NULL for length
+ * @param apdu_size number of bytes available in the buffer
+ * @param data  Pointer to the service data to be encoded
+ * @return number of bytes encoded, or zero if unable to encode or too large
+ */
+size_t readrange_ack_service_encode(
+    uint8_t *apdu, size_t apdu_size, const BACNET_READ_RANGE_DATA *data)
+{
+    size_t apdu_len = 0; /* total length of the apdu, return value */
+
+    apdu_len = readrange_ack_encode(NULL, data);
+    if (apdu_len > apdu_size) {
+        apdu_len = 0;
+    } else {
+        apdu_len = readrange_ack_encode(apdu, data);
+    }
+
+    return apdu_len;
+}
+
 /**
  * Build a ReadRange response packet
  *
  * @param apdu  Pointer to the buffer.
- * @param invoke_id  ID invoked.
+ * @param invoke_id original invoke id for request
- * @param rrdata  Pointer to the read range data structure used for
+ * @param data  Pointer to the property data to be encoded
- * encoding.
+ * @return number of bytes encoded
- *
- * @return The count of encoded bytes.
  */
 int rr_ack_encode_apdu(
-    uint8_t *apdu, uint8_t invoke_id, const BACNET_READ_RANGE_DATA *rrdata)
+    uint8_t *apdu, uint8_t invoke_id, const BACNET_READ_RANGE_DATA *data)
 {
-    int imax = 0;
-    int len = 0; /* length of each encoding */
     int apdu_len = 0; /* total length of the apdu, return value */
+    int len = 0;
 
     if (apdu) {
         apdu[0] = PDU_TYPE_COMPLEX_ACK; /* complex ACK service */
         apdu[1] = invoke_id; /* original invoke id from request */
         apdu[2] = SERVICE_CONFIRMED_READ_RANGE; /* service choice */
-        apdu_len = 3;
-        /* service ack follows */
-        apdu_len += encode_context_object_id(
-            &apdu[apdu_len], 0, rrdata->object_type, rrdata->object_instance);
-        apdu_len += encode_context_enumerated(
-            &apdu[apdu_len], 1, rrdata->object_property);
-        /* context 2 array index is optional */
-        if (rrdata->array_index != BACNET_ARRAY_ALL) {
-            apdu_len += encode_context_unsigned(
-                &apdu[apdu_len], 2, rrdata->array_index);
-        }
-        /* Context 3 BACnet Result Flags */
-        apdu_len +=
-            encode_context_bitstring(&apdu[apdu_len], 3, &rrdata->ResultFlags);
-        /* Context 4 Item Count */
-        apdu_len +=
-            encode_context_unsigned(&apdu[apdu_len], 4, rrdata->ItemCount);
-        /* Context 5 Property list - reading the standard it looks like an
-         * empty list still requires an opening and closing tag as the
-         * tagged parameter is not optional
-         */
-        apdu_len += encode_opening_tag(&apdu[apdu_len], 5);
-        if (rrdata->ItemCount != 0) {
-            imax = rrdata->application_data_len;
-            if (imax > (MAX_APDU - apdu_len - 2 /*closing*/)) {
-                imax = (MAX_APDU - apdu_len - 2);
-            }
-            for (len = 0; len < imax; len++) {
-                apdu[apdu_len++] = rrdata->application_data[len];
-            }
-        }
-        apdu_len += encode_closing_tag(&apdu[apdu_len], 5);
-
-        if ((rrdata->ItemCount != 0) &&
-            (rrdata->RequestType != RR_BY_POSITION) &&
-            (rrdata->RequestType != RR_READ_ALL)) {
-            /* Context 6 Sequence number of first item */
-            if (apdu_len < (MAX_APDU - 4)) {
-                apdu_len += encode_context_unsigned(
-                    &apdu[apdu_len], 6, rrdata->FirstSequence);
-            }
-        }
     }
+    len = 3;
+    apdu_len += len;
+    if (apdu) {
+        apdu += len;
+    }
+    len = readrange_ack_encode(apdu, data);
+    apdu_len += len;
 
     return apdu_len;
 }
@@ -475,137 +548,139 @@ int rr_ack_encode_apdu(
 /**
  * Decode the received ReadRange response
  *
- *  @param apdu  Pointer to the APDU buffer.
+ *  @param apdu Pointer to the APDU buffer.
- *  @param apdu_len  Bytes valid in the APDU buffer.
+ *  @param apdu_size Number of bytes in the APDU buffer.
- *  @param rrdata  Pointer to the data filled while decoding.
+ *  @param data Pointer to the data filled while decoding (can be NULL).
- *
+ *  @return number of bytes decoded, or #BACNET_STATUS_ERROR
- *  @return Bytes decoded.
  */
 int rr_ack_decode_service_request(
-    uint8_t *apdu,
+    uint8_t *apdu, int apdu_size, BACNET_READ_RANGE_DATA *data)
-    int apdu_len, /* total length of the apdu */
-    BACNET_READ_RANGE_DATA *rrdata)
 {
-    uint8_t tag_number = 0;
+    int apdu_len = 0;
-    uint32_t len_value_type = 0;
+    int len = 0;
-    int tag_len = 0; /* length of tag decode */
+    int data_len = 0;
-    int len = 0; /* total length of decodes */
+    BACNET_OBJECT_TYPE object_type = OBJECT_NONE;
-    int start_len;
+    uint32_t value32 = 0;
-    BACNET_OBJECT_TYPE object_type = OBJECT_NONE; /* object type */
-    uint32_t property = 0; /* for decoding */
     BACNET_UNSIGNED_INTEGER unsigned_value;
+    BACNET_BIT_STRING *bitstring = NULL;
 
     /* Check apdu_len against the len during decode. */
-    if (apdu && (apdu_len >= 5 /* minimum */)) {
+    if (!apdu) {
-        /* Tag 0: Object ID */
+        return BACNET_STATUS_ERROR;
-        if (!decode_is_context_tag(&apdu[0], 0)) {
+    }
-            return -1;
+    /* objectIdentifier    [0] BACnetObjectIdentifier */
+    len = bacnet_object_id_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 0, &object_type, &value32);
+    if (len > 0) {
+        apdu_len += len;
+        if (data) {
+            data->object_type = object_type;
+            data->object_instance = value32;
         }
-        len = 1;
+    } else {
-        len += decode_object_id(
+        return BACNET_STATUS_ERROR;
-            &apdu[len], &object_type, &rrdata->object_instance);
+    }
-        rrdata->object_type = object_type;
+    /* propertyIdentifier [1] BACnetPropertyIdentifier */
-
+    len = bacnet_enumerated_context_decode(
-        /* Tag 1: Property ID */
+        &apdu[apdu_len], apdu_size - apdu_len, 1, &value32);
-        if (len >= apdu_len) {
+    if (len > 0) {
-            return -1;
+        apdu_len += len;
+        if (data) {
+            data->object_property = (BACNET_PROPERTY_ID)value32;
         }
-        len += decode_tag_number_and_value(
+    } else {
-            &apdu[len], &tag_number, &len_value_type);
+        return BACNET_STATUS_ERROR;
-        if (tag_number != 1) {
+    }
-            return -1;
+    /* propertyArrayIndex [2] Unsigned OPTIONAL */
+    len = bacnet_unsigned_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 2, &unsigned_value);
+    if (len > 0) {
+        apdu_len += len;
+        if (data) {
+            data->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
         }
-        len += decode_enumerated(&apdu[len], len_value_type, &property);
+    } else if (len == 0) {
-        rrdata->object_property = (BACNET_PROPERTY_ID)property;
+        /* OPTIONAL missing - skip adding len */
-
+        if (data) {
-        /* Tag 2: Optional Array Index */
+            data->array_index = BACNET_ARRAY_ALL;
-        if (len >= apdu_len) {
-            return -1;
         }
-        tag_len = decode_tag_number_and_value(
+    } else {
-            &apdu[len], &tag_number, &len_value_type);
+        return BACNET_STATUS_ERROR;
-        if (tag_number == 2) {
+    }
-            len += tag_len;
+    /* resultFlags [3] BACnetResultFlags */
-            len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
+    if (data) {
-            rrdata->array_index = (BACNET_ARRAY_INDEX)unsigned_value;
+        bitstring = &data->ResultFlags;
-        } else {
+    }
-            rrdata->array_index = BACNET_ARRAY_ALL;
+    len = bacnet_bitstring_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 3, bitstring);
+    if (len > 0) {
+        apdu_len += len;
+    } else {
+        return BACNET_STATUS_ERROR;
+    }
+    /* itemCount [4] Unsigned */
+    len = bacnet_unsigned_context_decode(
+        &apdu[apdu_len], apdu_size - apdu_len, 4, &unsigned_value);
+    if (len > 0) {
+        apdu_len += len;
+        if (data) {
+            data->ItemCount = (uint32_t)unsigned_value;
         }
-
+    } else {
-        /* Tag 3: Result Flags */
+        return BACNET_STATUS_ERROR;
-        if (len >= apdu_len) {
+    }
-            return -1;
+    /* itemData [5] SEQUENCE OF ABSTRACT-SYNTAX.&TYPE */
-        }
+    if (!bacnet_is_opening_tag_number(
-        len += decode_tag_number_and_value(
+            &apdu[apdu_len], apdu_size - apdu_len, 5, &len)) {
-            &apdu[len], &tag_number, &len_value_type);
+        return BACNET_STATUS_ERROR;
-        if (tag_number != 3) {
+    }
-            return -1;
+    /* determine the length of the data blob
-        }
+       note: APDU must include the opening tag in order to find
-        if (len >= apdu_len) {
+       the matching closing tag */
-            return -1;
+    data_len =
-        }
+        bacnet_enclosed_data_length(&apdu[apdu_len], apdu_size - apdu_len);
-        len +=
+    if (data_len == BACNET_STATUS_ERROR) {
-            decode_bitstring(&apdu[len], len_value_type, &rrdata->ResultFlags);
+        return BACNET_STATUS_ERROR;
-
+    }
-        /* Tag 4: Item count */
+    /* count the opening tag number length AFTER getting the data length */
-        if (len >= apdu_len) {
+    apdu_len += len;
-            return -1;
+    /* sanity check */
-        }
+    if (data_len > MAX_APDU) {
-        len += decode_tag_number_and_value(
+        /* not enough size in application_data to store the data chunk */
-            &apdu[len], &tag_number, &len_value_type);
+        return BACNET_STATUS_ERROR;
-        if (tag_number != 4) {
+    } else if (data) {
-            return -1;
+        /* don't decode the application tag number or its data here */
-        }
+        data->application_data = &apdu[apdu_len];
-        if (len >= apdu_len) {
+        data->application_data_len = data_len;
-            return -1;
+    }
-        }
+    apdu_len += data_len;
-        len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value);
+    if (!bacnet_is_closing_tag_number(
-        rrdata->ItemCount = (uint32_t)unsigned_value;
+            &apdu[apdu_len], apdu_size - apdu_len, 5, &len)) {
-        if (len >= apdu_len) {
+        return BACNET_STATUS_ERROR;
-            return -1;
+    }
-        }
+    /* count the closing tag number length */
-        if (decode_is_opening_tag_number(&apdu[len], 5)) {
+    apdu_len += len;
-            len++; /* A tag number of 5 is not extended so only one octet
+    /* firstSequenceNumber [6] Unsigned32 OPTIONAL
-                    * Setup the start position and length of the data
+        -- used only if 'Item Count' > 0 and
-                    * returned from the request don't decode the application
+        -- the request was either of type 'By Sequence Number'
-                    * tag number or its data here. */
+        -- or 'By Time' */
-            rrdata->application_data = &apdu[len];
+    if (apdu_len < apdu_size) {
-            start_len = len;
+        len = bacnet_unsigned_context_decode(
-            while (len < apdu_len) {
+            &apdu[apdu_len], apdu_size - apdu_len, 6, &unsigned_value);
-                if (IS_CONTEXT_SPECIFIC(apdu[len]) &&
+        if (len > 0) {
-                    (decode_is_closing_tag_number(&apdu[len], 5))) {
+            apdu_len += len;
-                    rrdata->application_data_len = len - start_len;
+            if (data) {
-                    len++; /* Step over single byte closing tag */
+                data->FirstSequence = (uint32_t)unsigned_value;
-                    break;
+            }
-                } else {
+        } else if (len == 0) {
-                    /* Don't care about tag number, just skipping over
+            /* OPTIONAL missing - skip adding len */
-                     * anyway */
+            if (data) {
-                    len += decode_tag_number_and_value(
+                data->FirstSequence = 0;
-                        &apdu[len], NULL, &len_value_type);
-                    len += len_value_type; /* Skip over data value as well */
-                    if (len >= apdu_len) { /* APDU is exhausted so we have
-                                            * failed to find closing tag */
-                        return (-1);
-                    }
-                }
             }
         } else {
-            return -1;
+            return BACNET_STATUS_ERROR;
-        }
-        if (len < apdu_len) { /* Still something left to look at? */
-            /* Tag 6: FirstSequence */
-            len += decode_tag_number_and_value(
-                &apdu[len], &tag_number, &len_value_type);
-            if (tag_number != 6) {
-                return -1;
-            }
-            if (len < apdu_len) {
-                len += decode_unsigned(
-                    &apdu[len], len_value_type, &unsigned_value);
-                rrdata->FirstSequence = (uint32_t)unsigned_value;
-            }
         }
     }
 
-    return len;
+    return apdu_len;
 }

src/bacnet/readrange.h

@@ -44,14 +44,13 @@ typedef struct BACnet_Read_Range_Data {
 /** Defines to indicate which type of read range request it is.
    Not really a bit map but we do it like this to allow quick
    checking of request against capabilities for the property */
-
 #define RR_BY_POSITION 1
 #define RR_BY_SEQUENCE 2
 #define RR_BY_TIME 4
-#define RR_READ_ALL \
+/**< Read all of the list, and don't encode OPTIONAL range in the request */
-    8 /**< Read all of array - so don't send any range in the request */
+#define RR_READ_ALL 8
-#define RR_ARRAY_OF_LISTS \
+/**< For info functionality indicates array of lists if set */
-    16 /**< For info functionality indicates array of lists if set */
+#define RR_ARRAY_OF_LISTS 16
 
 /** Bit String Enumerations */
 typedef enum {
@@ -135,6 +134,11 @@ int rr_decode_service_request(
 BACNET_STACK_EXPORT
 int rr_ack_encode_apdu(
     uint8_t *apdu, uint8_t invoke_id, const BACNET_READ_RANGE_DATA *rrdata);
+BACNET_STACK_EXPORT
+int readrange_ack_encode(uint8_t *apdu, const BACNET_READ_RANGE_DATA *data);
+BACNET_STACK_EXPORT
+size_t readrange_ack_service_encode(
+    uint8_t *apdu, size_t apdu_size, const BACNET_READ_RANGE_DATA *data);
 
 BACNET_STACK_EXPORT
 int rr_ack_decode_service_request(

test/CMakeLists.txt

@@ -107,6 +107,7 @@ list(APPEND testdirs
   bacnet/property
   bacnet/ptransfer
   bacnet/rd
+  bacnet/readrange
   bacnet/reject
   bacnet/rp
   bacnet/rpm

test/Makefile

@@ -140,6 +140,8 @@ libwebsockets:
 	bash -c 'cd libwebsockets;mkdir build;cd build;cmake .. -DLWS_WITH_LIBUV=ON -DLWS_WITH_MINIMAL_EXAMPLES=0 -DLWS_MAX_SMP=32;make'
 	sudo bash -c 'cd libwebsockets;cd build;make install'
 
+test-bsc: bsc-datalink bsc-node bsc-hub bsc-bvlc bsc-socket websockets
+
 .PHONY: clean
 clean:
 	-rm -rf $(BUILD_DIR)

test/bacnet/readrange/CMakeLists.txt

@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: MIT
+
+cmake_minimum_required(VERSION 3.10 FATAL_ERROR)
+
+get_filename_component(basename ${CMAKE_CURRENT_SOURCE_DIR} NAME)
+project(test_${basename}
+    VERSION 1.0.0
+    LANGUAGES C)
+
+
+string(REGEX REPLACE
+    "/test/bacnet/[a-zA-Z_/-]*$"
+    "/src"
+    SRC_DIR
+    ${CMAKE_CURRENT_SOURCE_DIR})
+string(REGEX REPLACE
+    "/test/bacnet/[a-zA-Z_/-]*$"
+    "/test"
+    TST_DIR
+    ${CMAKE_CURRENT_SOURCE_DIR})
+set(ZTST_DIR "${TST_DIR}/ztest/src")
+
+add_compile_definitions(
+    BIG_ENDIAN=0
+    CONFIG_ZTEST=1
+    )
+
+include_directories(
+    ${SRC_DIR}
+    ${TST_DIR}/ztest/include
+    )
+
+add_executable(${PROJECT_NAME}
+    # File(s) under test
+    ${SRC_DIR}/bacnet/readrange.c
+    # Support files and stubs (pathname alphabetical)
+    ${SRC_DIR}/bacnet/bacdcode.c
+    ${SRC_DIR}/bacnet/bacint.c
+    ${SRC_DIR}/bacnet/bacreal.c
+    ${SRC_DIR}/bacnet/bacstr.c
+    ${SRC_DIR}/bacnet/proplist.c
+    ${SRC_DIR}/bacnet/basic/sys/bigend.c
+    # Test and test library files
+    ./src/main.c
+    ${ZTST_DIR}/ztest_mock.c
+    ${ZTST_DIR}/ztest.c
+    )

test/bacnet/readrange/src/main.c

@@ -0,0 +1,211 @@
+/**
+ * @file
+ * @brief Unit test for ReadRange services encode and decode
+ * @author Steve Karg <skarg@users.sourceforge.net>
+ * @date April 2025
+ * @copyright SPDX-License-Identifier: MIT
+ */
+#include <zephyr/ztest.h>
+#include <bacnet/bacdcode.h>
+#include <bacnet/readrange.h>
+
+/**
+ * @addtogroup bacnet_tests
+ * @{
+ */
+
+static const char *read_range_request_type(int type)
+{
+    switch (type) {
+        case RR_BY_POSITION:
+            return "RR_BY_POSITION";
+        case RR_BY_SEQUENCE:
+            return "RR_BY_SEQUENCE";
+        case RR_BY_TIME:
+            return "RR_BY_TIME";
+        case RR_READ_ALL:
+            return "RR_READ_ALL";
+        default:
+            return "UNKNOWN";
+    }
+}
+
+/**
+ * @brief Test
+ */
+static void testReadRangeAckUnit(BACNET_READ_RANGE_DATA *data)
+{
+    uint8_t apdu[480] = { 0 };
+    uint8_t apdu2[480] = { 0 };
+    int apdu_len = 0, test_len = 0, null_len = 0;
+    BACNET_READ_RANGE_DATA test_data = { 0 };
+    BACNET_OBJECT_TYPE object_type = OBJECT_DEVICE;
+    uint32_t object_instance = 0;
+    BACNET_OBJECT_TYPE object = 0;
+
+    data->application_data_len = encode_bacnet_object_id(
+        &apdu2[0], data->object_type, data->object_instance);
+    data->application_data = &apdu2[0];
+
+    null_len = readrange_ack_service_encode(&apdu[0], sizeof(apdu), NULL);
+    zassert_equal(null_len, 0, NULL);
+    null_len = readrange_ack_service_encode(&apdu[0], 0, NULL);
+    zassert_equal(null_len, 0, NULL);
+    null_len = readrange_ack_service_encode(NULL, sizeof(apdu), data);
+    zassert_not_equal(null_len, 0, NULL);
+    apdu_len = readrange_ack_service_encode(&apdu[0], sizeof(apdu), data);
+    zassert_equal(
+        apdu_len, null_len, "apdu_len=%d null_len=%d", apdu_len, null_len);
+    zassert_not_equal(apdu_len, 0, NULL);
+    zassert_not_equal(apdu_len, BACNET_STATUS_ERROR, NULL);
+    null_len = rr_ack_decode_service_request(NULL, apdu_len, &test_data);
+    zassert_true(null_len < 0, NULL);
+    test_len = rr_ack_decode_service_request(&apdu[0], apdu_len, &test_data);
+    zassert_equal(apdu_len, test_len, NULL);
+    zassert_not_equal(test_len, -1, NULL);
+
+    zassert_equal(test_data.object_type, data->object_type, NULL);
+    zassert_equal(test_data.object_instance, data->object_instance, NULL);
+    zassert_equal(test_data.object_property, data->object_property, NULL);
+    zassert_equal(test_data.array_index, data->array_index, NULL);
+    zassert_equal(
+        test_data.application_data_len, data->application_data_len,
+        "test app len=%d app len=%d", test_data.application_data_len,
+        data->application_data_len);
+
+    /* since object property == object_id, decode the application data using
+       the appropriate decode function */
+    test_len =
+        decode_object_id(test_data.application_data, &object, &object_instance);
+    object_type = object;
+    zassert_equal(object_type, data->object_type, NULL);
+    zassert_equal(object_instance, data->object_instance, NULL);
+    while (apdu_len) {
+        apdu_len--;
+        if (apdu_len == 17) {
+            /* boundary of optional parameters, so becomes valid */
+            continue;
+        }
+        test_len =
+            rr_ack_decode_service_request(&apdu[0], apdu_len, &test_data);
+        zassert_true(
+            test_len < 0, "test_len=%d apdu_len=%d", test_len, apdu_len);
+    }
+}
+
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST(rp_tests, testReadRangeAck)
+#else
+static void testReadRangeAck(void)
+#endif
+{
+    BACNET_READ_RANGE_DATA data = { 0 };
+
+    data.object_type = OBJECT_DEVICE;
+    data.object_instance = 1;
+    data.object_property = PROP_OBJECT_IDENTIFIER;
+    data.array_index = 0;
+    data.RequestType = RR_READ_ALL;
+    testReadRangeAckUnit(&data);
+    data.array_index = BACNET_ARRAY_ALL;
+    for (int i = 0; i < 3; i++) {
+        data.ItemCount = i;
+        data.RequestType = RR_READ_ALL;
+        testReadRangeAckUnit(&data);
+        /*  firstSequenceNumber - used only if 'Item Count' > 0 and
+            the request was either of type 'By Sequence Number' or 'By Time' */
+        for (int j = 0; j < 3; j++) {
+            data.FirstSequence = j;
+            data.RequestType = RR_BY_TIME;
+            testReadRangeAckUnit(&data);
+            data.RequestType = RR_BY_SEQUENCE;
+            testReadRangeAckUnit(&data);
+        }
+        data.FirstSequence = 0;
+        data.RequestType = RR_BY_POSITION;
+        testReadRangeAckUnit(&data);
+    }
+}
+
+static void testReadRangeUnit(BACNET_READ_RANGE_DATA *data)
+{
+    uint8_t apdu[480] = { 0 };
+    int apdu_len = 0, test_len = 0, null_len = 0;
+    BACNET_READ_RANGE_DATA test_data;
+
+    null_len = read_range_request_encode(&apdu[0], 0, data);
+    zassert_equal(null_len, 0, NULL);
+    null_len = read_range_request_encode(&apdu[0], sizeof(apdu), NULL);
+    zassert_equal(null_len, 0, NULL);
+    null_len = read_range_request_encode(NULL, sizeof(apdu), data);
+    zassert_not_equal(null_len, 0, NULL);
+    apdu_len = read_range_request_encode(&apdu[0], sizeof(apdu), data);
+    zassert_equal(
+        apdu_len, null_len, "apdu_len=%d null_len=%d", apdu_len, null_len);
+    zassert_not_equal(apdu_len, 0, NULL);
+    null_len = rr_decode_service_request(NULL, apdu_len, &test_data);
+    zassert_true(null_len < 0, NULL);
+    test_len = rr_decode_service_request(&apdu[0], apdu_len, &test_data);
+    zassert_equal(
+        apdu_len, test_len, "apdu_len=%d test_len=%d", apdu_len, test_len);
+    zassert_not_equal(test_len, -1, NULL);
+    zassert_equal(test_data.object_type, data->object_type, NULL);
+    zassert_equal(test_data.object_instance, data->object_instance, NULL);
+    zassert_equal(test_data.object_property, data->object_property, NULL);
+    zassert_equal(test_data.array_index, data->array_index, NULL);
+    while (apdu_len) {
+        apdu_len--;
+        test_len = rr_decode_service_request(&apdu[0], apdu_len, &test_data);
+        if (apdu_len == 7) {
+            /* boundary of optional parameters, so becomes valid */
+            continue;
+        }
+        zassert_true(
+            test_len < 0, "test_len=%d apdu_len=%d request=%s array=%u",
+            test_len, apdu_len, read_range_request_type(data->RequestType),
+            data->array_index);
+    }
+}
+
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST(rp_tests, testReadRange)
+#else
+static void testReadRange(void)
+#endif
+{
+    BACNET_READ_RANGE_DATA data = { 0 };
+
+    data.object_type = OBJECT_DEVICE;
+    data.object_instance = 1;
+    data.object_property = PROP_OBJECT_IDENTIFIER;
+    data.array_index = 0;
+    data.RequestType = RR_READ_ALL;
+    testReadRangeUnit(&data);
+    data.array_index = BACNET_ARRAY_ALL;
+    data.RequestType = RR_READ_ALL;
+    testReadRangeUnit(&data);
+    data.RequestType = RR_BY_POSITION;
+    testReadRangeUnit(&data);
+    data.RequestType = RR_BY_SEQUENCE;
+    testReadRangeUnit(&data);
+    data.RequestType = RR_BY_TIME;
+    testReadRangeUnit(&data);
+
+    return;
+}
+/**
+ * @}
+ */
+
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST_SUITE(rp_tests, NULL, NULL, NULL, NULL, NULL);
+#else
+void test_main(void)
+{
+    ztest_test_suite(
+        readrange_tests, ztest_unit_test(testReadRange),
+        ztest_unit_test(testReadRangeAck));
+
+    ztest_run_test_suite(readrange_tests);
+}
+#endif