Skip to content

Commit

Permalink
Merge pull request #9616 from tannewt/fix_c6_crash
Browse files Browse the repository at this point in the history
Three fixes found for ESP32-C6
  • Loading branch information
dhalbert authored Sep 12, 2024
2 parents 2581deb + b5517cb commit fad755d
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 39 deletions.
11 changes: 11 additions & 0 deletions devices/ble_hci/common-hal/_bleio/Characteristic.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
}
}

bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self) {
return self->handle == BLE_GATT_HANDLE_INVALID;
}

void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) {
if (common_hal_bleio_characteristic_deinited(self)) {
return;
}
self->handle = BLE_GATT_HANDLE_INVALID;
}

mp_obj_tuple_t *common_hal_bleio_characteristic_get_descriptors(bleio_characteristic_obj_t *self) {
return mp_obj_new_tuple(self->descriptor_list->len, self->descriptor_list->items);
}
Expand Down
64 changes: 32 additions & 32 deletions ports/espressif/common-hal/_bleio/Characteristic.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "shared-bindings/_bleio/PacketBuffer.h"
#include "shared-bindings/_bleio/Service.h"
#include "shared-bindings/time/__init__.h"
#include "supervisor/shared/safe_mode.h"

#include "common-hal/_bleio/Adapter.h"
#include "common-hal/_bleio/Service.h"
Expand Down Expand Up @@ -100,27 +101,20 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
self->flags |= BLE_GATT_CHR_F_WRITE_AUTHEN;
}

if (initial_value_bufinfo != NULL) {
// Copy the initial value if it's on the heap. Otherwise it's internal and we may not be able
// to allocate.
self->current_value_len = initial_value_bufinfo->len;
if (gc_alloc_possible()) {
self->current_value = m_malloc(max_length);
self->current_value_alloc = max_length;
if (gc_nbytes(initial_value_bufinfo->buf) > 0) {
memcpy(self->current_value, initial_value_bufinfo->buf, self->current_value_len);
}
} else {
self->current_value = initial_value_bufinfo->buf;
assert(self->current_value_len == max_length);
}
if (gc_alloc_possible()) {
self->current_value = m_malloc(max_length);
} else {
self->current_value = port_malloc(max_length, false);
if (self->current_value != NULL) {
self->current_value_alloc = max_length;
self->current_value_len = 0;
if (self->current_value == NULL) {
reset_into_safe_mode(SAFE_MODE_NO_HEAP);
}
}
self->current_value_alloc = max_length;
self->current_value_len = 0;

if (initial_value_bufinfo != NULL) {
common_hal_bleio_characteristic_set_value(self, initial_value_bufinfo);
}

if (gc_alloc_possible()) {
self->descriptor_list = mp_obj_new_list(0, NULL);
Expand All @@ -140,6 +134,26 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
}
}

bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self) {
return self->current_value == NULL;
}

void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) {
if (common_hal_bleio_characteristic_deinited(self)) {
return;
}
if (self->current_value == NULL) {
return;
}

if (gc_nbytes(self->current_value) > 0) {
m_free(self->current_value);
} else {
port_free(self->current_value);
}
self->current_value = NULL;
}

