diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ecf9816..6bbf37fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -190,6 +190,7 @@ The git repositories are hosted at the following sites: ### Fixed +* Fixed ReadRangeACK by sequence in Trend Log object. (#1150) * Fixed Device Management-Backup and Restore-B functionality to keep configuration files during the restore operation. (#1250) * Fixed Device Management-Backup and Restore-B Backup_Failure_Timeout diff --git a/src/bacnet/basic/object/trendlog.c b/src/bacnet/basic/object/trendlog.c index f3f15d54..72e7b540 100644 --- a/src/bacnet/basic/object/trendlog.c +++ b/src/bacnet/basic/object/trendlog.c @@ -298,6 +298,48 @@ bool Trend_Log_Object_Name( return status; } +/** + * @brief Get the total record count for a Trend Log object + * @param object_instance - object-instance number of the object + * @return total record count + */ +uint32_t Trend_Log_Total_Record_Count(uint32_t object_instance) +{ + uint32_t total_records = 0; + if (object_instance < MAX_TREND_LOGS) { + total_records = LogInfo[object_instance].ulTotalRecordCount; + } + return total_records; +} + +/** + * @brief Get the record count for a Trend Log object + * @param object_instance - object-instance number of the object + * @return record count + */ +uint32_t Trend_Log_Record_Count(uint32_t object_instance) +{ + uint32_t record_count = 0; + if (object_instance < MAX_TREND_LOGS) { + record_count = LogInfo[object_instance].ulRecordCount; + } + return record_count; +} + +/** + * @brief Get the buffer size for a Trend Log object + * @param object_instance - object-instance number of the object + * @return buffer size + */ +uint32_t Trend_Log_Buffer_Size(uint32_t object_instance) +{ + uint32_t buffer_size = 0; + if (object_instance < MAX_TREND_LOGS) { + buffer_size = TL_MAX_ENTRIES; + } + return buffer_size; +} + /* return the length of the apdu encoded or BACNET_STATUS_ERROR for error or BACNET_STATUS_ABORT for abort message */ int Trend_Log_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata) @@ -1176,7 +1218,8 @@ int TL_encode_by_sequence(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) uint32_t uiSequence = 0; /* Tracking sequence number when encoding */ uint32_t uiRemaining = 0; /* Amount of unused space in packet */ uint32_t uiFirstSeq = 0; /* Sequence number for 1st record in log */ - + uint32_t total_entries = 0; + uint32_t max_fit = 0; uint32_t uiBegin = 0; /* Starting Sequence number for request */ uint32_t uiEnd = 0; /* Ending Sequence number for request */ bool bWrapReq = false; /* Has request sequence range spanned the max for @@ -1267,7 +1310,22 @@ int TL_encode_by_sequence(uint8_t *apdu, BACNET_READ_RANGE_DATA *pRequest) } } } - + if (pRequest->Count < 0) { + /* adjust uiBegin when Count < 0 and total requested + items exceed the maximum encodable items (max_fit).*/ + if (uiEnd >= uiBegin) { + total_entries = uiEnd - uiBegin + 1; + max_fit = uiRemaining / TL_MAX_ENC; + if ((max_fit > 0) && (total_entries > max_fit)) { + /* Adjust beginning index so returned items match z = max_fit */ + uiBegin = uiEnd - max_fit + 1; + /* MORE_ITEMS must be set because + request range not fully delivered */ + bitstring_set_bit( + &pRequest->ResultFlags, RESULT_FLAG_MORE_ITEMS, true); + } + } + } /* We now have a range that lies completely within the log buffer * and we need to figure out where that starts in the buffer. */ diff --git a/src/bacnet/basic/object/trendlog.h b/src/bacnet/basic/object/trendlog.h index 2a8fef3b..9e39fee8 100644 --- a/src/bacnet/basic/object/trendlog.h +++ b/src/bacnet/basic/object/trendlog.h @@ -136,6 +136,13 @@ BACNET_STACK_EXPORT bool Trend_Log_Object_Name( uint32_t object_instance, BACNET_CHARACTER_STRING *object_name); +BACNET_STACK_EXPORT +uint32_t Trend_Log_Total_Record_Count(uint32_t object_instance); +BACNET_STACK_EXPORT +uint32_t Trend_Log_Record_Count(uint32_t object_instance); +BACNET_STACK_EXPORT +uint32_t Trend_Log_Buffer_Size(uint32_t object_instance); + BACNET_STACK_EXPORT int Trend_Log_Read_Property(BACNET_READ_PROPERTY_DATA *rpdata); diff --git a/test/bacnet/basic/object/trendlog/src/main.c b/test/bacnet/basic/object/trendlog/src/main.c index 15b01f5b..7115cc46 100644 --- a/test/bacnet/basic/object/trendlog/src/main.c +++ b/test/bacnet/basic/object/trendlog/src/main.c @@ -37,6 +37,109 @@ static void test_Trend_Log_ReadProperty(void) Trend_Log_Read_Property, Trend_Log_Write_Property, known_fail_property_list); } + +static void test_Trend_Log_ReadRange_BySequence(void) +{ + uint8_t apdu[MAX_APDU] = { 0 }; + BACNET_READ_RANGE_DATA pRequest = { 0 }; + int len = 0; + uint32_t object_instance = 0; + int log_index = 0; + uint32_t i; + uint32_t total_records = 0; + uint32_t first_seq = 0; + + Trend_Log_Init(); + object_instance = Trend_Log_Index_To_Instance(0); + log_index = Trend_Log_Instance_To_Index(object_instance); + + /* Populate the log with some entries to make sure it's not empty and we + * know the state */ + for (i = 0; i < 100; i++) { + TL_Insert_Status_Rec(log_index, LOG_STATUS_LOG_INTERRUPTED, true); + } + + /* Get the current state of the log */ + total_records = Trend_Log_Total_Record_Count(object_instance); + zassert_true(total_records > 0, "Failed to get total record count"); + /* The first sequence number currently in the log buffer */ + first_seq = total_records - Trend_Log_Record_Count(object_instance) + 1; + zassert_true(first_seq > 0, "Failed to get first sequence number"); + + /* Verify BY_SEQUENCE with positive count within range */ + pRequest.object_type = OBJECT_TRENDLOG; + pRequest.object_instance = object_instance; + pRequest.object_property = PROP_LOG_BUFFER; + pRequest.array_index = BACNET_ARRAY_ALL; + pRequest.RequestType = RR_BY_SEQUENCE; + pRequest.Range.RefSeqNum = first_seq; + pRequest.Count = 10; + pRequest.Overhead = 0; + + len = rr_trend_log_encode(apdu, &pRequest); + zassert_true(len > 0, "Encoding failed for positive count"); + zassert_equal(pRequest.ItemCount, 10, "Expected 10 items"); + zassert_equal( + pRequest.FirstSequence, first_seq, "Expected FirstSequence %u, got %u", + (unsigned)first_seq, (unsigned)pRequest.FirstSequence); + + /* Verify BY_SEQUENCE with positive count and truncation at end of log */ + pRequest.Range.RefSeqNum = total_records - 5; + pRequest.Count = 10; + len = rr_trend_log_encode(apdu, &pRequest); + zassert_true(len > 0, "Encoding failed"); + zassert_equal(pRequest.ItemCount, 6, "Expected 6 items (truncated at end)"); + zassert_equal( + pRequest.FirstSequence, total_records - 5, + "Expected FirstSequence %u, got %u", (unsigned)(total_records - 5), + (unsigned)pRequest.FirstSequence); + zassert_true( + bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_LAST_ITEM), + "Expected LAST_ITEM flag"); + + /* Verify BY_SEQUENCE with negative count and truncation at beginning of log + */ + pRequest.Range.RefSeqNum = first_seq + 5; + pRequest.Count = -10; + len = rr_trend_log_encode(apdu, &pRequest); + zassert_true(len > 0, "Encoding failed"); + zassert_equal( + pRequest.ItemCount, 6, "Expected 6 items (truncated at start)"); + zassert_equal( + pRequest.FirstSequence, first_seq, + "Expected FirstSequence %u (start of log), got %u", (unsigned)first_seq, + (unsigned)pRequest.FirstSequence); + zassert_true( + bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_FIRST_ITEM), + "Expected FIRST_ITEM flag"); + + /* Verify BY_SEQUENCE with negative count and APDU capacity truncation (THE + * FIX) */ + /* Requesting everything from the end. This should definitely trigger + * truncation. */ + pRequest.ItemCount = 0; + pRequest.Range.RefSeqNum = total_records; + pRequest.Count = + -(int32_t)total_records; /* Try to get more than will fit */ + + len = rr_trend_log_encode(apdu, &pRequest); + zassert_true(len > 0, "Encoding failed for negative count"); + zassert_true( + pRequest.ItemCount < total_records, "Expected truncated ItemCount"); + zassert_true( + bitstring_bit(&pRequest.ResultFlags, RESULT_FLAG_MORE_ITEMS), + "Expected MORE_ITEMS flag"); + + /* THE FIX: FirstSequence should now correctly reflect the shifted uiBegin. + uiBegin = uiEnd - max_fit + 1. + In the code, uiEnd is RefSeqNum (total_records). + So FirstSequence should be total_records - ItemCount + 1 */ + zassert_equal( + pRequest.FirstSequence, total_records - pRequest.ItemCount + 1, + "FirstSequence %u does not match calculated start %u", + (unsigned)pRequest.FirstSequence, + (unsigned)(total_records - pRequest.ItemCount + 1)); +} /** * @} */ @@ -44,7 +147,8 @@ static void test_Trend_Log_ReadProperty(void) void test_main(void) { ztest_test_suite( - trendlog_tests, ztest_unit_test(test_Trend_Log_ReadProperty)); + trendlog_tests, ztest_unit_test(test_Trend_Log_ReadProperty), + ztest_unit_test(test_Trend_Log_ReadRange_BySequence)); ztest_run_test_suite(trendlog_tests); }