10000 Merge pull request #9374 from dhalbert/espressif-ble-fixes · EternityForest/circuitpython@b2724fe · GitHub
[go: up one dir, main page]
Skip to content

Commit b2724fe

Browse files
tannewttannewt
authored
Merge pull request adafruit#9374 from dhalbert/espressif-ble-fixes
Various Espressif BLE fixes
2 parents f1d29ee + b228cba commit b2724fe

File tree

8 files changed

+209
-110
lines changed

8 files changed

+209
-110
lines changed

‎locale/circuitpython.pot

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2007,6 +2007,10 @@ msgstr ""
20072007
msgid "Too many channels in sample."
20082008
msgstr ""
20092009

2010+
#: ports/espressif/common-hal/_bleio/Characteristic.c
2011+
msgid "Too many descriptors"
2012+
msgstr ""
2013+
20102014
#: shared-module/displayio/__init__.c
20112015
msgid "Too many display busses; forgot displayio.release_displays() ?"
20122016
msgstr ""
@@ -2215,8 +2219,9 @@ msgstr ""
22152219
msgid "Unsupported hash algorithm"
22162220
msgstr ""
22172221

2222+
#: ports/espressif/common-hal/_bleio/Adapter.c
22182223
#: ports/espressif/common-hal/dualbank/__init__.c
2219-
msgid "Update Failed"
2224+
msgid "Update failed"
22202225
msgstr ""
22212226

22222227
#: ports/espressif/common-hal/_bleio/Characteristic.c

‎ports/espressif/common-hal/_bleio/Adapter.c

Lines changed: 59 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "shared-bindings/_bleio/Connection.h"
2525
#include "shared-bindings/_bleio/ScanEntry.h"
2626
#include "shared-bindings/time/__init__.h"
27+
#include "shared/runtime/interrupt_char.h"
2728

2829
#include "controller/ble_ll_adv.h"
2930
#include "nimble/hci_common.h"
@@ -47,22 +48,23 @@
4748
#include "shared-module/os/__init__.h"
4849
#endif
4950

50-
bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT];
51+
// Status variables used while busy-waiting for events.
52+
static volatile bool _nimble_sync;
53+
static volatile int _connection_status;
5154

52-
bool ble_active = false;
55+
bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT];
5356

5457
static void nimble_host_task(void *param) {
5558
nimble_port_run();
5659
nimble_port_freertos_deinit();
5760
}
5861

59-
static TaskHandle_t cp_task = NULL;
6062

6163
static void _on_sync(void) {
6264
int rc = ble_hs_util_ensure_addr(false);
6365
assert(rc == 0);
6466

65-
xTaskNotifyGive(cp_task);
67+
_nimble_sync = true;
6668
}
6769

6870
// All examples have this. It'd make sense in a header.
@@ -133,15 +135,26 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable
133135

134136
ble_store_config_init();
135137