mp_obj_tuple_t *common_hal_bleio_characteristic_get_descriptors(bleio_characteristic_obj_t *self) {
if (self->descriptor_list == NULL) {
return mp_const_empty_tuple;
Expand Down Expand Up @@ -245,21 +259,7 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
}

self->current_value_len = bufinfo->len;
// If we've already allocated an internal buffer or the provided buffer
// is on the heap, then copy into the internal buffer.
if (self->current_value_alloc > 0 || gc_nbytes(bufinfo->buf) > 0) {
if (self->current_value_alloc < bufinfo->len) {
self->current_value = m_realloc(self->current_value, bufinfo->len);
// Get the number of bytes from the heap because it may be more
// than the len due to gc block size.
self->current_value_alloc = gc_nbytes(self->current_value);
}
memcpy(self->current_value, bufinfo->buf, bufinfo->len);
} else {
// Otherwise, use the provided buffer to delay any heap allocation.
self->current_value = bufinfo->buf;
self->current_value_alloc = 0;
}
memcpy(self->current_value, bufinfo->buf, self->current_value_len);

ble_gatts_chr_updated(self->handle);
}
Expand Down
1 change: 1 addition & 0 deletions ports/espressif/common-hal/_bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ void bleio_characteristic_buffer_extend(bleio_characteristic_buffer_obj_t *self,
for (uint16_t i = 0; i < len; i++) {
if (data[i] == mp_interrupt_char) {
mp_sched_keyboard_interrupt();
ringbuf_clear(&self->ringbuf);
} else {
ringbuf_put(&self->ringbuf, data[i]);
}
Expand Down
1 change: 1 addition & 0 deletions ports/espressif/common-hal/_bleio/PacketBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) {
return;
}
bleio_characteristic_clear_observer(self->characteristic);
self->characteristic = NULL;
ble_event_remove_handler(packet_buffer_on_ble_client_evt, self);
ringbuf_deinit(&self->ringbuf);
}
1 change: 1 addition & 0 deletions ports/espressif/supervisor/usb_serial_jtag.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ static void _copy_out_of_fifo(void) {
for (size_t i = 0; i < len; ++i) {
if (rx_buf[i] == mp_interrupt_char) {
mp_sched_keyboard_interrupt();
ringbuf_clear(&ringbuf);
} else {
ringbuf_put(&ringbuf, rx_buf[i]);
}
Expand Down
13 changes: 13 additions & 0 deletions ports/nordic/common-hal/_bleio/Characteristic.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
}
}

bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self) {
return self->handle == BLE_GATT_HANDLE_INVALID;
}

void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) {
if (common_hal_bleio_characteristic_deinited(self)) {
return;
}
self->handle = BLE_GATT_HANDLE_INVALID;
// TODO: Can we remove this from the soft device? Right now we assume the
// reset clears things.
}

