arduino-libraries / ArduinoBLE

ArduinoBLE library for Arduino
GNU Lesser General Public License v2.1
291 stars 198 forks source link

Arduino Nano 33 IoT large transfer fix (MCU hang bug) #316

Open coderand opened 11 months ago

coderand commented 11 months ago

uint16_t handle = *(uint16_t*)data

hangs Arduino Nano 33 IoT in cases when data pointer is unaligned. This happens when receiving large packets of BLEStringCharacteristic.

Cortex®-M0 does not support unaligned memory access.

This doesn't not happen when setting such characteristics from iPhone (iOS) or Android devices, but happens 100% of the time on PC using either Bleak (Python) or QT6 libraries.

In those cases data pointer at the time of this call is not two bytes (sizeof(uint16_t)) aligned and dereferencing such pointer hangs the MCU.

To resolve the issue we copy that uint16_t handle byte by byte without monkeying with ORs, Shifts or other binary operations to avoid endianness issues on other platforms.

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

coderand commented 11 months ago

Alternatively, instead of uint16_t handle = *(uint16_t*)data; it could be: uint16_t handle; memcpy(&handle, data, sizeof(handle)); that's a bit more consistent with the rest of the coding style there.

Apart from that, it looks like there could be a few more potential bugs of this sort but I have no way to test or confirm those. The most obvious one would probably be in ATTClass::readOrReadBlobReq method, where uint8_t data[] is dereferenced as *(uint16_t *)data as well.

coderand commented 11 months ago

This mainly happens when setting long values such as "012345678901234567890123456789" on Windows 10 using pretty much any BLE tool or API. Writing characteristic with "Microsoft LE Expolorer", using Python/Bleak or Qt6

The Arduino NANO 33 IoT Sketch is:

#include <ArduinoBLE.h>
#include <utility/ATT.h>
#include <utility/GATT.h>

BLEService testService("180B");
BLEStringCharacteristic testCharacteristic("00000003-0000-1000-8000-00805f9b34fb", BLEWrite | BLERead | BLENotify, 185);

void setup() {
  Serial.begin(9600);
  while (!Serial);

  ATT.setMaxMtu(185);

  if (!BLE.begin()) {
    while (1);
  }

  GATT.setDeviceName("IoT-Hang");
  BLE.setLocalName("IoT-Hang");
  BLE.setDeviceName("IoT-Hang");
  BLE.setAdvertisedService(testService);
  testService.addCharacteristic(testCharacteristic);
  BLE.addService(testService);
  BLE.advertise();
}

void loop() {
  BLEDevice central = BLE.central();

  if (central && central.connected()) {
    if (testCharacteristic.written()) {
      Serial.println(testCharacteristic.value());
    }
  }
}