Feat - refactor SD card, improve bluetooth throughput, remove wifi feature, fix issues#5897
Feat - refactor SD card, improve bluetooth throughput, remove wifi feature, fix issues#5897TuEmb wants to merge 21 commits intoBasedHardware:mainfrom
Conversation
Greptile SummaryThis PR is a substantial firmware refactor that removes the WiFi feature, migrates the SD card filesystem from FATFS to LittleFS (for power-loss safety and wear-leveling), upgrades the BLE audio sync protocol to support multi-file operations, and improves BLE throughput through tighter connection intervals, semaphore-throttled audio notifications, and MTU retry logic. Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Phone as Phone App
participant BLE as BLE Stack (transport.c)
participant Store as Storage Thread (storage.c)
participant SD as SD Worker (sd_card.c / LittleFS)
Phone->>BLE: Write CMD_LIST_FILES (0x10)
BLE->>Store: list_files_requested = 1
Store->>SD: REQ_GET_FILE_LIST (prio queue)
SD-->>Store: filenames + sizes
Store->>Phone: Notify [count][ts1][sz1]...[tsN][szN]
Phone->>BLE: Write CMD_READ_FILE (0x11) [index][offset]
BLE->>Store: setup_file_transfer(index, offset)
Store->>Store: remaining_length = file_size - offset
loop BLE send loop (write_to_gatt)
Store->>SD: REQ_READ_DATA(filename, offset, chunk) (prio queue)
SD-->>Store: audio bytes
Store->>Phone: Notify [ts:4][data:chunk]
end
Store->>SD: Auto-delete file (if not active recording)
Store->>Phone: Notify [100] (done)
Phone->>BLE: Write CMD_DELETE_FILE (0x12) [index]
BLE->>Store: delete_file_index = index
Store->>SD: REQ_DELETE_FILE (worker loop)
SD-->>Store: result
Store->>Phone: Notify [result]
Phone->>BLE: Time-sync write [epoch:4]
BLE->>BLE: rtc_set_utc_time()
BLE->>SD: sd_notify_time_synced() → REQ_TIME_SYNCED (prio queue)
SD->>SD: Rename TMP_*.txt → epoch.txt
Last reviewed commit: "disable log for rele..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static char sync_file_list[MAX_AUDIO_FILES][MAX_FILENAME_LEN]; | ||
| static uint32_t sync_file_sizes[MAX_AUDIO_FILES]; | ||
| static int sync_file_count = 0; |
There was a problem hiding this comment.
sync_file_count, sync_file_list, and sync_file_sizes are static module-level variables that are written by the storage thread (in refresh_file_list_cache, line 246–254) and read concurrently from the BLE GATT write callback (parse_storage_command, lines 375–400) — which runs on the BLE host thread.
Without a mutex or atomic access, a classic TOCTOU race exists:
- BLE callback reads
sync_file_count = 5. - Storage thread resets
sync_file_count = 0and starts repopulatingsync_file_list. - BLE callback uses index 4 against a partially-populated
sync_file_list, reading stale or zero-initialized data.
The consequence is that the storage layer could set up a transfer (setup_file_transfer) with a garbage filename, leading to an immediate read error or a corrupted transfer session.
A lightweight fix is to protect reads/writes with a K_MUTEX_DEFINE or to copy the required slice of the list under lock before using it in parse_storage_command.
| if (interval > 24) { | ||
| LOG_WRN("Connection interval still high (%u units). Re-requesting preferred params.", interval); | ||
| update_conn_params(conn); | ||
| } |
There was a problem hiding this comment.
_le_param_updated is a BLE stack callback. When interval > 24, it calls update_conn_params, which contains a retry loop that sleeps up to CONN_PARAM_RETRY_COUNT × CONN_PARAM_RETRY_DELAY_MS = 3 × 300 ms = 900 ms on the BLE host thread.
Two problems:
- Latency spike: Blocking the BLE host thread for up to 900 ms can delay or drop incoming/outgoing GATT operations, causing the phone to see ATT timeouts.
- Negotiation loop: If the central device keeps settling on an interval slightly above 24 units (e.g., a phone that always negotiates 30 ms), every
_le_param_updatedcallback will trigger another round ofupdate_conn_params→ re-request → new_le_param_updated, forming a tight re-negotiation cycle for the lifetime of the connection.
The retry logic is appropriate in _transport_connected, but from _le_param_updated a single non-blocking call (no retry loop, no sleep) is safer:
| if (interval > 24) { | |
| LOG_WRN("Connection interval still high (%u units). Re-requesting preferred params.", interval); | |
| update_conn_params(conn); | |
| } | |
| if (interval > 24) { | |
| LOG_WRN("Connection interval still high (%u units). Re-requesting preferred params.", interval); | |
| bt_conn_le_param_update(conn, &(struct bt_le_conn_param){ | |
| .interval_min = 6, .interval_max = 12, .latency = 0, .timeout = 400 | |
| }); | |
| } |
| LOG_INF("Time synchronized successfully"); | ||
|
|
||
| /* Notify SD card module so it can rename temp files to real timestamps */ | ||
| sd_notify_time_synced(epoch_s); |
There was a problem hiding this comment.
sd_notify_time_synced per time-sync event
sd_notify_time_synced(epoch_s) is called here directly from time_sync_write_handler (line 244). However, rtc_set_utc_time (called at line 235) now schedules rtc_persist_work, whose handler rtc_persist_work_handler in rtc.c also calls sd_notify_time_synced (line 32 of rtc.c).
This means for every BLE time-sync event:
sd_notify_time_syncedfires immediately (this line, BLE callback thread).sd_notify_time_syncedfires again shortly after (system work queue fromrtc.c).
The SD worker receives two REQ_TIME_SYNCED requests. The second rename attempt will encounter current_file_needs_rename == false and silently return, so there is no corruption — but it does queue an extra priority message, needlessly occupying the priority queue and adding log noise.
Since rtc.c now owns the SD notification as part of RTC persistence, the call here in transport.c should be removed:
| sd_notify_time_synced(epoch_s); | |
| LOG_INF("Time synchronized successfully"); |
| ## Enable the lines below to enable logs via UART/USB | ||
| CONFIG_LOG=y | ||
| CONFIG_LOG_PRINTK=y | ||
| CONFIG_UART_CONSOLE=y |
There was a problem hiding this comment.
CONFIG_UART_CONSOLE=y
CONFIG_UART_CONSOLE=y appears twice in this file — at line 191 (under the "Console" section) and again at line 201 (under the "Logs" section). The second occurrence is harmless but creates confusion about where the canonical setting lives. The one in the Logs block appears to be a copy-paste artefact from the comment block above it.
| CONFIG_UART_CONSOLE=y | |
| CONFIG_LOG_PRINTK=y | |
| CONFIG_LOG_PROCESS_THREAD_PRIORITY=5 |
| // Delay a bit before PHY request to avoid early HCI race on some phones. | ||
| k_sleep(K_MSEC(300)); |
There was a problem hiding this comment.
The comment and k_sleep call at lines 569–570 use 8-space indentation while all surrounding code in _transport_connected uses 4-space indentation, suggesting this block was copied from inside a nested scope and never re-indented.
| // Delay a bit before PHY request to avoid early HCI race on some phones. | |
| k_sleep(K_MSEC(300)); | |
| // Delay a bit before PHY request to avoid early HCI race on some phones. | |
| k_sleep(K_MSEC(300)); |
|
Power consumption profile @4.2V: Offline mode: 8.65 mA Live mode: 7.68 mA Sync mode: 17.7 mA Deepsleep: 34 uA |
1 participant
Fixed #4429 #4351 #4213 #4101 #4052 #4077 #3976 #5634
This pull request removes WiFi support from the OMI firmware and introduces several improvements to audio storage and Bluetooth configuration. The WiFi-related code, configuration options, and dependencies have been removed from both the build system and source files. Additionally, the SD card storage subsystem is enhanced with new request types and structures to better support file management. Bluetooth and logging configurations are streamlined and improved, and the default filesystem is switched to LittleFS for better reliability.
Major removals and refactors:
omi.conf,Kconfig,CMakeLists.txt, and source files, including conditional compilation, initialization, and shutdown logic.Audio storage and SD card improvements:
sd_card.hfor advanced file management, including file stats, file listing, deletion, and rotation support.Bluetooth and logging configuration:
Filesystem and memory usage:
These changes collectively focus the firmware on offline and Bluetooth functionality, improve reliability and maintainability, and lay groundwork for more robust audio file management.