8000 extmod/nimble: Add timeout for HCI sync on startup. · micropython/micropython@126f972 · GitHub
[go: up one dir, main page]
Skip to content

Commit 126f972

Browse files
jimmodpgeorge
jimmo
authored and
dpgeorge
committed
extmod/nimble: Add timeout for HCI sync on startup.
This allows `ble.active(1)` to fail correctly if the HCI controller is unavailable. It also avoids an infine loop in the NimBLE event handler where NimBLE doesn't correctly detect that the HCI controller is unavailable and keeps trying to reset. Furthermore, it fixes an issue where GATT service registrations were left allocated, which led to a bad realloc if the stack was activated multiple times.
1 parent 311b851 commit 126f972

File tree

3 files changed

+49
-10
lines changed

3 files changed

+49
-10
lines changed

‎extmod/nimble/modbluetooth_nimble.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252

5353
STATIC uint8_t nimble_address_mode = BLE_OWN_ADDR_RANDOM;
5454

55+
#define NIMBLE_STARTUP_TIMEOUT 2000
56+
STATIC struct ble_npl_sem startup_sem;
57+
5558
// Any BLE_HS_xxx code not in this table will default to MP_EIO.
5659
STATIC int8_t ble_hs_err_to_errno_table[] = {
5760
[BLE_HS_EAGAIN] = MP_EAGAIN,
@@ -206,6 +209,8 @@ STATIC void sync_cb(void) {
206209
ble_svc_gap_device_name_set(MICROPY_PY_BLUETOOTH_DEFAULT_GAP_NAME);
207210

208211
mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE;
212+
213+
ble_npl_sem_release(&startup_sem);
209214
}
210215

211216
STATIC void gatts_register_cb(struct ble_gatt_register_ctxt *ctxt, void *arg) {
@@ -355,6 +360,8 @@ int mp_bluetooth_init(void) {
355360
ble_hs_cfg.gatts_register_cb = gatts_register_cb;
356361
ble_hs_cfg.store_status_cb = ble_store_util_status_rr;
357362

363+
ble_npl_sem_init(&startup_sem, 0);
364+
358365
MP_STATE_PORT(bluetooth_nimble_root_pointers) = m_new0(mp_bluetooth_nimble_root_pointers_t, 1);
359366
mp_bluetooth_gatts_db_create(&MP_STATE_PORT(bluetooth_nimble_root_pointers)->gatts_db);
360367

@@ -370,21 +377,28 @@ int mp_bluetooth_init(void) {
370377
// Initialise NimBLE memory and data structures.
371378
nimble_port_init();
372379

373-
// By default, just register the default gap/gatt service.
374-
ble_svc_gap_init();
375-
ble_svc_gatt_init();
376-
377380
// Make sure that the HCI UART and event handling task is running.
378381
mp_bluetooth_nimble_port_start();
379382

380383
// Static initialization is complete, can start processing events.
381384
mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC;
382385

383-
// Wait for sync callback
384-
while (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE) {
385-
MICROPY_EVENT_POLL_HOOK
386+
ble_npl_sem_pend(&startup_sem, NIMBLE_STARTUP_TIMEOUT);
387+
388+
if (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE) {
389+
mp_bluetooth_deinit();
390+
return MP_ETIMEDOUT;
386391
}
387392

393+
// By default, just register the default gap/gatt service.
394+
ble_svc_gap_init();
395+
ble_svc_gatt_init();
396+
// The preceeding two calls allocate service definitions on the heap,
397+
// then we must now call gatts_start to register those services
398+
// and free the heap memory.
399+
// Otherwise it will be realloc'ed on the next stack startup.
400+
ble_gatts_start();
401+
388402
DEBUG_printf("mp_bluetooth_init: ready\n");
389403

390404
return 0;

‎extmod/nimble/nimble/nimble_npl_os.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "extmod/nimble/hal/hal_uart.h"
3333

3434
#include "extmod/modbluetooth.h"
35+
#include "extmod/nimble/modbluetooth_nimble.h"
3536

3637
#define DEBUG_OS_printf(...) // printf(__VA_ARGS__)
3738
#define DEBUG_MALLOC_printf(...) // printf(__VA_ARGS__)
@@ -180,7 +181,8 @@ struct ble_npl_eventq *global_eventq = NULL;
180181

181182
void mp_bluetooth_nimble_os_eventq_run_all(void) {
182183
for (struct ble_npl_eventq *evq = global_eventq; evq != NULL; evq = evq->nextq) {
183-
while (evq->head != NULL) {
184+
int n = 0;
185+
while (evq->head != NULL && mp_bluetooth_nimble_ble_state > MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) {
184186
struct ble_npl_event *ev = evq->head;
185187
evq->head = ev->next;
186188
if (ev->next) {
@@ -191,6 +193,13 @@ void mp_bluetooth_nimble_os_eventq_run_all(void) {
191193
DEBUG_EVENT_printf("event_run(%p)\n", ev);
192194
ev->fn(ev);
193195
DEBUG_EVENT_printf("event_run(%p) done\n", ev);
196+
197+
if (++n > 3) {
198+
// Limit to running 3 tasks per queue.
199+
// Some tasks (such as reset) can enqueue themselves
200< 6D40 span class="diff-text-marker">+
// making this an infinite loop (while in PENDSV).
201+
break;
202+
}
194203
}
195204
}
196205
}
@@ -348,11 +357,11 @@ ble_npl_error_t ble_npl_sem_pend(struct ble_npl_sem *sem, ble_npl_time_t timeout
348357
}
349358

350359
if (sem->count == 0) {
351-
printf("NimBLE: HCI ACK timeout\n");
360+
DEBUG_SEM_printf("ble_npl_sem_pend: semaphore timeout\n");
352361
return BLE_NPL_TIMEOUT;
353362
}
354363

355-
DEBUG_SEM_printf("got response in %u ms\n", (int)(mp_hal_ticks_ms() - t0));
364+
DEBUG_SEM_printf("ble_npl_sem_pend: acquired in %u ms\n", (int)(mp_hal_ticks_ms() - t0));
356365
}
357366
sem->count -= 1;
358367
return BLE_NPL_OK;

‎ports/unix/mpbthciport.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ STATIC int configure_uart(void) {
105105
// Apply immediately.
106106
if (tcsetattr(uart_fd, TCSANOW, &toptions) < 0) {
107107
DEBUG_printf("Couldn't set term attributes");
108+
109+
close(uart_fd);
110+
uart_fd = -1;
111+
108112
return -1;
109113
}
110114

@@ -149,6 +153,10 @@ int mp_bluetooth_hci_uart_init(uint32_t port, uint32_t baudrate) {
149153
int mp_bluetooth_hci_uart_deinit(void) {
150154
DEBUG_printf("mp_bluetooth_hci_uart_deinit\n");
151155

156+
if (uart_fd == -1) {
157+
return 0;
158+
}
159+
152160
// Wait for the poll loop to terminate when the state is set to OFF.
153161
pthread_join(hci_poll_thread_id, NULL);
154162

@@ -168,6 +176,10 @@ int mp_bluetooth_hci_uart_set_baudrate(uint32_t baudrate) {
168176
int mp_bluetooth_hci_uart_readchar(void) {
169177
// DEBUG_printf("mp_bluetooth_hci_uart_readchar\n");
170178

179+
if (uart_fd == -1) {
180+
return -1;
181+
}
182+
171183
uint8_t c;
172184
ssize_t bytes_read = read(uart_fd, &c, 1);
173185

@@ -184,6 +196,10 @@ int mp_bluetooth_hci_uart_readchar(void) {
184196
int mp_bluetooth_hci_uart_write(const uint8_t *buf, size_t len) {
185197
// DEBUG_printf("mp_bluetooth_hci_uart_write\n");
186198

199+
if (uart_fd == -1) {
200+
return 0;
201+
}
202+
187203
#if DEBUG_HCI_DUMP
188204
printf("[% 8ld] TX: %02x", mp_hal_ticks_ms(), buf[0]);
189205
for (size_t i = 1; i < len; ++i) {

0 commit comments

Comments
 (0)
0