feat: add TSDB support for high granularity Flash operations by Oliver0927i · Pull Request #398 · armink/FlashDB

Oliver0927i · 2026-03-12T03:27:08Z

No description provided.

armink · 2026-03-12T03:30:29Z

有些冲突需要先解决下哈

Oliver0927i · 2026-03-12T03:40:22Z

已解决

Copilot

Pull request overview

This PR adds TSDB (Time Series Database) support for 64-bit and 128-bit Flash write granularity, which was previously explicitly unsupported with a compile-time error. The approach mirrors changes previously made to KVDB: struct fields are converted from native types to byte arrays aligned to write granularity, memcpy is used for reading/writing these fields, and an align_write helper is introduced for blob data.

Changes:

Removes the compile-time error blocking 64/128-bit write granularity in TSDB and adjusts sector_hdr_data and log_idx_data structs with byte-array fields and padding for alignment.
Adds an align_write function (mirroring the KVDB equivalent) to handle write-granularity-aligned Flash writes for blob data.
Updates read_sector_info, format_sector, write_tsl, and update_sec_status to use memcpy for field access and aligned writes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

src/fdb_tsdb.c

Copilot · 2026-03-12T06:41:47Z

src/fdb_tsdb.c

+#endif
+
+    memset(align_data, FDB_BYTE_ERASED, align_data_size);
+    result = _fdb_flash_write((fdb_db_t) db, addr, buf, FDB_WG_ALIGN_DOWN(size), false);


The KVDB version of align_write (in fdb_kvdb.c:1195) guards the first _fdb_flash_write call with if(FDB_WG_ALIGN_DOWN(size) > 0), but this TSDB version unconditionally calls _fdb_flash_write with FDB_WG_ALIGN_DOWN(size) which could be 0 when size is smaller than the write granularity. This should include the same guard to avoid writing 0 bytes.

@Oliver0927i 这个可以参考下 AI 的建议

Copilot · 2026-03-12T06:41:47Z

src/fdb_tsdb.c

+    uint8_t magic[TSL_UINT32_ALIGN_SIZE];        /**< magic word(`T`, `S`, `L`, `0`) */
+    uint8_t start_time[TSL_UINT32_ALIGN_SIZE];   /**< the first start node's timestamp */


Critical bug: TSL_UINT32_ALIGN_SIZE is FDB_WG_ALIGN(sizeof(uint32_t)) which is 4 bytes for FDB_WRITE_GRAN values of 1, 8, or 32. However, fdb_time_t can be int64_t (8 bytes) when FDB_USING_TIMESTAMP_64BIT is defined. The memcpy calls that copy sizeof(fdb_time_t) bytes from/to these uint8_t[TSL_UINT32_ALIGN_SIZE] arrays (e.g., sec_hdr.start_time, sec_hdr.end_info[i].time) will overflow the buffer and corrupt adjacent struct members. The time fields should use FDB_WG_ALIGN(sizeof(fdb_time_t)) instead of TSL_UINT32_ALIGN_SIZE.

@Oliver0927i 这个也可以参考下 AI 的建议

Copilot · 2026-03-12T06:41:47Z

src/fdb_tsdb.c

+#if (FDB_WRITE_GRAN == 64) || (FDB_WRITE_GRAN == 128)
+    uint8_t padding[4];                          /**< align padding for 64bit write granularity */
 #endif


The padding comment says "align padding for 64bit write granularity" but this same padding is also applied for FDB_WRITE_GRAN == 128. The comment is misleading for the 128-bit case. Additionally, for 128-bit write granularity, 4 bytes of padding may not be sufficient — you should verify that the total struct size after padding is correctly aligned to FDB_WRITE_GRAN/8 (16 bytes for 128-bit).

Suggested change

#if (FDB_WRITE_GRAN == 64) || (FDB_WRITE_GRAN == 128)

uint8_t padding[4]; /**< align padding for 64bit write granularity */

#endif

#if (FDB_WRITE_GRAN == 64)

