Bugfix/read-range-address-list-encode (#1149)

* Fix: Corrected `rr_address_list_encode` to properly handle the end of the address cache and added a new test case to validate ReadRange operations near the cache limit.
This commit is contained in:
Steve Karg
2026-03-12 09:05:04 -05:00
committed by GitHub
parent 07bfc7c61c
commit f525e7c484
3 changed files with 94 additions and 16 deletions
+12 -11
View File
@@ -9,8 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
The git repositories are hosted at the following sites: The git repositories are hosted at the following sites:
* https://bacnet.sourceforge.net/
* https://github.com/bacnet-stack/bacnet-stack/ * <https://bacnet.sourceforge.net/>
* <https://github.com/bacnet-stack/bacnet-stack/>
## [Unreleased] - 2026-03-06 ## [Unreleased] - 2026-03-06
@@ -190,6 +191,7 @@ The git repositories are hosted at the following sites:
### Fixed ### Fixed
* Fixed the ReadRange-ACK of the Address_List property. (#1149)
* Fixed ReadRangeACK by sequence in Trend Log object. (#1150) * Fixed ReadRangeACK by sequence in Trend Log object. (#1150)
* Fixed Device Management-Backup and Restore-B functionality to keep * Fixed Device Management-Backup and Restore-B functionality to keep
configuration files during the restore operation. (#1250) configuration files during the restore operation. (#1250)
@@ -527,6 +529,7 @@ The git repositories are hosted at the following sites:
## [1.4.1] - 2025-04-11 ## [1.4.1] - 2025-04-11
### Security ### Security
* Secured ReadRange service codecs. Added ReadRange unit testing. * Secured ReadRange service codecs. Added ReadRange unit testing.
Secured ReadRange-ACK handler to enable APDU size checking. (#957) Secured ReadRange-ACK handler to enable APDU size checking. (#957)
* Secured BACnet/SC URL handling by changing all the sprintf * Secured BACnet/SC URL handling by changing all the sprintf
@@ -614,7 +617,7 @@ The git repositories are hosted at the following sites:
handlers. (#908) handlers. (#908)
* Fixed multi-state-input and multi-state-value basic objects usage of the * Fixed multi-state-input and multi-state-value basic objects usage of the
Write_Enabled flag by adding an API to get/set the flag. (#903) Write_Enabled flag by adding an API to get/set the flag. (#903)
* Fixed usage of 8-bit modulo operator off-by-one maximums. (#901) * Fixed usage of 8-bit modulo operator off-by-one maximums. (#901)
* Fixed legacy make build recipe for library. 'make library' now builds. * Fixed legacy make build recipe for library. 'make library' now builds.
* Fixed IPv6 to leave multicast when registering as foreign device. (#899) * Fixed IPv6 to leave multicast when registering as foreign device. (#899)
* Fixed IPv6 handler to ignore original-broadcast when registered as * Fixed IPv6 handler to ignore original-broadcast when registered as
@@ -765,7 +768,7 @@ The git repositories are hosted at the following sites:
* Changed the datalink abstraction to enable selecting multiple datalinks * Changed the datalink abstraction to enable selecting multiple datalinks
using BACDL_MULTIPLE and one or more other BACDL defines. (#717) using BACDL_MULTIPLE and one or more other BACDL defines. (#717)
* Moved west manifest, zephyr folder, and ports/zephyr folders to * Moved west manifest, zephyr folder, and ports/zephyr folders to
another repository https://github.com/bacnet-stack/bacnet-stack-zephyr another repository <https://github.com/bacnet-stack/bacnet-stack-zephyr>
so that the rapid pace of Zephyr OS development changes will have so that the rapid pace of Zephyr OS development changes will have
a less impact on the development of the BACnet Stack library. (#757) a less impact on the development of the BACnet Stack library. (#757)
* Removed static scope on character array used for object-name since the array * Removed static scope on character array used for object-name since the array
@@ -954,7 +957,6 @@ The git repositories are hosted at the following sites:
bacapp_decode_context_data_len() as they are no longer used in any code bacapp_decode_context_data_len() as they are no longer used in any code
in the library.(#702) in the library.(#702)
## [1.3.7] - 2024-06-26 ## [1.3.7] - 2024-06-26
### Security ### Security
@@ -1026,7 +1028,6 @@ The git repositories are hosted at the following sites:
priority 6. (#640) priority 6. (#640)
* Fixed basic analog-value alarm-ack functionality. (#639) * Fixed basic analog-value alarm-ack functionality. (#639)
### Removed ### Removed
* Removed local dlmstp.c module from stm32f10x port in favor of using * Removed local dlmstp.c module from stm32f10x port in favor of using
@@ -1489,7 +1490,7 @@ tags. (#491)
### Added ### Added
* Added minimim support for BACnet protocol-revision 0 through 24. See  * Added minimim support for BACnet protocol-revision 0 through 24. See
BACNET_PROTOCOL_REVISION define in bacdef.h BACNET_PROTOCOL_REVISION define in bacdef.h
* Added example objects for color and color temperature, new for protocol-revision 24. * Added example objects for color and color temperature, new for protocol-revision 24.
* Added current-command-priority to output objects revision 17 or later. * Added current-command-priority to output objects revision 17 or later.
@@ -1515,20 +1516,20 @@ these objects: file, analog output, binary output, multi-state output, color,
color temperature). color temperature).
* Unit tests and functional tests have been moved from the source C files * Unit tests and functional tests have been moved from the source C files
into their own test files under the test folder in a src/ mirrored folder into their own test files under the test folder in a src/ mirrored folder
structure under test/.  structure under test/.
* The testing framework was moved from ctest to ztest, using CMake, CTest, and LCOV.  * The testing framework was moved from ctest to ztest, using CMake, CTest, and LCOV.
It's now possible to visually view code coverage from the testing of each file It's now possible to visually view code coverage from the testing of each file
in the library. The tests are run in continuous integration.  Adding new tests in the library. The tests are run in continuous integration.  Adding new tests
can be done by as copying an existing folder under test/ as a starting point, and can be done by as copying an existing folder under test/ as a starting point, and
adding the new test folder name into the CMakeLists.txt under test/ folder, or adding the new test folder name into the CMakeLists.txt under test/ folder, or
editing the existing test C file and extending or fixing the existing test.  editing the existing test C file and extending or fixing the existing test.
* Most (all?) of the primitive value encoders are now using a snprintf() * Most (all?) of the primitive value encoders are now using a snprintf()
style API pattern, where passing NULL in the buffer parameter will style API pattern, where passing NULL in the buffer parameter will
return the length that the buffer would use (i.e. returns the length that the return the length that the buffer would use (i.e. returns the length that the
buffer needs to be).  This makes simple to write the parent encoders to check buffer needs to be).  This makes simple to write the parent encoders to check
the length versus their buffer before attempting the encoding at the expense of the length versus their buffer before attempting the encoding at the expense of
an extra function call to get the length. an extra function call to get the length.
* BACnetARRAY property types now have a helper function to use for encoding (see bacnet_array_encode)  * BACnetARRAY property types now have a helper function to use for encoding (see bacnet_array_encode)
## Deprecated ## Deprecated
+13 -3
View File
@@ -987,16 +987,26 @@ int rr_address_list_encode(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest)
/* Chalk up another one for the response count */ /* Chalk up another one for the response count */
pRequest->ItemCount++; pRequest->ItemCount++;
if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
/* valid entry at the end of the table */
uiLast = uiTotal;
break;
}
while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) != while ((pMatch->Flags & (BAC_ADDR_IN_USE | BAC_ADDR_BIND_REQ)) !=
BAC_ADDR_IN_USE) { BAC_ADDR_IN_USE) {
/* Find next bound entry */ /* Find next bound entry */
pMatch++; pMatch++;
/* Can normally not happen. */
if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) { if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
/* Issue with the table. */ /* valid entry at the end of the table */
return (0); uiLast = uiTotal;
break;
} }
} }
if (pMatch > &Address_Cache[MAX_ADDRESS_CACHE - 1]) {
/* valid entry at the end of the table */
uiLast = uiTotal;
break;
}
} }
/* Set remaining result flags if necessary */ /* Set remaining result flags if necessary */
if (uiFirst == 1) { if (uiFirst == 1) {
+69 -2
View File
@@ -142,6 +142,71 @@ static void testAddressFile(void)
} }
#endif #endif
#if defined(CONFIG_ZTEST_NEW_API)
ZTEST(address_tests, test_rr_address)
#else
static void test_rr_address(void)
#endif
{
uint8_t apdu[MAX_APDU];
BACNET_READ_RANGE_DATA pRequest = { 0 };
BACNET_ADDRESS src = { 0 };
unsigned i;
int len;
address_init();
/* Fill the cache to its limit */
for (i = 0; i < MAX_ADDRESS_CACHE; i++) {
src.mac_len = 1;
src.mac[0] = (uint8_t)i;
address_add(i, 480, &src);
}
/* Test ReadRange ALL */
pRequest.object_type = OBJECT_DEVICE;
pRequest.object_instance = 123;
pRequest.object_property = PROP_DEVICE_ADDRESS_BINDING;
pRequest.array_index = BACNET_ARRAY_ALL;
pRequest.RequestType = RR_READ_ALL;
pRequest.Overhead = 0;
len = rr_address_list_encode(apdu, &pRequest);
zassert_not_equal(len, 0, "ReadRange ALL failed - returned 0");
zassert_true(
bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_MORE_ITEMS),
"Expected MORE_ITEMS flag");
zassert_true(
bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_FIRST_ITEM),
"Expected FIRST_ITEM flag");
/* Test ReadRange from the end to verify the fix */
pRequest.RequestType = RR_BY_POSITION;
pRequest.Range.RefIndex = MAX_ADDRESS_CACHE - 5 + 1;
pRequest.Count = 5;
bitstring_init(&pRequest.ResultFlags);
len = rr_address_list_encode(apdu, &pRequest);
zassert_not_equal(len, 0, "ReadRange end failed - returned 0");
zassert_equal(pRequest.ItemCount, 5, "Expected 5 items");
zassert_true(
bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_LAST_ITEM),
"Expected LAST_ITEM flag");
/* Specifically test hitting the very last physical entry in Address_Cache
*/
pRequest.RequestType = RR_BY_POSITION;
pRequest.Range.RefIndex = MAX_ADDRESS_CACHE;
pRequest.Count = 1;
bitstring_init(&pRequest.ResultFlags);
len = rr_address_list_encode(apdu, &pRequest);
zassert_not_equal(len, 0, "ReadRange last item failed - returned 0");
zassert_equal(pRequest.ItemCount, 1, "Expected 1 item");
zassert_true(
bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_LAST_ITEM),
"Expected LAST_ITEM flag for the last item");
}
#if defined(CONFIG_ZTEST_NEW_API) #if defined(CONFIG_ZTEST_NEW_API)
ZTEST(address_tests, testAddress) ZTEST(address_tests, testAddress)
#else #else
@@ -209,11 +274,13 @@ void test_main(void)
#ifdef BACNET_ADDRESS_CACHE_FILE #ifdef BACNET_ADDRESS_CACHE_FILE
ztest_test_suite( ztest_test_suite(
address_tests, ztest_unit_test(testAddressFile), address_tests, ztest_unit_test(testAddressFile),
ztest_unit_test(testAddress)); ztest_unit_test(testAddress), ztest_unit_test(test_rr_address));
ztest_run_test_suite(address_tests); ztest_run_test_suite(address_tests);
#else #else
ztest_test_suite(address_tests, ztest_unit_test(testAddress)); ztest_test_suite(
address_tests, ztest_unit_test(testAddress),
ztest_unit_test(test_rr_address));
ztest_run_test_suite(address_tests); ztest_run_test_suite(address_tests);
#endif #endif