From b0e2c81666b9441b60d8a88af7c5e15fb2702c5e Mon Sep 17 00:00:00 2001 From: geeksville Date: Thu, 23 Jul 2020 15:34:54 -0700 Subject: [PATCH] nimble software update WIP builds --- src/esp32/BluetoothSoftwareUpdate.cpp | 90 +++++++++++++++++---------- src/nimble/BluetoothUtil.cpp | 66 ++++++++++++++++++++ src/nimble/BluetoothUtil.h | 47 +++++--------- src/nimble/NimbleBluetoothAPI.cpp | 11 ++-- src/nimble/NimbleBluetoothAPI.h | 3 - src/nimble/NimbleDefs.h | 7 +++ 6 files changed, 150 insertions(+), 74 deletions(-) diff --git a/src/esp32/BluetoothSoftwareUpdate.cpp b/src/esp32/BluetoothSoftwareUpdate.cpp index d970f1c03..5aa073a11 100644 --- a/src/esp32/BluetoothSoftwareUpdate.cpp +++ b/src/esp32/BluetoothSoftwareUpdate.cpp @@ -3,15 +3,15 @@ #include "../concurrency/LockGuard.h" #include "../timing.h" #include "BluetoothSoftwareUpdate.h" +#include "PowerFSM.h" #include "RadioLibInterface.h" #include "configuration.h" +#include "nimble/BluetoothUtil.h" #include #include -#include "CallbackCharacteristic.h" - -int16_t updateResultHandle; +int16_t updateResultHandle = -1; static CRC32 crc; static uint32_t rebootAtMsec = 0; // If not zero we will reboot at this time (used to reboot shortly after the update completes) @@ -21,57 +21,70 @@ static uint32_t updateExpectedSize, updateActualSize; static concurrency::Lock *updateLock; /// Handle writes & reads to total size -int totalSize_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) +int update_size_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) { concurrency::LockGuard g(updateLock); - // Check if there is enough to OTA Update - uint32_t len = getValue32(c, 0); - updateExpectedSize = len; - updateActualSize = 0; - crc.reset(); - bool canBegin = Update.begin(len); - DEBUG_MSG("Setting update size %u, result %d\n", len, canBegin); - if (!canBegin) { - // Indicate failure by forcing the size to 0 - uint32_t zero = 0; - c->setValue(zero); - } else { - // This totally breaks abstraction to up up into the app layer for this, but quick hack to make sure we only - // talk to one service during the sw update. - // DEBUG_MSG("FIXME, crufty shutdown of mesh bluetooth for sw update."); - // void stopMeshBluetoothService(); - // stopMeshBluetoothService(); - if (RadioLibInterface::instance) - RadioLibInterface::instance->sleep(); // FIXME, nasty hack - the RF95 ISR/SPI code on ESP32 can fail while we are - // writing flash - shut the radio off during updates + // Check if there is enough to OTA Update + chr_readwrite32le(&updateExpectedSize, ctxt, arg); + + if (ctxt->op == BLE_GATT_ACCESS_OP_WRITE_CHR && updateExpectedSize != 0) { + updateActualSize = 0; + crc.reset(); + bool canBegin = Update.begin(updateExpectedSize); + DEBUG_MSG("Setting update size %u, result %d\n", updateExpectedSize, canBegin); + if (!canBegin) { + // Indicate failure by forcing the size to 0 (client will read it back) + updateExpectedSize = 0; + } else { + // This totally breaks abstraction to up up into the app layer for this, but quick hack to make sure we only + // talk to one service during the sw update. + // DEBUG_MSG("FIXME, crufty shutdown of mesh bluetooth for sw update."); + // void stopMeshBluetoothService(); + // stopMeshBluetoothService(); + + if (RadioLibInterface::instance) + RadioLibInterface::instance->sleep(); // FIXME, nasty hack - the RF95 ISR/SPI code on ESP32 can fail while we are + // writing flash - shut the radio off during updates + } } + + return 0; } #define MAX_BLOCKSIZE 512 /// Handle writes to data -int totalSize_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) +int update_data_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) { concurrency::LockGuard g(updateLock); - std::string value = c->getValue(); - uint32_t len = value.length(); - assert(len <= MAX_BLOCKSIZE); + static uint8_t data[MAX_BLOCKSIZE]; // we temporarily copy here because I'm worried that a fast sender might be able overwrite srcbuf - memcpy(data, c->getData(), len); + + uint16_t len = 0; + + auto rc = ble_hs_mbuf_to_flat(ctxt->om, data, sizeof(data), &len); + assert(rc == 0); + // DEBUG_MSG("Writing %u\n", len); crc.update(data, len); Update.write(data, len); updateActualSize += len; powerFSM.trigger(EVENT_RECEIVED_TEXT_MSG); // Not exactly correct, but we want to force the device to not sleep now + + return 0; } +static uint8_t update_result; + /// Handle writes to crc32 -int totalSize_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) +int update_crc32_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) { concurrency::LockGuard g(updateLock); - uint32_t expectedCRC = getValue32(c, 0); + uint32_t expectedCRC = 0; + chr_readwrite32le(&expectedCRC, ctxt, arg); + uint32_t actualCRC = crc.finalize(); DEBUG_MSG("expected CRC %u\n", expectedCRC); @@ -97,9 +110,18 @@ int totalSize_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_ga if (RadioLibInterface::instance) RadioLibInterface::instance->startReceive(); // Resume radio - assert(resultC); - resultC->setValue(&result, 1); - resultC->notify(); + assert(updateResultHandle >= 0); + update_result = result; + DEBUG_MSG("BLE notify update result\n"); + auto res = ble_gattc_notify(curConnectionHandle, updateResultHandle); + assert(res == 0); + + return 0; +} + +int update_result_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) +{ + return chr_readwrite8(&update_result, sizeof(update_result), ctxt, arg); } void bluetoothRebootCheck() diff --git a/src/nimble/BluetoothUtil.cpp b/src/nimble/BluetoothUtil.cpp index 83386542d..29a7f22d0 100644 --- a/src/nimble/BluetoothUtil.cpp +++ b/src/nimble/BluetoothUtil.cpp @@ -365,6 +365,10 @@ void gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, void *arg) fromNumValHandle = ctxt->chr.val_handle; DEBUG_MSG("FromNum handle %d\n", fromNumValHandle); } + if (ctxt->chr.chr_def->uuid == &update_result_uuid.u) { + updateResultHandle = ctxt->chr.val_handle; + DEBUG_MSG("update result handle %d\n", updateResultHandle); + } break; case BLE_GATT_REGISTER_OP_DSC: @@ -377,6 +381,68 @@ void gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, void *arg) } } +/** + * A helper function that implements simple read and write handling for a uint32_t + * + * If a read, the provided value will be returned over bluetooth. If a write, the value from the received packet + * will be written into the variable. + */ +int chr_readwrite32le(uint32_t *v, struct ble_gatt_access_ctxt *ctxt, void *arg) +{ + uint8_t le[4]; + + if (ctxt->op == BLE_GATT_ACCESS_OP_READ_CHR) { + DEBUG_MSG("BLE reading a uint32\n"); + put_le32(le, *v); + auto rc = os_mbuf_append(ctxt->om, le, sizeof(le)); + assert(rc == 0); + } else if (ctxt->op == BLE_GATT_ACCESS_OP_WRITE_CHR) { + uint16_t len = 0; + + auto rc = ble_hs_mbuf_to_flat(ctxt->om, le, sizeof(le), &len); + assert(rc == 0); + if (len < sizeof(le)) { + DEBUG_MSG("Error: wrongsized write32\n"); + *v = 0; + } else { + *v = get_le32(le); + DEBUG_MSG("BLE writing a uint32\n"); + } + } else { + DEBUG_MSG("Unexpected readwrite32 op\n"); + return BLE_ATT_ERR_UNLIKELY; + } + + return 0; // success +} + +/** + * A helper for readwrite access to an array of bytes (with no endian conversion) + */ +int chr_readwrite8(uint8_t *v, size_t vlen, struct ble_gatt_access_ctxt *ctxt, void *arg) +{ + if (ctxt->op == BLE_GATT_ACCESS_OP_READ_CHR) { + DEBUG_MSG("BLE reading bytes\n"); + auto rc = os_mbuf_append(ctxt->om, v, vlen); + assert(rc == 0); + } else if (ctxt->op == BLE_GATT_ACCESS_OP_WRITE_CHR) { + uint16_t len = 0; + + auto rc = ble_hs_mbuf_to_flat(ctxt->om, v, vlen, &len); + assert(rc == 0); + if (len < vlen) + DEBUG_MSG("Error: wrongsized write\n"); + else { + DEBUG_MSG("BLE writing bytes\n"); + } + } else { + DEBUG_MSG("Unexpected readwrite8 op\n"); + return BLE_ATT_ERR_UNLIKELY; + } + + return 0; // success +} + // This routine is called multiple times, once each time we come back from sleep void reinitBluetooth() { diff --git a/src/nimble/BluetoothUtil.h b/src/nimble/BluetoothUtil.h index 8330afc47..7a1c62463 100644 --- a/src/nimble/BluetoothUtil.h +++ b/src/nimble/BluetoothUtil.h @@ -1,36 +1,10 @@ #pragma once +#include #include -#include "SimpleAllocator.h" -#include -#include -#include -#include - -#ifdef CONFIG_BLUEDROID_ENABLED - -// Help routine to add a description to any BLECharacteristic and add it to the service -void addWithDesc(BLEService *service, BLECharacteristic *c, const char *description); - -void dumpCharacteristic(BLECharacteristic *c); - -/** converting endianness pull out a 32 bit value */ -uint32_t getValue32(BLECharacteristic *c, uint32_t defaultValue); - -BLEServer *initBLE(StartBluetoothPinScreenCallback startBtPinScreen, StopBluetoothPinScreenCallback stopBtPinScreen, - std::string devName, std::string hwVendor, std::string swVersion, std::string hwVersion = ""); - -/// Add a characteristic that we will delete when we restart -BLECharacteristic *addBLECharacteristic(BLECharacteristic *c); - -/// Add a characteristic that we will delete when we restart -BLEDescriptor *addBLEDescriptor(BLEDescriptor *c); - -/// Any bluetooth objects you allocate _must_ come from this pool if you want to be able to call deinitBLE() -extern SimpleAllocator btPool; - -#endif +/// We only allow one BLE connection at a time +extern int16_t curConnectionHandle; // TODO(girts): create a class for the bluetooth utils helpers? using StartBluetoothPinScreenCallback = std::function; @@ -40,4 +14,17 @@ using StopBluetoothPinScreenCallback = std::function; void updateBatteryLevel(uint8_t level); void deinitBLE(); void loopBLE(); -void reinitBluetooth(); \ No newline at end of file +void reinitBluetooth(); + +/** + * A helper function that implements simple read and write handling for a uint32_t + * + * If a read, the provided value will be returned over bluetooth. If a write, the value from the received packet + * will be written into the variable. + */ +int chr_readwrite32le(uint32_t *v, struct ble_gatt_access_ctxt *ctxt, void *arg); + +/** + * A helper for readwrite access to an array of bytes (with no endian conversion) + */ +int chr_readwrite8(uint8_t *v, size_t vlen, struct ble_gatt_access_ctxt *ctxt, void *arg); \ No newline at end of file diff --git a/src/nimble/NimbleBluetoothAPI.cpp b/src/nimble/NimbleBluetoothAPI.cpp index 0de53e814..a093cbf5d 100644 --- a/src/nimble/NimbleBluetoothAPI.cpp +++ b/src/nimble/NimbleBluetoothAPI.cpp @@ -1,11 +1,13 @@ #include "NimbleBluetoothAPI.h" #include "PhoneAPI.h" #include "configuration.h" +#include "nimble/BluetoothUtil.h" #include "nimble/NimbleDefs.h" +#include // This scratch buffer is used for various bluetooth reads/writes - but it is safe because only one bt operation can be in // proccess at once -static uint8_t trBytes[max(FromRadio_size, ToRadio_size)]; +static uint8_t trBytes[FromRadio_size < ToRadio_size ? ToRadio_size : FromRadio_size]; static uint32_t fromNum; uint16_t fromNumValHandle; @@ -60,10 +62,5 @@ int fromradio_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_ga int fromnum_callback(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) { - // DEBUG_MSG("BLE fromNum called\n"); - auto rc = os_mbuf_append(ctxt->om, &fromNum, - sizeof(fromNum)); // FIXME - once we report real numbers we will need to consider endianness - assert(rc == 0); - - return 0; // success + return chr_readwrite32le(&fromNum, ctxt, arg); } diff --git a/src/nimble/NimbleBluetoothAPI.h b/src/nimble/NimbleBluetoothAPI.h index 419bf0e0a..5fa66cc24 100644 --- a/src/nimble/NimbleBluetoothAPI.h +++ b/src/nimble/NimbleBluetoothAPI.h @@ -4,9 +4,6 @@ extern uint16_t fromNumValHandle; -/// We only allow one BLE connection at a time -extern int16_t curConnectionHandle; - class BluetoothPhoneAPI : public PhoneAPI { /** diff --git a/src/nimble/NimbleDefs.h b/src/nimble/NimbleDefs.h index 8af85c4cb..6b1db22e5 100644 --- a/src/nimble/NimbleDefs.h +++ b/src/nimble/NimbleDefs.h @@ -1,10 +1,17 @@ #pragma once +// Keep nimble #defs from messing up the build +#ifndef max +#define max max +#define min min +#endif + #include "esp_nimble_hci.h" #include "host/ble_hs.h" #include "host/ble_uuid.h" #include "nimble/nimble_port.h" #include "nimble/nimble_port_freertos.h" +#include #ifdef __cplusplus extern "C" {