Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid malloc/free while in HCI callbacks #2219

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/hidmaster.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ BTDeviceInfo Class
------------------

The ``BluetoothHCI`` class implements a scanning function for classic
and BLE devices and can return a ``std::list`` of discovered devices to an application.
and BLE devices and can return a ``std::vector`` of discovered devices to an application.
Iterate over the list using any of the STL iteration methods (i.e. ``for (auto e : list)``).
The elements of this list are ``BTDeviceInfo`` objects which have the following
member functions:
Expand Down Expand Up @@ -179,7 +179,7 @@ bool BluetoothHIDMaster::hciRunning()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Returns if the Bluetooth stack has passed the initial HCI start up phase. Until this returns ``true`` no Bluetooth operations can be performed.

std::list<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async)
std::vector<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Passes through the ``BluetoothHCI::scan()`` function to manually scan for a list of nearby devices. If you want to connect to the first found device, this is not needed.

Expand Down
2 changes: 1 addition & 1 deletion libraries/BluetoothAudio/src/A2DPSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class A2DPSource : public Stream {

bool begin();

std::list<BTDeviceInfo> scan(uint32_t mask = BluetoothHCI::speaker_cod, int scanTimeSec = 5, bool async = false) {
std::vector<BTDeviceInfo> scan(uint32_t mask = BluetoothHCI::speaker_cod, int scanTimeSec = 5, bool async = false) {
return _hci.scan(mask, scanTimeSec, async);
}

Expand Down
31 changes: 16 additions & 15 deletions libraries/BluetoothHCI/src/BluetoothDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,59 +24,60 @@

class BTDeviceInfo {
public:
// Classic Bluetooth Device
BTDeviceInfo(uint32_t dc, const uint8_t addr[6], int rssi, const char *name) {
_deviceClass = dc;
memcpy(_address, addr, sizeof(_address));
_addressType = -1;
sprintf(_addressString, "%02x:%02x:%02x:%02x:%02x:%02x", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
_rssi = rssi;
_name = strdup(name);
strncpy(_name, name, sizeof(_name));
_name[sizeof(_name) - 1] = 0;
}

// Bluetooth BLE Device
BTDeviceInfo(uint32_t dc, const uint8_t addr[6], int addressType, int rssi, const char *name, size_t nameLen) {
_deviceClass = dc;
memcpy(_address, addr, sizeof(_address));
sprintf(_addressString, "%02x:%02x:%02x:%02x:%02x:%02x", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
_addressType = addressType;
_rssi = rssi;
_name = (char *)malloc(nameLen + 1);
memcpy(_name, name, nameLen);
_name[nameLen] = 0;
}
// Copy constructor to ensure we deep-copy the string
BTDeviceInfo(const BTDeviceInfo &b) {
_deviceClass = b._deviceClass;
memcpy(_address, b._address, sizeof(_address));
_addressType = b._addressType;
memcpy(_addressString, b._addressString, sizeof(_addressString));
_rssi = b._rssi;
_name = strdup(b._name);
memcpy(_name, name, std::min(nameLen, sizeof(_name)));
_name[std::min(nameLen, sizeof(_name) - 1)] = 0;
}

~BTDeviceInfo() {
free(_name);
}

uint32_t deviceClass() {
return _deviceClass;
}

const uint8_t *address() {
return _address;
}

const char *addressString() {
return _addressString;
}

int rssi() {
return _rssi;
}

const char *name() {
return _name;
}

int addressType() {
return _addressType;
}

private:
uint32_t _deviceClass;
uint8_t _address[6];
int _addressType;
char _addressString[18];
int8_t _rssi;
char *_name;
char _name[241];
};
18 changes: 14 additions & 4 deletions libraries/BluetoothHCI/src/BluetoothHCI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ bool BluetoothHCI::running() {
return _hciRunning;
}

std::list<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool async) {
std::vector<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool async) {
_scanMask = mask;
_btdList.reserve(MAX_DEVICES_TO_DISCOVER);
_btdList.clear();
if (!_running) {
return _btdList;
Expand Down Expand Up @@ -135,8 +136,9 @@ std::list<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool
return _btdList;
}

std::list<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
std::vector<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
_scanMask = uuid;
_btdList.reserve(MAX_DEVICES_TO_DISCOVER);
_btdList.clear();
if (!_running) {
return _btdList;
Expand Down Expand Up @@ -168,6 +170,10 @@ std::list<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
return _btdList;
}

void BluetoothHCI::scanFree() {
_btdList.clear();
_btdList.shrink_to_fit();
}

void BluetoothHCI::parse_advertisement_data(uint8_t *packet) {
bd_addr_t address;
Expand Down Expand Up @@ -298,7 +304,9 @@ void BluetoothHCI::parse_advertisement_data(uint8_t *packet) {
}
if (!seen) {
BTDeviceInfo btd(uuid, address, address_type, rssi, nameptr ? nameptr : "", nameptr ? namelen : 0);
_btdList.push_back(btd);
if (_btdList.size() < MAX_DEVICES_TO_DISCOVER) {
_btdList.push_back(btd);
}
}
}
}
Expand Down Expand Up @@ -394,7 +402,9 @@ void BluetoothHCI::hci_packet_handler(uint8_t packet_type, uint16_t channel, uin
}
}
BTDeviceInfo btd(cod, address, rssi, name);
_btdList.push_back(btd);
if (_btdList.size() < MAX_DEVICES_TO_DISCOVER) {
_btdList.push_back(btd);
}
}
break;
case GAP_EVENT_INQUIRY_COMPLETE:
Expand Down
14 changes: 10 additions & 4 deletions libraries/BluetoothHCI/src/BluetoothHCI.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ class BluetoothHCI {

static const uint32_t speaker_cod = 0x200000 | 0x040000 | 0x000400; // Service Class: Rendering | Audio, Major Device Class: Audio
static const uint32_t any_cod = 0;
std::list<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
std::vector<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
bool scanAsyncDone();
std::list<BTDeviceInfo> scanAsyncResult();
std::vector<BTDeviceInfo> scanAsyncResult();

std::list<BTDeviceInfo> scanBLE(uint32_t uuid, int scanTimeSec = 5);
std::vector<BTDeviceInfo> scanBLE(uint32_t uuid, int scanTimeSec = 5);

void scanFree(); // Free allocated scan buffers

friend class BluetoothHIDMaster;

Expand All @@ -60,7 +62,11 @@ class BluetoothHCI {
btstack_packet_callback_registration_t hci_event_callback_registration;
volatile bool _hciRunning = false;
uint32_t _scanMask;
std::list<BTDeviceInfo> _btdList;
// Use a .reserve()'d vector to avoid any memory allocations in the
// HCI callback, since the callback happens at IRQ time. Any more
// elements than MAX_DEVICES_TO_DISCOVER will be thrown out
enum { MAX_DEVICES_TO_DISCOVER = 32 };
std::vector<BTDeviceInfo> _btdList;
volatile bool _scanning = false;
bool _running = false;

Expand Down
4 changes: 2 additions & 2 deletions libraries/BluetoothHIDMaster/src/BluetoothHIDMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ void BluetoothHIDMaster::onJoystick(void (*cb)(void *, int, int, int, int, uint8
_joystickData = cbData;
}

std::list<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async) {
std::vector<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async) {
return _hci.scan(mask, scanTimeSec, async);
}

bool BluetoothHIDMaster::scanAsyncDone() {
return _hci.scanAsyncDone();
}
std::list<BTDeviceInfo> BluetoothHIDMaster::scanAsyncResult() {
std::vector<BTDeviceInfo> BluetoothHIDMaster::scanAsyncResult() {
return _hci.scanAsyncResult();
}

Expand Down
4 changes: 2 additions & 2 deletions libraries/BluetoothHIDMaster/src/BluetoothHIDMaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ class BluetoothHIDMaster {
static const uint32_t mouse_cod = 0x2540;
static const uint32_t joystick_cod = 0x2508;
static const uint32_t any_cod = 0;
std::list<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
std::vector<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
bool scanAsyncDone();
std::list<BTDeviceInfo> scanAsyncResult();
std::vector<BTDeviceInfo> scanAsyncResult();

bool connect(const uint8_t *addr);
bool connectKeyboard();
Expand Down