136-
cp_task = xTaskGetCurrentTaskHandle();
137-
138+
_nimble_sync = false;
138139
nimble_port_freertos_init(nimble_host_task);
139-
// Wait for sync.
140-
ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(200));
140+
// Wait for sync from nimble task.
141+
const uint64_t timeout_time_ms = common_hal_time_monotonic_ms() + 200;
142+
while (!_nimble_sync && (common_hal_time_monotonic_ms() < timeout_time_ms)) {
143+
RUN_BACKGROUND_TASKS;
144+
if (mp_hal_is_interrupted()) {
145+
// Return prematurely. Then the interrupt will be raised.
146+
return;
147+
}
148+
}
149+
150+
if (!_nimble_sync) {
151+
mp_raise_RuntimeError(MP_ERROR_TEXT("Update failed"));
152+
}
141153
} else {
142154
int ret = nimble_port_stop();
143-
while (xTaskGetHandle("nimble_host") != NULL) {
144-
vTaskDelay(pdMS_TO_TICKS(2));
155+
while (xTaskGetHandle("nimble_host") != NULL && !mp_hal_is_interrupted()) {
156+
RUN_BACKGROUND_TASKS;
157+
common_hal_time_delay_ms(2);
145158
}
146159
if (ret == 0) {
147160
nimble_port_deinit();
@@ -277,8 +290,19 @@ mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t
277290
duration_ms = BLE_HS_FOREVER;
278291
}
279292

280-
CHECK_NIMBLE_ERROR(ble_gap_disc(own_addr_type, duration_ms, &disc_params,
281-
_scan_event, self->scan_results));
293+
int tries = 5;
294+
int status;
295+
// BLE_HS_EBUSY may occasionally occur, indicating something has not finished. Retry a few times if so.
296+
do {
297+
status = ble_gap_disc(own_addr_type, duration_ms, &disc_params,
298+
_scan_event, self->scan_results);
299+
if (status != BLE_HS_EBUSY) {
300+
break;
301+
}
302+
common_hal_time_delay_ms(50);
303+
RUN_BACKGROUND_TASKS;
304+
} while (tries-- > 0);
305+
CHECK_NIMBLE_ERROR(status);
282306

283307
return MP_OBJ_FROM_PTR(self->scan_results);
284308
}
@@ -309,15 +333,16 @@ static int _mtu_reply(uint16_t conn_handle,
309333
if (error->status == 0) {
310334
connection->mtu = mtu;
311335
}
312-
xTaskNotify(cp_task, conn_handle, eSetValueWithOverwrite);
336+
// Set status var to connection handle to report that connection is now established.
337+
// Another routine is waiting for this.
338+
_connection_status = conn_handle;
313339
return 0;
314340
}
315341

316342
static void _new_connection(uint16_t conn_handle) {
317343
// Set the tx_power for the connection higher than the advertisement.
318344
esp_ble_tx_power_set(conn_handle, ESP_PWR_LVL_N0);
319345

320-
321346
// Find an empty connection. One should always be available because the SD has the same
322347
// total connection limit.
323348
bleio_connection_internal_t *connection = NULL;
@@ -353,13 +378,14 @@ static int _connect_event(struct ble_gap_event *event, void *self_in) {
353378
switch (event->type) {
354379
case BLE_GAP_EVENT_CONNECT:
355380
if (event->connect.status == 0) {
356-
// This triggers an MTU exchange. Its reply will unblock CP.
381+
// This triggers an MTU exchange. Its reply will exit the loop waiting for a connection.
357382
_new_connection(event->connect.conn_handle);
358383
// Set connections objs back to NULL since we have a new
359384
// connection and need a new tuple.
360385
self->connection_objs = NULL;
361386
} else {
362-
xTaskNotify(cp_task, -event->connect.status, eSetValueWithOverwrite);
387+
// The loop waiting for the connection to be comnpleted will stop when _connection_status changes.
388+
_connection_status = -event->connect.status;
363389
}
364390
break;
365391

@@ -397,24 +423,31 @@ mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_addre
397423
ble_addr_t addr;
398424
_convert_address(address, &addr);
399425

400-
cp_task = xTaskGetCurrentTaskHandle();
401-
// Make sure we don't have a pending notification from a previous time. This
402-
// can happen if a previous wait timed out before the notification was given.
403-
xTaskNotifyStateClear(cp_task);
426+
const int timeout_ms = SEC_TO_UNITS(timeout, UNIT_1_MS) + 0.5f;
404427
CHECK_NIMBLE_ERROR(
405428
ble_gap_connect(own_addr_type, &addr,
406-
SEC_TO_UNITS(timeout, UNIT_1_MS) + 0.5f,
429+
timeout_ms,
407430
&conn_params,
408431
_connect_event, self));
409432

410-
int error_code;
411433
// Wait an extra 50 ms to give the connect method the opportunity to time out.
412-
CHECK_NOTIFY(xTaskNotifyWait(0, 0, (uint32_t *)&error_code, pdMS_TO_TICKS(timeout * 1000 + 50)));
434+
435+
const uint64_t timeout_time_ms = common_hal_time_monotonic_ms() + timeout_ms;
436+
// _connection_status gets set to either a positive connection handle or a negative error code.
437+
_connection_status = BLEIO_HANDLE_INVALID;
438+
while (_connection_status == BLEIO_HANDLE_INVALID && (common_hal_time_monotonic_ms() < timeout_time_ms)) {
439+
RUN_BACKGROUND_TASKS;
440+
if (mp_hal_is_interrupted()) {
441+
// Return prematurely. Then the interrupt exception will be raised.
442+
return mp_const_none;
443+
}
444+
}
445+
413446
// Negative values are error codes, connection handle otherwise.
414-
if (error_code < 0) {
415-
CHECK_BLE_ERROR(-error_code);
447+
if (_connection_status < 0) {
448+
CHECK_BLE_ERROR(-_connection_status);
416449
}
417-
uint16_t conn_handle = error_code;
450+
const uint16_t conn_handle = _connection_status;
418451

419452
// TODO: If we have keys, then try and encrypt the connection.
420453

‎ports/espressif/common-hal/_bleio/Characteristic.c

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,47 @@
77
#include <string.h>
88

99
#include "py/runtime.h"
10+
#include "shared/runtime/interrupt_char.h"
1011

1112
#include "shared-bindings/_bleio/__init__.h"
1213
#include "shared-bindings/_bleio/Characteristic.h"
1314
#include "shared-bindings/_bleio/CharacteristicBuffer.h"
1415
#include "shared-bindings/_bleio/Descriptor.h"
1516
#include "shared-bindings/_bleio/PacketBuffer.h"
1617
#include "shared-bindings/_bleio/Service.h"
18+
#include "shared-bindings/time/__init__.h"
1719

1820
#include "common-hal/_bleio/Adapter.h"
1921
#include "common-hal/_bleio/Service.h"
2022

21-
2223
static int characteristic_on_ble_gap_evt(struct ble_gap_event *event, void *param);
2324

25+
static volatile int _completion_status;
26+ 2851
static uint64_t _timeout_start_time;
27+
28+
static void _reset_completion_status(void) {
29+
_completion_status = 0;
30+
}
31+
32+
// Wait for a status change, recorded in a callback.
33+
// Try twice because sometimes we get a BLE_HS_EAGAIN.
34+
// Maybe we should try more than twice.
35+
static int _wait_for_completion(uint32_t timeout_msecs) {
36+
for (int tries = 1; tries <= 2; tries++) {
37+
_timeout_start_time = common_hal_time_monotonic_ms();
38+
while ((_completion_status == 0) &&
39+
(common_hal_time_monotonic_ms() < _timeout_start_time + timeout_msecs) &&
40+
!mp_hal_is_interrupted()) {
41+
RUN_BACKGROUND_TASKS;
42+
}
43+
if (_completion_status != BLE_HS_EAGAIN) {
44+
// Quit, because either the status is either zero (OK) or it's an error.
45+
break;
46+
}
47+
}
48+
return _completion_status;
49+
}
50+
2451
void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_service_obj_t *service,
2552
uint16_t handle, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props,
2653
bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm,
@@ -34,7 +61,7 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
3461
self->props = props;
3562
self->read_perm = read_perm;
3663
self->write_perm = write_perm;
37-
self->observer = NULL;
64+
self->observer = mp_const_none;
3865

3966
// Map CP's property values to Nimble's flag values.
4067
self->flags = 0;
@@ -125,7 +152,6 @@ bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_character
125152
}
126153

127154
typedef struct {
128-
TaskHandle_t task;
129155
uint8_t *buf;
130156
uint16_t len;
131157
} _read_info_t;
@@ -148,9 +174,9 @@ static int _read_cb(uint16_t conn_handle,
148174
// For debugging.
149175
mp_printf(&mp_plat_print, "Read status: %d\n", error->status);
150176
#endif
151-
xTaskNotify(read_info->task, error->status, eSetValueWithOverwrite);
152177
break;
153178
}
179+
_completion_status = error->status;
154180

155181
return 0;
156182
}
@@ -163,14 +189,12 @@ size_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *sel
163189
uint16_t conn_handle = bleio_connection_get_conn_handle(self->service->connection);
164190
if (common_hal_bleio_service_get_is_remote(self->service)) {
165191
_read_info_t read_info = {
166-
.task = xTaskGetCurrentTaskHandle(),
167192
.buf = buf,
168193
.len = len
169194
};
195+
_reset_completion_status();
170196
CHECK_NIMBLE_ERROR(ble_gattc_read(conn_handle, self->handle, _read_cb, &read_info));
171-
int error_code;
172-
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
173-
CHECK_BLE_ERROR(error_code);
197+
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
174198
return read_info.len;
175199
} else {
176200
len = MIN(self->current_value_len, len);
@@ -189,8 +213,7 @@ static int _write_cb(uint16_t conn_handle,
189213
const struct ble_gatt_error *error,
190214
struct ble_gatt_attr *attr,
191215
void *arg) {
192-
TaskHandle_t task = (TaskHandle_t)arg;
193-
xTaskNotify(task, error->status, eSetValueWithOverwrite);
216+
_completion_status = error->status;
194217

195218
return 0;
196219
}
@@ -201,10 +224,9 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
201224
if ((self->props & CHAR_PROP_WRITE_NO_RESPONSE) != 0) {
202225
CHECK_NIMBLE_ERROR(ble_gattc_write_no_rsp_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len));
203226
} else {
204-
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len, _write_cb, xTaskGetCurrentTaskHandle()));
205-
int error_code;
206-
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
207-
CHECK_BLE_ERROR(error_code);
227+
_reset_completion_status();
228+
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len, _write_cb, NULL));
229+
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
208230
}
209231< 10000 /td>
} else {
210232
// Validate data length for local characteristics only.
@@ -338,6 +360,10 @@ bleio_characteristic_properties_t common_hal_bleio_characteristic_get_properties
338360
void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self,
339361
bleio_descriptor_obj_t *descriptor) {
340362
size_t i = self->descriptor_list->len;
363+
if (i >= MAX_DESCRIPTORS) {
364+
mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("Too many descriptors"));
365+
}
366+
341367
mp_obj_list_append(MP_OBJ_FROM_PTR(self->descriptor_list),
342368
MP_OBJ_FROM_PTR(descriptor));
343369

@@ -368,10 +394,9 @@ void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self,
368394
(notify ? 1 << 0 : 0) |
369395
(indicate ? 1 << 1: 0);
370396

371-
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->cccd_handle, &cccd_value, 2, _write_cb, xTaskGetCurrentTaskHandle()));
372-
int error_code;
373-
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
374-
CHECK_BLE_ERROR(error_code);
397+
_reset_completion_status();
398+
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->cccd_handle, &cccd_value, 2, _write_cb, NULL));
399+
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
375400
}
376401

377402
void bleio_characteristic_set_observer(bleio_characteristic_obj_t *self, mp_obj_t observer) {

‎ports/espressif/common-hal/_bleio/CharacteristicBuffer.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ uint32_t common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer
7070
}
7171

7272
uint32_t num_bytes_read = ringbuf_get_n(&self->ringbuf, data, len);
73-
7473
return num_bytes_read;
7574
}
7675

0 commit comments

Comments
 (0)
0