nimble software update WIP builds

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 <CRC32.h>
 #include <Update.h>
 
-#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()

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()
 {

src/nimble/BluetoothUtil.h

@@ -1,36 +1,10 @@
 #pragma once
 
+#include <Arduino.h>
 #include <functional>
 
-#include "SimpleAllocator.h"
-#include <Arduino.h>
-#include <BLEDevice.h>
-#include <BLEServer.h>
-#include <BLEUtils.h>
-
-#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<void(uint32_t pass_key)>;
@@ -40,4 +14,17 @@ using StopBluetoothPinScreenCallback = std::function<void(void)>;
 void updateBatteryLevel(uint8_t level);
 void deinitBLE();
 void loopBLE();
-void reinitBluetooth();
+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);

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 <Arduino.h>
 
 // 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);
 }

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
 {
     /**

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 <Arduino.h>
 
 #ifdef __cplusplus
 extern "C" {