mp_obj_tuple_t *common_hal_bleio_characteristic_get_descriptors(bleio_characteristic_obj_t *self) {
if (self->descriptor_list == NULL) {
return mp_const_empty_tuple;
Expand Down
1 change: 1 addition & 0 deletions ports/nordic/common-hal/_bleio/CharacteristicBuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static void write_to_ringbuf(bleio_characteristic_buffer_obj_t *self, uint8_t *d
for (uint16_t i = 0; i < len; i++) {
if (data[i] == mp_interrupt_char) {
mp_sched_keyboard_interrupt();
ringbuf_clear(&self->ringbuf);
} else {
ringbuf_put(&self->ringbuf, data[i]);
}
Expand Down
7 changes: 7 additions & 0 deletions ports/silabs/common-hal/_bleio/Characteristic.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ void common_hal_bleio_characteristic_construct(
}
}

bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self) {
return false;
}

void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) {
}

// A tuple of Descriptor that describe this characteristic
mp_obj_tuple_t *common_hal_bleio_characteristic_get_descriptors(
bleio_characteristic_obj_t *self) {
Expand Down
7 changes: 2 additions & 5 deletions py/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -759,13 +759,10 @@ void gc_info(gc_info_t *info) {
GC_EXIT();
}

// CIRCUITPY-CHANGE
// CIRCUITPY-CHANGE: C code may be used when the VM heap isn't active. This
// allows that code to test if it is. It can use the outer pool if needed.
bool gc_alloc_possible(void) {
#if MICROPY_GC_SPLIT_HEAP
return MP_STATE_MEM(gc_last_free_area) != 0;
#else
return MP_STATE_MEM(area).gc_pool_start != 0;
#endif
}

void *gc_alloc(size_t n_bytes, unsigned int alloc_flags) {
Expand Down
28 changes: 28 additions & 0 deletions shared-bindings/_bleio/Characteristic.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "shared-bindings/_bleio/Characteristic.h"
#include "shared-bindings/_bleio/Service.h"
#include "shared-bindings/_bleio/UUID.h"
#include "shared-bindings/util.h"


//| class Characteristic:
//| """Stores information about a BLE service characteristic and allows reading
Expand Down Expand Up @@ -137,14 +139,29 @@ static mp_obj_t bleio_characteristic_add_to_service(size_t n_args, const mp_obj_
static MP_DEFINE_CONST_FUN_OBJ_KW(bleio_characteristic_add_to_service_fun_obj, 1, bleio_characteristic_add_to_service);
static MP_DEFINE_CONST_CLASSMETHOD_OBJ(bleio_characteristic_add_to_service_obj, MP_ROM_PTR(&bleio_characteristic_add_to_service_fun_obj));

//| def deinit(self) -> None:
//| """Deinitialises the Characteristic and releases any hardware resources for reuse."""
//| ...
static mp_obj_t bleio_characteristic_deinit(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
common_hal_bleio_characteristic_deinit(self);
return mp_const_none;
}
static MP_DEFINE_CONST_FUN_OBJ_1(bleio_characteristic_deinit_obj, bleio_characteristic_deinit);

static void check_for_deinit(bleio_characteristic_obj_t *self) {
if (common_hal_bleio_characteristic_deinited(self)) {
raise_deinited_error();
}
}

//| properties: int
//| """An int bitmask representing which properties are set, specified as bitwise or'ing of
//| of these possible values.
//| `BROADCAST`, `INDICATE`, `NOTIFY`, `READ`, `WRITE`, `WRITE_NO_RESPONSE`."""
static mp_obj_t bleio_characteristic_get_properties(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

return MP_OBJ_NEW_SMALL_INT(common_hal_bleio_characteristic_get_properties(self));
}
Expand All @@ -159,6 +176,7 @@ MP_PROPERTY_GETTER(bleio_characteristic_properties_obj,
//| Will be ``None`` if the 128-bit UUID for this characteristic is not known."""
static mp_obj_t bleio_characteristic_get_uuid(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

bleio_uuid_obj_t *uuid = common_hal_bleio_characteristic_get_uuid(self);
return uuid ? MP_OBJ_FROM_PTR(uuid) : mp_const_none;
Expand All @@ -172,6 +190,7 @@ MP_PROPERTY_GETTER(bleio_characteristic_uuid_obj,
//| """The value of this characteristic."""
static mp_obj_t bleio_characteristic_get_value(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

uint8_t temp[512];
size_t actual_len = common_hal_bleio_characteristic_get_value(self, temp, sizeof(temp));
Expand All @@ -181,6 +200,7 @@ static MP_DEFINE_CONST_FUN_OBJ_1(bleio_characteristic_get_value_obj, bleio_chara

static mp_obj_t bleio_characteristic_set_value(mp_obj_t self_in, mp_obj_t value_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

mp_buffer_info_t bufinfo;
mp_get_buffer_raise(value_in, &bufinfo, MP_BUFFER_READ);
Expand All @@ -199,6 +219,7 @@ MP_PROPERTY_GETSET(bleio_characteristic_value_obj,
//| """The max length of this characteristic."""
static mp_obj_t bleio_characteristic_get_max_length(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

return MP_OBJ_NEW_SMALL_INT(common_hal_bleio_characteristic_get_max_length(self));
}
Expand All @@ -211,6 +232,8 @@ MP_PROPERTY_GETTER(bleio_characteristic_max_length_obj,
//| """A tuple of :py:class:`Descriptor` objects related to this characteristic. (read-only)"""
static mp_obj_t bleio_characteristic_get_descriptors(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

// Return list as a tuple so user won't be able to change it.
return MP_OBJ_FROM_PTR(common_hal_bleio_characteristic_get_descriptors(self));
}
Expand All @@ -224,6 +247,7 @@ MP_PROPERTY_GETTER(bleio_characteristic_descriptors_obj,
//| """The Service this Characteristic is a part of."""
static mp_obj_t bleio_characteristic_get_service(mp_obj_t self_in) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
check_for_deinit(self);

return common_hal_bleio_characteristic_get_service(self);
}
Expand All @@ -241,6 +265,7 @@ MP_PROPERTY_GETTER(bleio_characteristic_service_obj,
//| ...
static mp_obj_t bleio_characteristic_set_cccd(mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
check_for_deinit(self);

enum { ARG_notify, ARG_indicate };
static const mp_arg_t allowed_args[] = {
Expand All @@ -258,6 +283,9 @@ static mp_obj_t bleio_characteristic_set_cccd(mp_uint_t n_args, const mp_obj_t *
static MP_DEFINE_CONST_FUN_OBJ_KW(bleio_characteristic_set_cccd_obj, 1, bleio_characteristic_set_cccd);

static const mp_rom_map_elem_t bleio_characteristic_locals_dict_table[] = {
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&bleio_characteristic_deinit_obj) },
{ MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&bleio_characteristic_deinit_obj) },

{ MP_ROM_QSTR(MP_QSTR_add_to_service), MP_ROM_PTR(&bleio_characteristic_add_to_service_obj) },
{ MP_ROM_QSTR(MP_QSTR_descriptors), MP_ROM_PTR(&bleio_characteristic_descriptors_obj) },
{ MP_ROM_QSTR(MP_QSTR_properties), MP_ROM_PTR(&bleio_characteristic_properties_obj) },
Expand Down
2 changes: 2 additions & 0 deletions shared-bindings/_bleio/Characteristic.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ extern size_t common_hal_bleio_characteristic_get_max_length(bleio_characteristi
extern size_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *self, uint8_t *buf, size_t len);
extern void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self, bleio_descriptor_obj_t *descriptor);
extern void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_service_obj_t *service, uint16_t handle, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_buffer_info_t *initial_value_bufinfo, const char *user_description);
extern bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self);
extern void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self);
extern void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self, bool notify, bool indicate);
extern void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, mp_buffer_info_t *bufinfo);
4 changes: 4 additions & 0 deletions supervisor/shared/bluetooth/bluetooth.c
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ void supervisor_stop_bluetooth(void) {

ble_started = false;

#if CIRCUITPY_BLE_FILE_SERVICE
supervisor_stop_bluetooth_file_transfer();
#endif

#if CIRCUITPY_SERIAL_BLE
supervisor_stop_bluetooth_serial();
#endif
Expand Down
7 changes: 7 additions & 0 deletions supervisor/shared/bluetooth/file_transfer.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ void supervisor_start_bluetooth_file_transfer(void) {
&static_handler_entry);
}

void supervisor_stop_bluetooth_file_transfer(void) {
common_hal_bleio_packet_buffer_deinit(&_transfer_packet_buffer);
common_hal_bleio_characteristic_deinit(&supervisor_ble_transfer_characteristic);
common_hal_bleio_characteristic_deinit(&supervisor_ble_version_characteristic);
common_hal_bleio_service_deinit(&supervisor_ble_service);
}

#define COMMAND_SIZE 1024

#define ANY_COMMAND 0x00
Expand Down
4 changes: 3 additions & 1 deletion supervisor/shared/bluetooth/file_transfer.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include <stdbool.h>

void supervisor_bluetooth_file_transfer_background(void);
void supervisor_start_bluetooth_file_transfer(void);
void supervisor_stop_bluetooth_file_transfer(void);

void supervisor_bluetooth_file_transfer_background(void);
void supervisor_bluetooth_file_transfer_disconnected(void);
5 changes: 5 additions & 0 deletions supervisor/shared/bluetooth/serial.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ void supervisor_stop_bluetooth_serial(void) {
return;
}
common_hal_bleio_packet_buffer_flush(&_tx_packet_buffer);
common_hal_bleio_packet_buffer_deinit(&_tx_packet_buffer);
common_hal_bleio_characteristic_deinit(&supervisor_ble_circuitpython_rx_characteristic);
common_hal_bleio_characteristic_deinit(&supervisor_ble_circuitpython_tx_characteristic);
common_hal_bleio_characteristic_deinit(&supervisor_ble_circuitpython_version_characteristic);
common_hal_bleio_service_deinit(&supervisor_ble_circuitpython_service);
}

bool ble_serial_connected(void) {
Expand Down
3 changes: 2 additions & 1 deletion supervisor/shared/usb/host_keyboard.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,9 @@ static void send_bufn_core(const char *buf, size_t n) {
for (; n--; buf++) {
int code = *buf;
if (code == mp_interrupt_char) {
ringbuf_clear(&_incoming_ringbuf);
mp_sched_keyboard_interrupt();
return;
continue;
}
if (ringbuf_num_empty(&_incoming_ringbuf) == 0) {
// Drop on the floor
Expand Down
1 change: 1 addition & 0 deletions supervisor/shared/web_workflow/websocket.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ void websocket_background(void) {
while (ringbuf_num_empty(&_incoming_ringbuf) > 0 &&
_read_next_payload_byte(&c)) {
if (c == mp_interrupt_char) {
ringbuf_clear(&_incoming_ringbuf);
mp_sched_keyboard_interrupt();
continue;
}
Expand Down

0 comments on commit fad755d

Please sign in to comment.