feat - refactor nand flash as ring buffer by TuEmb · Pull Request #7216 · BasedHardware/omi

TuEmb · 2026-05-10T04:11:20Z

Core Idea

Offline audio is a sequence-based ring buffer, not a file list.
One logical record = 444 bytes:
- 0..3: UTC timestamp, uint32 BE
- 4..443: audio payload, 440 bytes
Reading does not acknowledge data.
Only CMD_RING_ADVANCE persists consumer progress.

GATT

Service: 30295780-4301-EABD-2904-2849ADFEAE43
Control char: 30295781-4301-EABD-2904-2849ADFEAE43
- properties: Write + Notify
- use this for commands and protocol notifications
Status char: 30295782-4301-EABD-2904-2849ADFEAE43
- properties: Read + Notify
- mainly used for cached status polling

Subscribe to notifications on the Control characteristic.

Status Char Read

Returns 16 bytes = 4 x uint32 LE:

0..3: used_bytes
4..7: unread_packets
8..11: free_bytes
12..15: rtc_valid

Commands

Write commands to the Control characteristic.

`0x10` `CMD_RING_INFO`

Request: [0x10]
Meaning: request current ring state

`0x11` `CMD_RING_READ`

Request: [0x11][start_seq: u64 BE]
Or: [0x11][start_seq: u64 BE][packet_count: u32 BE]
If packet_count is omitted or 0, firmware streams everything available from start_seq

`0x12` `CMD_RING_ADVANCE`

Request: [0x12][new_read_seq: u64 BE]
Meaning: durably commit consumer progress

`0x13` `CMD_RING_CLEAR`

Request: [0x13]
Meaning: clear the entire ring

`0x03` `CMD_STOP_SYNC`

Request: [0x03]
Meaning: stop current transfer only
Does not persist progress

Notifications

All notifications are sent on the Control characteristic.

`0x01` `NOTIFY_ACK`

Format: [0x01][status]

`0x02` `NOTIFY_INFO`

Format:
[0x02][read_seq: u64 BE][write_seq: u64 BE][capacity_packets: u32 BE][dropped_packets: u64 BE][packet_size: u16 BE]
Current packet_size = 444

`0x05` `NOTIFY_READ_BEGIN`

Format:
[0x05][transfer_start_seq: u64 BE][packet_count: u32 BE]

`0x03` `NOTIFY_DATA`

Format:
[0x03][raw_bytes...]
Important: these are BLE chunks, not guaranteed to align to 444-byte records
The app must concatenate all payload bytes and rebuild full records itself

`0x04` `NOTIFY_DONE`

Format:
[0x04][status][next_seq: u64 BE]
If the app stored everything successfully, use next_seq in CMD_RING_ADVANCE

Status Codes

0: success
6: invalid command
9: storage not ready
10: sequence out of range

Important Semantics

read_seq = oldest unread packet
write_seq = next sequence after newest committed packet
if the ring overwrites old data:
- firmware auto-advances read_seq
- firmware increments dropped_packets
if the app requests a start_seq older than current read_seq, firmware returns 10

Recommended App Flow

Connect.
Subscribe to Control notifications.
Send CMD_RING_INFO.
Wait for NOTIFY_INFO.
Send CMD_RING_READ(read_seq).
Wait for NOTIFY_READ_BEGIN.
Collect NOTIFY_DATA chunks and rebuild 444-byte records.
Wait for NOTIFY_DONE.
If data is safely saved, send CMD_RING_ADVANCE(next_seq).
Repeat until read_seq == write_seq.

Migration Note

Old flow was file-based:

list files
read file
delete file

New flow is ring-based:

get ring info
read by sequence
advance by sequence
clear ring

greptile-apps · 2026-05-10T04:18:36Z

Greptile Summary

This PR replaces the LittleFS file-based offline audio storage with a raw-sector ring buffer written directly to NAND flash sectors. Audio packets are batched into 16 KB aligned sectors, metadata is wear-leveled across 64 dedicated sectors using a generation counter, and the BLE storage service is updated to speak the new sequence-number-based protocol (CMD_RING_INFO, CMD_RING_READ, CMD_RING_ADVANCE, CMD_RING_CLEAR).