uint8_t padding[4]; /**< align padding for 64bit write granularity */

#endif

#if (FDB_WRITE_GRAN == 128)

uint8_t padding[12]; /**< align padding for 128bit write granularity */

#endif

#endif /* FDB_TSDB_FIXED_BLOB_SIZE */

@Oliver0927i 这个可以参考下 AI 的建议

Copilot · 2026-03-12T06:41:47Z

src/fdb_tsdb.c

    FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time,  sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);
    /* write blob data */
-    FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);
+    result = align_write(db, idx.log_addr, blob->buf, blob->size);


Bug: When FDB_TSDB_FIXED_BLOB_SIZE is defined, the log_idx_data struct does not have a log_addr member (it's inside #ifndef FDB_TSDB_FIXED_BLOB_SIZE), so idx.log_addr on this line will cause a compilation error. The original code correctly used the local variable log_addr (declared on line 357) instead of idx.log_addr. This should be reverted to use log_addr.

Suggested change

result = align_write(db, idx.log_addr, blob->buf, blob->size);

result = align_write(db, log_addr, blob->buf, blob->size);

Copilot · 2026-03-12T06:41:48Z

src/fdb_tsdb.c

+static fdb_err_t align_write(fdb_tsdb_t db, uint32_t addr, const uint32_t *buf, size_t size)
+{
+    fdb_err_t result = FDB_NO_ERR;
+    size_t align_remain;
+
+#if (FDB_WRITE_GRAN / 8 > 0)
+    uint8_t align_data[FDB_WRITE_GRAN / 8];
+    size_t align_data_size = sizeof(align_data);
+#else
+    /* For compatibility with C89 */
+    uint8_t align_data_u8, *align_data = &align_data_u8;
+    size_t align_data_size = 1;
+#endif
+
+    memset(align_data, FDB_BYTE_ERASED, align_data_size);
+    result = _fdb_flash_write((fdb_db_t) db, addr, buf, FDB_WG_ALIGN_DOWN(size), false);
+
+    align_remain = size - FDB_WG_ALIGN_DOWN(size);
+    if (result == FDB_NO_ERR && align_remain) {
+        memcpy(align_data, (uint8_t *) buf + FDB_WG_ALIGN_DOWN(size), align_remain);
+        result = _fdb_flash_write((fdb_db_t) db, addr + FDB_WG_ALIGN_DOWN(size), (uint32_t *) align_data,
+                align_data_size, false);
+    }
+
+    return result;
+}


This align_write function is nearly identical to the one in fdb_kvdb.c:1180. Consider extracting it into a shared utility (e.g., in fdb_utils.c) to avoid code duplication and ensure bug fixes are applied consistently to both.

@Oliver0927i 这个完全赞同 AI 的建议

Copilot · 2026-03-12T06:41:48Z

src/fdb_tsdb.c

+#if (FDB_WRITE_GRAN == 64)
+    uint8_t padding[4];                          /**< align padding for 64bit write granularity */
+#endif
+#if (FDB_WRITE_GRAN == 128)
+    uint8_t padding[12];                         /**< align padding for 128bit write granularity */
+#endif


Both #if (FDB_WRITE_GRAN == 64) and #if (FDB_WRITE_GRAN == 128) declare uint8_t padding[...] with the same member name. While they are mutually exclusive preprocessor branches, some compilers may issue a warning about redeclaration. This can be safely combined into a single #if (FDB_WRITE_GRAN == 64) || (FDB_WRITE_GRAN == 128) block using a computed size, similar to how the log_idx_data padding is done.

Suggested change

#if (FDB_WRITE_GRAN == 64)

uint8_t padding[4]; /**< align padding for 64bit write granularity */

#endif

#if (FDB_WRITE_GRAN == 128)

uint8_t padding[12]; /**< align padding for 128bit write granularity */

#endif

#if (FDB_WRITE_GRAN == 64) || (FDB_WRITE_GRAN == 128)

uint8_t padding[(FDB_WRITE_GRAN / 8) - 4]; /**< align padding for 64bit or 128bit write granularity */

#endif

armink · 2026-03-12T06:42:37Z

CI 里面的自动化测试用例没有通过，看着是编译都直接报错了，可以找下原因

Oliver0927i · 2026-03-13T02:18:22Z

@armink 已经更新了，现在CI 能跑过~

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

src/fdb_tsdb.c

+            sizeof(uint8_t) * TSL_STATUS_TABLE_SIZE
+            + sizeof(fdb_time_t)
+#ifndef FDB_TSDB_FIXED_BLOB_SIZE
+            + sizeof(uint32_t) * 2
+#endif
+        ) -
+        (
+            sizeof(uint8_t) * TSL_STATUS_TABLE_SIZE
+            + sizeof(fdb_time_t)
+#ifndef FDB_TSDB_FIXED_BLOB_SIZE
+            + sizeof(uint32_t) * 2
+#endif
+        )


