From d8ca96c9c83c9edb212359a763c83c51e27279f2 Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Wed, 25 Feb 2026 17:16:03 +0100 Subject: [PATCH] ble tuning --- zephcore/adapters/ble/ZephyrBLE.cpp | 104 +++++++++++++++--- .../adapters/datastore/ZephyrDataStore.cpp | 90 +++++++++++++-- zephcore/adapters/datastore/ZephyrDataStore.h | 1 + zephcore/boards/common/nrf52_common.conf | 3 + zephcore/boards/common/nrf54l_common.conf | 3 + zephcore/boards/common/zephcore_common.conf | 10 ++ .../drivers/lora/lr11xx/lr11xx_lora.c | 2 +- 7 files changed, 185 insertions(+), 28 deletions(-) diff --git a/zephcore/adapters/ble/ZephyrBLE.cpp b/zephcore/adapters/ble/ZephyrBLE.cpp index 547d2ae..f3b38bc 100644 --- a/zephcore/adapters/ble/ZephyrBLE.cpp +++ b/zephcore/adapters/ble/ZephyrBLE.cpp @@ -110,6 +110,12 @@ static int64_t ble_tx_start_time = 0; /* Active interface tracking */ static enum zephcore_iface active_iface = ZEPHCORE_IFACE_NONE; +/* DLE tracking — set after successful DLE request to avoid double-request */ +static bool dle_requested; + +/* PHY override — request Coded|1M once if phone chose 2M */ +static bool phy_override_sent; + /* Advertising state */ static bool adv_switching = false; static bool adv_is_slow = false; @@ -125,6 +131,9 @@ static const struct bt_gatt_attr *nus_tx_attr; static void ble_tx_complete_cb(struct bt_conn *conn, void *user_data); static int secure_nus_send(struct bt_conn *conn, const void *data, uint16_t len); static void secure_nus_ccc_changed(const struct bt_gatt_attr *attr, uint16_t value); +#if defined(CONFIG_BT_USER_DATA_LEN_UPDATE) +static void request_dle(struct bt_conn *conn); +#endif static ssize_t secure_nus_rx_write(struct bt_conn *conn, const struct bt_gatt_attr *attr, const void *buf, uint16_t len, uint16_t offset, uint8_t flags); static void start_adv(void); @@ -262,23 +271,12 @@ static void connected(struct bt_conn *conn, uint8_t err) k_work_cancel_delayable(&adv_slow_work); adv_is_slow = false; - /* Request max data length (251 bytes) - must be before MTU exchange per BLE spec. - * Apple Accessory Design Guidelines: DLE before MTU handshake for best performance. - */ -#if defined(CONFIG_BT_USER_DATA_LEN_UPDATE) - { - struct bt_conn_le_data_len_param data_len_param = { - .tx_max_len = BT_GAP_DATA_LEN_MAX, - .tx_max_time = BT_GAP_DATA_TIME_MAX, - }; - int dle_err = bt_conn_le_data_len_update(conn, &data_len_param); - if (dle_err) { - LOG_WRN("Failed to request data length update: %d", dle_err); - } else { - LOG_INF("Requested max data length (251 bytes)"); - } - } -#endif + /* DLE is NOT requested here — the phone may start a PHY update LL + * procedure immediately, and BLE allows only one at a time. + * DLE is deferred to le_phy_updated() (after PHY negotiation completes) + * with a fallback in security_changed() if PHY update never fires. */ + dle_requested = false; + phy_override_sent = false; /* Do NOT proactively request security here. * @@ -388,6 +386,11 @@ static void security_changed(struct bt_conn *conn, bt_security_t level, enum bt_ } if (level >= BT_SECURITY_L2) { +#if defined(CONFIG_BT_USER_DATA_LEN_UPDATE) + /* Fallback DLE request — if le_phy_updated() already sent it, + * request_dle() returns immediately (dle_requested flag). */ + request_dle(conn); +#endif /* Request our preferred connection parameters. */ struct bt_le_conn_param conn_param = { .interval_min = BLE_DEFAULT_MIN_INTERVAL, @@ -432,6 +435,27 @@ static void le_param_updated(struct bt_conn *conn, uint16_t interval, } #if defined(CONFIG_BT_USER_DATA_LEN_UPDATE) +/* Request max DLE (251 bytes). Called from le_phy_updated() after PHY + * negotiation completes, and from security_changed() as a fallback if + * PHY update never fires. The dle_requested flag prevents double-request. */ +static void request_dle(struct bt_conn *conn) +{ + if (dle_requested) { + return; + } + struct bt_conn_le_data_len_param data_len_param = { + .tx_max_len = BT_GAP_DATA_LEN_MAX, + .tx_max_time = BT_GAP_DATA_TIME_MAX, + }; + int err = bt_conn_le_data_len_update(conn, &data_len_param); + if (err) { + LOG_WRN("Failed to request data length update: %d", err); + } else { + LOG_INF("Requested max data length (251 bytes)"); + dle_requested = true; + } +} + static void le_data_len_updated(struct bt_conn *conn, struct bt_conn_le_data_len_info *info) { ARG_UNUSED(conn); @@ -441,6 +465,49 @@ static void le_data_len_updated(struct bt_conn *conn, struct bt_conn_le_data_len } #endif +#if defined(CONFIG_BT_USER_PHY_UPDATE) +static const char *phy_name(uint8_t phy) +{ + switch (phy) { + case BT_GAP_LE_PHY_1M: return "1M"; + case BT_GAP_LE_PHY_2M: return "2M"; + case BT_GAP_LE_PHY_CODED: return "Coded"; + default: return "Unknown"; + } +} + +static void le_phy_updated(struct bt_conn *conn, struct bt_conn_le_phy_info *param) +{ + LOG_INF("BLE PHY updated: TX=%s RX=%s", phy_name(param->tx_phy), + phy_name(param->rx_phy)); + + /* If the phone chose 2M, override with Coded|1M preference. + * Coded gives ~4× BLE range (S=8); if the phone doesn't support it, + * the intersection is 1M (better range than 2M for a mesh device). + * Only try once to avoid ping-pong if the phone insists on 2M. */ + if (param->tx_phy == BT_GAP_LE_PHY_2M && !phy_override_sent) { + const struct bt_conn_le_phy_param phy_pref = { + .options = BT_CONN_LE_PHY_OPT_NONE, + .pref_tx_phy = BT_GAP_LE_PHY_CODED | BT_GAP_LE_PHY_1M, + .pref_rx_phy = BT_GAP_LE_PHY_CODED | BT_GAP_LE_PHY_1M, + }; + phy_override_sent = true; + int err = bt_conn_le_phy_update(conn, &phy_pref); + if (err) { + LOG_WRN("PHY override failed: %d, requesting DLE", err); + } else { + LOG_INF("Requested Coded|1M PHY (overriding 2M)"); + return; /* DLE when this callback fires again */ + } + } + + /* PHY is settled — request DLE. Deferred here from connected() + * because the phone starts a PHY procedure on connect and BLE + * only allows one LL procedure at a time. */ + request_dle(conn); +} +#endif + BT_CONN_CB_DEFINE(conn_callbacks) = { .connected = connected, .disconnected = disconnected, @@ -448,6 +515,9 @@ BT_CONN_CB_DEFINE(conn_callbacks) = { .le_param_req = le_param_req, .le_param_updated = le_param_updated, .security_changed = security_changed, +#if defined(CONFIG_BT_USER_PHY_UPDATE) + .le_phy_updated = le_phy_updated, +#endif #if defined(CONFIG_BT_USER_DATA_LEN_UPDATE) .le_data_len_updated = le_data_len_updated, #endif diff --git a/zephcore/adapters/datastore/ZephyrDataStore.cpp b/zephcore/adapters/datastore/ZephyrDataStore.cpp index 622846d..c66ec3d 100644 --- a/zephcore/adapters/datastore/ZephyrDataStore.cpp +++ b/zephcore/adapters/datastore/ZephyrDataStore.cpp @@ -82,9 +82,37 @@ void ZephyrDataStore::begin() migrateToExternalFS(); } + /* Clean up stale .tmp files from interrupted saves. + * If a .tmp file exists, the save was interrupted before the + * atomic rename — the original file is still intact. */ + cleanStaleTmpFiles(); + checkAdvBlobFile(); } +void ZephyrDataStore::cleanStaleTmpFiles() +{ + /* If a .tmp file exists, a save was interrupted before the atomic + * rename completed. Since we never unlink the original before + * rename, the original file is guaranteed intact. The .tmp may + * be partial (crash during write) or complete (crash between + * close and rename). Either way, delete it — we keep the last + * successfully-committed version. Worst case: we lose the one + * save that was in progress, but never lose ALL data. */ + const char *paths[] = { contactsFile(), channelsFile(), advBlobsFile() }; + char tmp_path[48]; + for (size_t i = 0; i < ARRAY_SIZE(paths); i++) { + snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", paths[i]); + struct fs_dirent ent; + if (fs_stat(tmp_path, &ent) == 0) { + LOG_INF("PREVIOUS REBOOT CORRUPTED FS! " + "Deleting temp file: %s (%zu bytes)", + tmp_path, ent.size); + fs_unlink(tmp_path); + } + } +} + bool ZephyrDataStore::exists(const char *path) { struct fs_dirent ent; @@ -505,14 +533,21 @@ void ZephyrDataStore::saveContacts(DataStoreHost *host) { const char *path = contactsFile(); LOG_INF("saveContacts: path=%s (_has_ext_fs=%d)", path, _has_ext_fs ? 1 : 0); + + /* Crash-safe write: write to .tmp, sync, then atomic rename. + * If power is lost before rename completes, the original file + * is still intact. Stale .tmp files are cleaned at boot. */ + char tmp_path[48]; + snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path); + struct fs_file_t file; fs_file_t_init(&file); - if (exists(path)) { - fs_unlink(path); + if (exists(tmp_path)) { + fs_unlink(tmp_path); } - int rc = fs_open(&file, path, FS_O_CREATE | FS_O_WRITE); + int rc = fs_open(&file, tmp_path, FS_O_CREATE | FS_O_WRITE); if (rc < 0) { - LOG_ERR("saveContacts: fs_open(%s) failed: %d", path, rc); + LOG_ERR("saveContacts: fs_open(%s) failed: %d", tmp_path, rc); return; } uint32_t idx = 0; @@ -535,7 +570,22 @@ void ZephyrDataStore::saveContacts(DataStoreHost *host) } int sync_rc = fs_sync(&file); fs_close(&file); - LOG_INF("saveContacts: saved %u contacts to %s (sync=%d)", idx, path, sync_rc); + + if (sync_rc < 0) { + LOG_ERR("saveContacts: sync failed (%d), keeping old file", sync_rc); + fs_unlink(tmp_path); + return; + } + + /* Atomic replace — LittleFS 2 rename() replaces the destination + * in a single CRC-protected metadata commit. No unlink needed. + * Power loss at any point: either old file or new file, never empty. */ + rc = fs_rename(tmp_path, path); + if (rc < 0) { + LOG_ERR("saveContacts: rename %s → %s failed: %d", tmp_path, path, rc); + } else { + LOG_INF("saveContacts: saved %u contacts to %s", idx, path); + } } void ZephyrDataStore::loadChannels(DataStoreHost *host) @@ -568,13 +618,18 @@ void ZephyrDataStore::loadChannels(DataStoreHost *host) void ZephyrDataStore::saveChannels(DataStoreHost *host) { const char *path = channelsFile(); + + /* Crash-safe write: write to .tmp, sync, then atomic rename. */ + char tmp_path[48]; + snprintf(tmp_path, sizeof(tmp_path), "%s.tmp", path); + struct fs_file_t file; fs_file_t_init(&file); - if (exists(path)) { - fs_unlink(path); + if (exists(tmp_path)) { + fs_unlink(tmp_path); } - if (fs_open(&file, path, FS_O_CREATE | FS_O_WRITE) < 0) { - LOG_ERR("Failed to open %s for write", path); + if (fs_open(&file, tmp_path, FS_O_CREATE | FS_O_WRITE) < 0) { + LOG_ERR("saveChannels: fs_open(%s) failed", tmp_path); return; } uint8_t channel_idx = 0; @@ -586,8 +641,23 @@ void ZephyrDataStore::saveChannels(DataStoreHost *host) if (fs_write(&file, (uint8_t *)ch.channel.secret, 32) != 32) break; channel_idx++; } + int sync_rc = fs_sync(&file); fs_close(&file); - LOG_DBG("Saved %u channels to %s", channel_idx, path); + + if (sync_rc < 0) { + LOG_ERR("saveChannels: sync failed (%d), keeping old file", sync_rc); + fs_unlink(tmp_path); + return; + } + + /* Atomic replace — LittleFS 2 rename() replaces the destination + * in a single CRC-protected metadata commit. No unlink needed. */ + int rc = fs_rename(tmp_path, path); + if (rc < 0) { + LOG_ERR("saveChannels: rename %s → %s failed: %d", tmp_path, path, rc); + } else { + LOG_DBG("saveChannels: saved %u channels to %s", channel_idx, path); + } } uint8_t ZephyrDataStore::getBlobByKey(const uint8_t key[], int key_len, uint8_t dest_buf[]) diff --git a/zephcore/adapters/datastore/ZephyrDataStore.h b/zephcore/adapters/datastore/ZephyrDataStore.h index ac36e75..87eff4b 100644 --- a/zephcore/adapters/datastore/ZephyrDataStore.h +++ b/zephcore/adapters/datastore/ZephyrDataStore.h @@ -76,6 +76,7 @@ private: const char *advBlobsFile() const { return _has_ext_fs ? EXT_ADV_BLOBS_FILE : INT_ADV_BLOBS_FILE; } int maxBlobRecs() const { return _has_ext_fs ? 100 : 20; } + void cleanStaleTmpFiles(); void checkAdvBlobFile(); void migrateToExternalFS(); bool openRead(const char *path, uint8_t *buf, size_t buf_sz, size_t &out_len); diff --git a/zephcore/boards/common/nrf52_common.conf b/zephcore/boards/common/nrf52_common.conf index e26dd97..e33e130 100644 --- a/zephcore/boards/common/nrf52_common.conf +++ b/zephcore/boards/common/nrf52_common.conf @@ -21,6 +21,9 @@ CONFIG_BT_CTLR_TX_PWR_PLUS_4=y # Data Length Extension — controller-side max (host-side in zephcore_common.conf) CONFIG_BT_CTLR_DATA_LENGTH_MAX=251 +# Coded PHY — enable in nRF LL controller (host auto-request in zephcore_common.conf) +CONFIG_BT_CTLR_PHY_CODED=y + # ========== Flash: nRF52-specific ========== # QSPI flash — disabled by default (safer — avoids hang if chip missing). # Boards with QSPI (Wio Tracker L1) override in board.conf. diff --git a/zephcore/boards/common/nrf54l_common.conf b/zephcore/boards/common/nrf54l_common.conf index a30b9da..756a255 100644 --- a/zephcore/boards/common/nrf54l_common.conf +++ b/zephcore/boards/common/nrf54l_common.conf @@ -20,6 +20,9 @@ CONFIG_BT_CTLR_TX_PWR_PLUS_4=y # Data Length Extension — controller-side max CONFIG_BT_CTLR_DATA_LENGTH_MAX=251 +# Coded PHY — enable in nRF LL controller (host auto-request in zephcore_common.conf) +CONFIG_BT_CTLR_PHY_CODED=y + # ========== SEGGER RTT (J-Link / CMSIS-DAP debug) ========== CONFIG_LOG_BACKEND_RTT=y CONFIG_USE_SEGGER_RTT=y diff --git a/zephcore/boards/common/zephcore_common.conf b/zephcore/boards/common/zephcore_common.conf index e08fe34..722949b 100644 --- a/zephcore/boards/common/zephcore_common.conf +++ b/zephcore/boards/common/zephcore_common.conf @@ -102,6 +102,16 @@ CONFIG_BT_RX_STACK_SIZE=2048 # Data Length Extension update support (host side) CONFIG_BT_USER_DATA_LEN_UPDATE=y +# PHY update callback — lets us log the negotiated PHY and sequence DLE +# after the PHY procedure completes (avoids LL procedure collision). +# +# NOTE: BT_USER_PHY_UPDATE disables BT_AUTO_PHY_UPDATE (Kconfig default). +# We intentionally do NOT set BT_AUTO_PHY_UPDATE — peripheral-initiated +# PHY updates cause iOS disconnects. The phone (central) initiates PHY +# update and le_phy_updated fires regardless. Coded PHY can be requested +# manually after security is established (future enhancement). +CONFIG_BT_USER_PHY_UPDATE=y + # Privacy - use Resolvable Private Addresses (Apple requirement) CONFIG_BT_PRIVACY=y CONFIG_BT_RPA_TIMEOUT=900 diff --git a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c index 565b473..f22917d 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c @@ -973,7 +973,7 @@ static DEVICE_API(lora, lr11xx_lora_api) = { static const struct lr11xx_config lr11xx_config_##n = { \ .bus = SPI_DT_SPEC_INST_GET(n, \ SPI_WORD_SET(8) | SPI_OP_MODE_MASTER | \ - SPI_TRANSFER_MSB, 0), \ + SPI_TRANSFER_MSB), \ .reset = GPIO_DT_SPEC_INST_GET(n, reset_gpios), \ .busy = GPIO_DT_SPEC_INST_GET(n, busy_gpios), \ .dio1 = GPIO_DT_SPEC_INST_GET(n, dio1_gpios), \