sd_card.c: Full rewrite. Each batch holds up to 36 audio packets; the ring wraps automatically, advancing read_seq and incrementing dropped_packets on overflow. Legacy compatibility shims delegate to the new ring API.
storage.c: BLE service rewritten around the ring API. The GATT read handler now returns lock-free atomic-cached stats; stop_requested is now checked inside the -ENOMEM BLE-TX retry path.
sd_card.h: Trimmed from 291 to 57 lines; exposes only the new ring API and sd_ring_info_t.

Confidence Score: 3/5

The core ring-buffer mechanics are well-structured, but silent audio loss and a service starvation path on timeout remain unresolved.

The previously-flagged silent discard of all pre-RTC-sync audio is unchanged. A timeout in any sd_ring_* blocking call leaves the in-flight flag set until the SD worker drains the request, during which all callers receive -EBUSY with no log visibility. The get_oldest_packet_name TOCTOU between sd_ring_get_info and sd_ring_read can produce spurious errors on the BLE file-list path. These issues sit on the core data-path of a newly rewritten storage layer.

omi/firmware/omi/src/sd_card.c needs the most attention: the in-flight timeout handling, the get_oldest_packet_name TOCTOU, and the no-RTC discard path.

Important Files Changed

Filename	Overview
omi/firmware/omi/src/sd_card.c	Complete rewrite replacing LittleFS with a raw sector ring buffer. Contains TOCTOU in get_oldest_packet_name, a starvation path on sd_ring_get_info timeout, and silent discard of pre-RTC-sync audio.
omi/firmware/omi/src/lib/core/storage.c	Rewrites the BLE storage service to use the new ring-buffer API. Replaces the blocking GATT read handler with lock-free atomic reads and adds proper stop_requested check in the -ENOMEM retry path.
omi/firmware/omi/src/lib/core/sd_card.h	Header trimmed from 291 to 57 lines; removes all LittleFS request types and exposes only the new ring-buffer API and sd_ring_info_t.
omi/firmware/omi/src/lib/core/transport.c	Minor update aligning sd_notify_ble_state calls with the new ring-buffer API. No functional regressions.

_{Reviews (2): Last reviewed commit: "improv - the audio sync speed increases ..." | Re-trigger Greptile}

greptile-apps · 2026-05-10T04:18:53Z

+                done_pending = true;
+                remaining_packets = 0;


HEARTBEAT macro and heartbeat_count are vestigial

#define HEARTBEAT 50 is defined but never used. heartbeat_count is incremented on every iteration of write_to_gatt via (heartbeat_count + 1U) % 101U but its value is never read or tested for any condition. Both can be removed.

TuEmb · 2026-05-10T08:00:01Z

@greptile-apps review

greptile-apps · 2026-05-10T08:09:11Z