src/fdb_tsdb.c

+#endif
+
+    memset(align_data, FDB_BYTE_ERASED, align_data_size);
+    result = _fdb_flash_write((fdb_db_t) db, addr, buf, FDB_WG_ALIGN_DOWN(size), false);


src/fdb_tsdb.c

    uint32_t reserved;
+
+    // Autofill to the FDB WRITE GRAN alignment
+    uint8_t padding[FDB_WG_ALIGN(sizeof(uint32_t))- sizeof(uint32_t)];


src/fdb_tsdb.c

 struct log_idx_data {
    uint8_t status_table[TSL_STATUS_TABLE_SIZE]; /**< node status, @see fdb_tsl_status_t */
    fdb_time_t time;                             /**< node timestamp */
 #ifndef FDB_TSDB_FIXED_BLOB_SIZE
    uint32_t log_len;                            /**< node total length (header + name + value), must align by FDB_WRITE_GRAN */
    uint32_t log_addr;                           /**< node address */
 #endif


fix: extract align_write function

Oliver0927i · 2026-03-13T08:33:12Z

@armink 已按照AI的建议修改~

feat: add TSDB support for high granularity Flash operations

7e954f4

Oliver0927i mentioned this pull request Mar 12, 2026

[功能建议] 请求支持 FDB_WRITE_GRAN 配置为 128 (最小16字节颗粒度)，适配 STM32H5 等新平台 #397

Open

Merge branch 'master' into feat/tsdb_support_high_granularity

1a06d1f

armink requested a review from Copilot March 12, 2026 06:37

Copilot started reviewing on behalf of armink March 12, 2026 06:38 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Oliver0927i added 4 commits March 12, 2026 17:08

Adjust timestamp size based on configuration

f5eef9c

fix: set log_idx_data Autofill to the FDB WRITE GRAN alignment

1ff9855

fix: set struct sector_hdr_data Autofill to the FDB WRITE GRAN alignment

519b831

fix: set Autofill to the FDB WRITE GRAN alignment

3c22743

armink requested a review from Copilot March 13, 2026 06:12

Copilot started reviewing on behalf of armink March 13, 2026 06:12 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

Oliver0927i added 2 commits March 13, 2026 16:14

fix: extract align_write function

eec16cc

Merge pull request #1 from Oliver0927i/feat/test

a828394

fix: extract align_write function

		uint8_t magic[TSL_UINT32_ALIGN_SIZE]; /*< magic word(`T`, `S`, `L`, `0`) /
		uint8_t start_time[TSL_UINT32_ALIGN_SIZE]; /*< the first start node's timestamp /

	result = align_write(db, idx.log_addr, blob->buf, blob->size);
	result = align_write(db, log_addr, blob->buf, blob->size);

Conversation

Oliver0927i commented Mar 12, 2026

Uh oh!

armink commented Mar 12, 2026

Uh oh!

Oliver0927i commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

armink Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

armink Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

armink Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

armink Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

armink commented Mar 12, 2026

Uh oh!

Oliver0927i commented Mar 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Oliver0927i commented Mar 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants