feat: inline timestamp markers for offline audio (firmware)

[mdmohsin7]

Summary

• Embeds inline 0xFF timestamp markers in the offline audio data stream so each recording segment (between BLE disconnects or power cycles) has an exact UTC start time
• Extends info.txt on SD card from 4 to 8 bytes to persist a recording start time alongside the read offset
• Exposes recording start time via the BLE storage characteristic (3rd uint32 value)
• Flushes the storage write buffer on device shutdown to prevent data loss

Split from #4851 — firmware changes only. See #5572 for the app counterpart.

Firmware changes

• transport.c: write_timestamp_to_storage() injects a 5-byte marker (0xFF + 4-byte LE epoch) before the first offline audio frame of each segment. storage_flush_buffer() flushes pending data on shutdown. needs_timestamp_marker flag resets on each BLE connect.
• sd_card.c: recording_start_time persisted in info.txt (backward-compatible 8-byte format)
• storage.c: BLE read characteristic returns [file_size, offset, recording_start_time]
• button.c: Calls storage_flush_buffer() before app_sd_off() in turnoff_all()

Backward compatibility

• New firmware + old app: Old parser hits bounds check on 255-byte frame size and skips to next block. Minimal data loss (5 bytes per marker).
• Old firmware + new app: No markers in stream, new code path never triggers. Works unchanged.
• info.txt: Old 4-byte format reads correctly (extra bytes default to zero).

Test plan

• Build firmware via Docker and flash OTA
• Connect phone → disconnect BLE → record ~1 min offline → reconnect briefly → disconnect → record ~1 min → sync
• Test power off during offline recording → power on → record → sync → verify two segments
• Test with old app version to confirm backward compatibility

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR injects inline 0xFF + 4-byte UTC timestamp markers into the offline audio stream, exposes the recording start time via the BLE storage characteristic (extending the read response from 8 to 12 bytes), and flushes the in-memory write buffer before SD card power-off on shutdown. The approach fits the existing SD worker / pusher-thread architecture in the Omi firmware, and the info.txt format change (4 → 8 bytes) is handled in a backward-compatible way.

Key issues found:

• Logic bug in timestamp flag handling (transport.c): needs_timestamp_marker is unconditionally cleared after calling write_timestamp_to_storage(), even when that function returns early because the RTC time is unavailable. This means an offline recording that starts before time-sync is never stamped, even after time becomes available mid-session.
• Linker error for non-offline-storage builds (transport.h / button.c): storage_flush_buffer() is declared and called unconditionally but only defined under CONFIG_OMI_ENABLE_OFFLINE_STORAGE, causing an undefined-reference link failure for any build that disables offline storage.
• Data race on shutdown (button.c): storage_flush_buffer() accesses the pusher thread's storage_temp_data and buffer_offset without any synchronisation, risking buffer corruption if the pusher is still processing audio when the button handler fires.

Confidence Score: 2/5

• Not safe to merge — contains a logic bug that silently drops timestamp markers and a build-breaking header guard omission.
• Two issues require fixes before merge: (1) the needs_timestamp_marker flag is always cleared regardless of whether a marker was actually written, which is the core correctness concern for the feature; (2) storage_flush_buffer() lacks the required #ifdef guard in its declaration, which will break any build configuration that disables offline storage. The race condition on shutdown is a lower-severity concern given typical shutdown sequencing, but it still needs addressing.
• transport.c (timestamp flag logic) and transport.h (missing compile guard on storage_flush_buffer declaration).

Important Files Changed

Filename	Overview
omi/firmware/omi/src/lib/core/transport.c	Core of the PR — adds inline 0xFF timestamp markers, write_timestamp_to_storage, and storage_flush_buffer. Contains a logic bug where needs_timestamp_marker is always cleared even when the marker write is skipped due to unavailable RTC time.
omi/firmware/omi/src/lib/core/transport.h	Adds unconditional declaration of storage_flush_buffer(), which is only defined under CONFIG_OMI_ENABLE_OFFLINE_STORAGE — causes a linker error in non-offline-storage builds.
omi/firmware/omi/src/lib/core/button.c	Adds storage_flush_buffer() call before SD card power-off; correct intent but lacks synchronisation with the still-running pusher thread.
omi/firmware/omi/src/sd_card.c	Extends info.txt from 4 bytes to 8 bytes to persist recording_start_time alongside the write offset; backward-compatible read path handled correctly. Adds set/get_recording_start_time() implementations.
omi/firmware/omi/src/lib/core/sd_card.h	Adds declarations for set_recording_start_time() / get_recording_start_time() with Doxygen comments; no issues.
omi/firmware/omi/src/lib/core/storage.c	BLE storage characteristic extended to expose recording_start_time as a third uint32_t; switch-case reformatted; no functional issues.

Sequence Diagram

sequenceDiagram
    participant MIC as Microphone / TX Queue
    participant PUSH as pusher thread
    participant SD as SD Worker Thread
    participant BLE as BLE Client

    MIC->>PUSH: read_from_tx_queue()

    alt BLE connected & subscribed
        PUSH->>PUSH: needs_timestamp_marker = true
        PUSH->>BLE: push_to_gatt()
    else No BLE connection (offline mode)
        alt needs_timestamp_marker == true
            PUSH->>PUSH: write_timestamp_to_storage()<br/>[0xFF | UTC epoch 4B]
            PUSH->>PUSH: needs_timestamp_marker = false
        end
        PUSH->>PUSH: set_recording_start_time(now)
        PUSH->>PUSH: write_to_storage() → storage_temp_data[]
        alt buffer_offset == MAX_WRITE_SIZE
            PUSH->>SD: write_to_file(storage_temp_data, MAX_WRITE_SIZE)
        end
    end

    note over BLE: BLE storage characteristic read
    BLE->>SD: storage_read_characteristic()
    SD-->>BLE: [file_size: u32 | offset: u32 | recording_start_time: u32]

    note over PUSH,SD: On shutdown (button press)
    PUSH->>SD: storage_flush_buffer() → write_to_file(remaining bytes)
    PUSH->>SD: app_sd_off()

Loading

_{Last reviewed commit: 048ca41}

[greptile-apps]

Timestamp marker flag cleared even when write is skipped

needs_timestamp_marker is set to false unconditionally, even though write_timestamp_to_storage() returns early (without writing anything) when get_utc_time() == 0.

If the RTC has not been synced at the moment recording starts, the marker is silently skipped and the flag is cleared. When time becomes available on a later audio frame, the flag remains false so no marker is ever inserted into that recording session. This means the entire offline recording could go without any timestamp markers, defeating the purpose of the feature.

The flag should only be cleared after a marker is actually written:

Suggested change

	if (needs_timestamp_marker) {
	write_timestamp_to_storage();
	needs_timestamp_marker = false;
	}
	if (needs_timestamp_marker) {
	if (get_utc_time() != 0) {
	write_timestamp_to_storage();
	needs_timestamp_marker = false;
	}
	}

Alternatively, have write_timestamp_to_storage() return a bool indicating success and only clear the flag on true.

[greptile-apps]

Linker error when CONFIG_OMI_ENABLE_OFFLINE_STORAGE is disabled

storage_flush_buffer() is declared here unconditionally and called in button.c unconditionally (line 389), but its definition in transport.c is inside the #ifdef CONFIG_OMI_ENABLE_OFFLINE_STORAGE block. Any build configuration that does not enable offline storage will fail with an undefined reference at link time.

The declaration should be guarded to match the definition:

Suggested change

	/**
	* @brief Flush any pending offline storage buffer to SD card
	*/
	void storage_flush_buffer(void);
	#ifdef CONFIG_OMI_ENABLE_OFFLINE_STORAGE
	/**
	* @brief Flush any pending offline storage buffer to SD card
	*/
	void storage_flush_buffer(void);
	#else
	static inline void storage_flush_buffer(void) {}
	#endif

The call site in button.c can then remain as-is (the inline no-op is compiled away when offline storage is disabled).

[greptile-apps]

Unsynchronised access to pusher thread's buffer

storage_flush_buffer() reads and zeroes buffer_offset and reads storage_temp_data, which are static variables in transport.c exclusively owned by the pusher thread (write_to_storage / write_timestamp_to_storage). This call is issued from the button handler without any lock or stopping the pusher first. If the pusher thread is still active (processing the last audio frames from the TX queue), a concurrent write to storage_temp_data / buffer_offset results in a data race that can corrupt the final flush.

Consider signaling pusher_stop_flag (or an equivalent mechanism) and waiting for the pusher thread to idle before calling storage_flush_buffer(), or guard the buffer accesses with a mutex/atomic check that is also respected inside write_to_storage.