+static int get_oldest_packet_name(char *buf, size_t buf_size)
 {
-    if (!filenames || !count || max_files <= 0)
+    if (!buf || buf_size == 0U) {
        return -EINVAL;
-    if (!is_mounted)
-        return -ENODEV;
+    }

-    int cache_res = ensure_file_cache();
-    if (cache_res < 0) {
-        return cache_res;
+    sd_ring_info_t info;
+    int ret = sd_ring_get_info(&info);
+    if (ret < 0) {
+        return ret;
    }

-    int file_count = cached_file_list_count;
-    if (file_count > max_files) {
-        file_count = max_files;
+    if (info.read_seq == info.write_seq) {
+        buf[0] = '\0';
+        return 0;
    }

-    for (int i = 0; i < file_count; i++) {
-        strncpy(filenames[i], cached_file_names[i], MAX_FILENAME_LEN - 1);
-        filenames[i][MAX_FILENAME_LEN - 1] = '\0';
-        if (sizes) {
-            sizes[i] = cached_file_sizes[i];
-        }
+    uint8_t packet[RAW_AUDIO_PACKET_BYTES];
+    uint32_t bytes_read = 0;
+    uint32_t packets_read = 0;
+    ret = sd_ring_read(info.read_seq, packet, sizeof(packet), &bytes_read, &packets_read);
+    if (ret < 0 || packets_read == 0U) {
+        return (ret < 0) ? ret : -ENOENT;
    }

-    *count = file_count;
+    format_timestamp_name(sys_get_be32(packet), buf, buf_size);
    return 0;
 }


Stale info.read_seq used in subsequent sd_ring_read call

get_oldest_packet_name snapshots info.read_seq via sd_ring_get_info, then issues a separate sd_ring_read(info.read_seq, ...). Between these two blocking worker round-trips, an external caller can issue CMD_RING_ADVANCE which updates ring_state.read_seq. If that happens, info.read_seq is now behind the current ring_state.read_seq, and read_packets_internal returns -ERANGE. The caller (get_audio_file_list_with_sizes) propagates this error, causing the file-list operation to fail silently from the BLE client's perspective. The fix is to either use a single atomic worker operation that returns both the oldest seq and its packet, or to handle -ERANGE by retrying with a fresh sd_ring_get_info.

greptile-apps · 2026-05-10T08:09:13Z

+    if (k_sem_take(&resp.sem, K_MSEC(5000)) != 0) {
+        return -ETIMEDOUT;
+    }
+
+    *info = resp.info;
+    return resp.res;


sd_ring_get_info leaves info_in_flight set after timeout, silently starving all callers

When k_sem_take times out, the function returns -ETIMEDOUT without clearing info_in_flight. info_in_flight is only cleared by the SD worker via release_resp_busy when it eventually processes the enqueued request. If the SD worker is stalled, every subsequent call to sd_ring_get_info will immediately return -EBUSY for the full duration — potentially freezing the entire storage service. The same pattern applies to sd_ring_read, sd_ring_advance, and sd_ring_clear. At minimum, the timeout branch should log a warning so operators can detect this condition.

TuEmb added 3 commits May 7, 2026 22:00

handle the NAND flash as ring buffer

7fc0fcf

update audio sync method for new ring buffer design

cfa9196

update audio stream method for new ring buffer design

54c0e71

greptile-apps Bot reviewed May 10, 2026

View reviewed changes

TuEmb added 3 commits May 10, 2026 13:03

fix issue of race condition for BLE

01a99e1

improve cache buffer

4b57fff

improv - the audio sync speed increases up to 100 KBps

fc647e0

greptile-apps Bot reviewed May 10, 2026

View reviewed changes

TuEmb added 3 commits May 10, 2026 15:15

fix(storage): avoid file-list failure on read_seq race

f1bb20c

fix - handle timeout for each BLE command

9b722bd

fix - don't suspend spi3 in NAND flash code

9ce2479

mdmohsin7 mentioned this pull request May 10, 2026

feat(app): support ring-buffer offline sync for fw 3.0.20+ #7218

Open

6 tasks

fix - suspend SPI3 when the device is off (current leakage)

41ac00b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat - refactor nand flash as ring buffer#7216

feat - refactor nand flash as ring buffer#7216
TuEmb wants to merge 10 commits into
BasedHardware:mainfrom
TuEmb:TuEmb/refactor_nand_flash

TuEmb commented May 10, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented May 10, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot May 10, 2026

Uh oh!

TuEmb commented May 10, 2026

Uh oh!

greptile-apps Bot May 10, 2026

Uh oh!

greptile-apps Bot May 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

TuEmb commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Core Idea

GATT

Status Char Read

Commands

0x10 CMD_RING_INFO

0x11 CMD_RING_READ

0x12 CMD_RING_ADVANCE

0x13 CMD_RING_CLEAR

0x03 CMD_STOP_SYNC

Notifications

0x01 NOTIFY_ACK

0x02 NOTIFY_INFO

0x05 NOTIFY_READ_BEGIN

0x03 NOTIFY_DATA

0x04 NOTIFY_DONE

Status Codes

Important Semantics

Recommended App Flow

Migration Note

Uh oh!

greptile-apps Bot commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

greptile-apps Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

TuEmb commented May 10, 2026

Uh oh!

greptile-apps Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

TuEmb commented May 10, 2026 •

edited

Loading

`0x10` `CMD_RING_INFO`

`0x11` `CMD_RING_READ`

`0x12` `CMD_RING_ADVANCE`

`0x13` `CMD_RING_CLEAR`

`0x03` `CMD_STOP_SYNC`

`0x01` `NOTIFY_ACK`

`0x02` `NOTIFY_INFO`

`0x05` `NOTIFY_READ_BEGIN`

`0x03` `NOTIFY_DATA`

`0x04` `NOTIFY_DONE`

greptile-apps Bot commented May 10, 2026 •

edited

Loading