Nimble sleep almost works

docs/software/TODO.md

@@ -9,6 +9,8 @@ Nimble tasks:
 - check BLE handle stability
 - started RPA long test, jul 22 6pm
 - implement nimble software update api
+- implement nimble battery level service
+- implement device info service remaining fields (hw version etc)
 
 * update protocol description per cyclomies email thread
 * update faq with antennas https://meshtastic.discourse.group/t/range-test-ideas-requested/738/2
@@ -34,6 +36,8 @@ Items to complete before 1.0.
 
 Items after the first final candidate release.
 
+- Turn on RPA addresses for the device side in Nimble
+- Try to teardown less of the Nimble protocol stack across sleep
 - dynamic frequency scaling could save a lot of power on ESP32, but it seems to corrupt uart (even with ref_tick set correctly)
 - Change back to using a fixed sized MemoryPool rather than MemoryDynamic (see bug #149)
 - scan to find channels with low background noise? (Use CAD mode of the RF95 to automatically find low noise channels)

src/mesh/NodeDB.cpp

@@ -103,7 +103,7 @@ void NodeDB::resetRadioConfig()
     crypto->setKey(channelSettings.psk.size, channelSettings.psk.bytes);
 
     // temp hack for quicker testing
-    // devicestate.no_save = true;
+    devicestate.no_save = true;
     if (devicestate.no_save) {
         DEBUG_MSG("***** DEVELOPMENT MODE - DO NOT RELEASE *****\n");
 
@@ -356,7 +356,7 @@ void NodeDB::updateFrom(const MeshPacket &mp)
             info->position.time = oldtime;
             info->has_position = true;
             updateGUIforNode = info;
-            notifyObservers(true);  //Force an update whether or not our node counts have changed
+            notifyObservers(true); // Force an update whether or not our node counts have changed
             break;
         }
 
@@ -371,7 +371,7 @@ void NodeDB::updateFrom(const MeshPacket &mp)
                     devicestate.has_rx_text_message = true;
                     updateTextMessage = true;
                     powerFSM.trigger(EVENT_RECEIVED_TEXT_MSG);
-                    notifyObservers(true);  //Force an update whether or not our node counts have changed
+                    notifyObservers(true); // Force an update whether or not our node counts have changed
                 }
             }
             break;
@@ -390,7 +390,7 @@ void NodeDB::updateFrom(const MeshPacket &mp)
             if (changed) {
                 updateGUIforNode = info;
                 powerFSM.trigger(EVENT_NODEDB_UPDATED);
-                notifyObservers(true);  //Force an update whether or not our node counts have changed
+                notifyObservers(true); // Force an update whether or not our node counts have changed
 
                 // Not really needed - we will save anyways when we go to sleep
                 // We just changed something important about the user, store our DB
@@ -400,7 +400,7 @@ void NodeDB::updateFrom(const MeshPacket &mp)
         }
 
         default: {
-            notifyObservers();  //If the node counts have changed, notify observers
+            notifyObservers(); // If the node counts have changed, notify observers
         }
         }
     }

src/nimble/BluetoothUtil.cpp

@@ -6,107 +6,7 @@
 #include <Update.h>
 #include <esp_gatt_defs.h>
 
-#ifdef CONFIG_BLUEDROID_ENABLED
+#if 0
-
-SimpleAllocator btPool;
-
-bool _BLEClientConnected = false;
-
-class MyServerCallbacks : public BLEServerCallbacks
-{
-    void onConnect(BLEServer *pServer) { _BLEClientConnected = true; };
-
-    void onDisconnect(BLEServer *pServer) { _BLEClientConnected = false; }
-};
-
-#define MAX_DESCRIPTORS 32
-#define MAX_CHARACTERISTICS 32
-
-static BLECharacteristic *chars[MAX_CHARACTERISTICS];
-static size_t numChars;
-static BLEDescriptor *descs[MAX_DESCRIPTORS];
-static size_t numDescs;
-
-/// Add a characteristic that we will delete when we restart
-BLECharacteristic *addBLECharacteristic(BLECharacteristic *c)
-{
-    assert(numChars < MAX_CHARACTERISTICS);
-    chars[numChars++] = c;
-    return c;
-}
-
-/// Add a characteristic that we will delete when we restart
-BLEDescriptor *addBLEDescriptor(BLEDescriptor *c)
-{
-    assert(numDescs < MAX_DESCRIPTORS);
-    descs[numDescs++] = c;
-
-    return c;
-}
-
-// Help routine to add a description to any BLECharacteristic and add it to the service
-// We default to require an encrypted BOND for all these these characterstics
-void addWithDesc(BLEService *service, BLECharacteristic *c, const char *description)
-{
-    c->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED);
-
-    BLEDescriptor *desc = new BLEDescriptor(BLEUUID((uint16_t)ESP_GATT_UUID_CHAR_DESCRIPTION), strlen(description) + 1);
-    assert(desc);
-    desc->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED);
-    desc->setValue(description);
-    c->addDescriptor(desc);
-    service->addCharacteristic(c);
-    addBLECharacteristic(c);
-    addBLEDescriptor(desc);
-}
-
-/**
- * Create standard device info service
- **/
-BLEService *createDeviceInfomationService(BLEServer *server, std::string hwVendor, std::string swVersion,
-                                          std::string hwVersion = "")
-{
-    BLEService *deviceInfoService = server->createService(BLEUUID((uint16_t)ESP_GATT_UUID_DEVICE_INFO_SVC));
-
-    BLECharacteristic *swC =
-        new BLECharacteristic(BLEUUID((uint16_t)ESP_GATT_UUID_SW_VERSION_STR), BLECharacteristic::PROPERTY_READ);
-    BLECharacteristic *mfC = new BLECharacteristic(BLEUUID((uint16_t)ESP_GATT_UUID_MANU_NAME), BLECharacteristic::PROPERTY_READ);
-    // BLECharacteristic SerialNumberCharacteristic(BLEUUID((uint16_t) ESP_GATT_UUID_SERIAL_NUMBER_STR),
-    // BLECharacteristic::PROPERTY_READ);
-
-    /*
-           * Mandatory characteristic for device info service?
-
-          BLECharacteristic *m_pnpCharacteristic = m_deviceInfoService->createCharacteristic(ESP_GATT_UUID_PNP_ID,
-      BLECharacteristic::PROPERTY_READ);
-
-      uint8_t sig, uint16_t vid, uint16_t pid, uint16_t version;
-          uint8_t pnp[] = { sig, (uint8_t) (vid >> 8), (uint8_t) vid, (uint8_t) (pid >> 8), (uint8_t) pid, (uint8_t) (version >>
-      8), (uint8_t) version }; m_pnpCharacteristic->setValue(pnp, sizeof(pnp));
-      */
-    swC->setValue(swVersion);
-    deviceInfoService->addCharacteristic(addBLECharacteristic(swC));
-    mfC->setValue(hwVendor);
-    deviceInfoService->addCharacteristic(addBLECharacteristic(mfC));
-    if (!hwVersion.empty()) {
-        BLECharacteristic *hwvC =
-            new BLECharacteristic(BLEUUID((uint16_t)ESP_GATT_UUID_HW_VERSION_STR), BLECharacteristic::PROPERTY_READ);
-        hwvC->setValue(hwVersion);
-        deviceInfoService->addCharacteristic(addBLECharacteristic(hwvC));
-    }
-    // SerialNumberCharacteristic.setValue("FIXME");
-    // deviceInfoService->addCharacteristic(&SerialNumberCharacteristic);
-
-    // m_manufacturerCharacteristic = m_deviceInfoService->createCharacteristic((uint16_t) 0x2a29,
-    // BLECharacteristic::PROPERTY_READ); m_manufacturerCharacteristic->setValue(name);
-
-    /* add these later?
-      ESP_GATT_UUID_SYSTEM_ID
-      */
-
-    // caller must call service->start();
-    return deviceInfoService;
-}
 
 static BLECharacteristic *batteryLevelC;
 
@@ -144,185 +44,13 @@ void updateBatteryLevel(uint8_t level)
     }
 }
 
-void dumpCharacteristic(BLECharacteristic *c)
-{
-    std::string value = c->getValue();
 
-    if (value.length() > 0) {
-        DEBUG_MSG("New value: ");
-        for (int i = 0; i < value.length(); i++)
-            DEBUG_MSG("%c", value[i]);
-
-        DEBUG_MSG("\n");
-    }
-}
-
-/** converting endianness pull out a 32 bit value */
-uint32_t getValue32(BLECharacteristic *c, uint32_t defaultValue)
-{
-    std::string value = c->getValue();
-    uint32_t r = defaultValue;
-
-    if (value.length() == 4)
-        r = value[0] | (value[1] << 8UL) | (value[2] << 16UL) | (value[3] << 24UL);
-
-    return r;
-}
-
-class MySecurity : public BLESecurityCallbacks
-{
-  protected:
-    bool onConfirmPIN(uint32_t pin)
-    {
-        Serial.printf("onConfirmPIN %u\n", pin);
-        return false;
-    }
-
-    uint32_t onPassKeyRequest()
-    {
-        Serial.println("onPassKeyRequest");
-        return 123511; // not used
-    }
-
-    void onPassKeyNotify(uint32_t pass_key)
-    {
-        Serial.printf("onPassKeyNotify %06u\n", pass_key);
-        startCb(pass_key);
-    }
-
-    bool onSecurityRequest()
-    {
-        Serial.println("onSecurityRequest");
-        return true;
-    }
-
-    void onAuthenticationComplete(esp_ble_auth_cmpl_t cmpl)
-    {
-        if (cmpl.success) {
-            uint16_t length;
-            esp_ble_gap_get_whitelist_size(&length);
-            Serial.printf(" authenticated and connected to phone\n");
-        } else {
-            Serial.printf("phone authenticate failed %d\n", cmpl.fail_reason);
-        }
-
-        // Remove our custom PIN request screen.
-        stopCb();
-    }
-
-  public:
-    StartBluetoothPinScreenCallback startCb;
-    StopBluetoothPinScreenCallback stopCb;
-};
-
-BLEServer *pServer;
-
-BLEService *pDevInfo, *pUpdate, *pBattery;
-
-void deinitBLE()
-{
-    assert(pServer);
-
-    pServer->getAdvertising()->stop();
-
-    if (pUpdate != NULL) {
-        destroyUpdateService();
-
-        pUpdate->stop(); // we delete them below
-        pUpdate->executeDelete();
-    }
-
-    pBattery->stop();
-    pBattery->executeDelete();
-
-    pDevInfo->stop();
-    pDevInfo->executeDelete();
-
-    // First shutdown bluetooth
-    BLEDevice::deinit(false);
-
-    // do not delete this - it is dynamically allocated, but only once - statically in BLEDevice
-    // delete pServer->getAdvertising();
-
-    if (pUpdate != NULL)
-        delete pUpdate;
-    delete pDevInfo;
-    delete pBattery;
-    delete pServer;
-
-    batteryLevelC = NULL; // Don't let anyone generate bogus notifies
-
-    for (int i = 0; i < numChars; i++) {
-        delete chars[i];
-    }
-    numChars = 0;
-
-    for (int i = 0; i < numDescs; i++)
-        delete descs[i];
-    numDescs = 0;
-
-    btPool.reset();
-}
-
-BLEServer *initBLE(StartBluetoothPinScreenCallback startBtPinScreen, StopBluetoothPinScreenCallback stopBtPinScreen,
-                   std::string deviceName, std::string hwVendor, std::string swVersion, std::string hwVersion)
-{
-    BLEDevice::init(deviceName);
-    BLEDevice::setEncryptionLevel(ESP_BLE_SEC_ENCRYPT);
-
-    /*
-     * Required in authentication process to provide displaying and/or input passkey or yes/no butttons confirmation
-     */
-    static MySecurity mySecurity;
-    mySecurity.startCb = startBtPinScreen;
-    mySecurity.stopCb = stopBtPinScreen;
-    BLEDevice::setSecurityCallbacks(&mySecurity);
-
-    // Create the BLE Server
-    pServer = BLEDevice::createServer();
-    static MyServerCallbacks myCallbacks;
-    pServer->setCallbacks(&myCallbacks);
-
-    pDevInfo = createDeviceInfomationService(pServer, hwVendor, swVersion, hwVersion);
-
-    pBattery = createBatteryService(pServer);
-
-#define BLE_SOFTWARE_UPDATE
-#ifdef BLE_SOFTWARE_UPDATE
-    pUpdate = createUpdateService(pServer, hwVendor, swVersion,
-                                  hwVersion); // We need to advertise this so our android ble scan operation can see it
-
-    pUpdate->start();
-#endif
-
-    // It seems only one service can be advertised - so for now don't advertise our updater
-    // pServer->getAdvertising()->addServiceUUID(pUpdate->getUUID());
-
-    // start all our services (do this after creating all of them)
-    pDevInfo->start();
-
-    // FIXME turn on this restriction only after the device is paired with a phone
-    // advert->setScanFilter(false, true); // We let anyone scan for us (FIXME, perhaps only allow that until we are paired with a
-    // phone and configured) but only let whitelist phones connect
-
-    static BLESecurity security; // static to avoid allocs
-    BLESecurity *pSecurity = &security;
-    pSecurity->setCapability(ESP_IO_CAP_OUT);
-
-    // FIXME - really should be ESP_LE_AUTH_REQ_SC_BOND but it seems there is a bug right now causing that bonding info to be lost
-    // occasionally?
-    pSecurity->setAuthenticationMode(ESP_LE_AUTH_REQ_SC_MITM_BOND);
-
-    pSecurity->setInitEncryptionKey(ESP_BLE_ENC_KEY_MASK | ESP_BLE_ID_KEY_MASK);
-
-    return pServer;
-}
 
 // Note: these callbacks might be coming in from a different thread.
 BLEServer *serve = initBLE(, , getDeviceName(), HW_VENDOR, optstr(APP_VERSION),
                            optstr(HW_VERSION)); // FIXME, use a real name based on the macaddr
 
-#else
+#endif
 
 #include "PhoneAPI.h"
 #include "PowerFSM.h"
@@ -436,9 +164,23 @@ void updateBatteryLevel(uint8_t level)
 
 void deinitBLE()
 {
+    DEBUG_MSG("Shutting down bluetooth\n");
+    ble_gatts_show_local();
+
     // FIXME - do we need to dealloc things? - what needs to stay alive across light sleep?
     auto ret = nimble_port_stop();
     assert(ret == ESP_OK);
+
+    nimble_port_deinit();          // teardown nimble datastructures
+    nimble_port_freertos_deinit(); // delete the task
+
+    ret = esp_nimble_hci_and_controller_deinit();
+    assert(ret == ESP_OK);
+
+    ret = ble_gatts_reset(); // Teardown the service tables, so the next restart assigns the same handle numbers
+    assert(ret == ESP_OK);
+
+    DEBUG_MSG("Done shutting down bluetooth\n");
 }
 
 void loopBLE()
@@ -452,7 +194,7 @@ extern "C" void ble_store_config_init(void);
 static void print_addr(const uint8_t v[], bool isReversed = true)
 {
     const int macaddrlen = 6;
-    
+
     for (int i = 0; i < macaddrlen; i++) {
         DEBUG_MSG("%02x%c", v[isReversed ? macaddrlen - i : i], (i == macaddrlen - 1) ? '\n' : ':');
     }
@@ -494,7 +236,7 @@ static void advertise();
  *                                  of the return code is specific to the
  *                                  particular GAP event being signalled.
  */
-static int bleprph_gap_event(struct ble_gap_event *event, void *arg)
+static int gap_event(struct ble_gap_event *event, void *arg)
 {
     struct ble_gap_conn_desc desc;
     int rc;
@@ -673,7 +415,7 @@ static void advertise(void)
     adv_params.conn_mode = BLE_GAP_CONN_MODE_UND;
     adv_params.disc_mode = BLE_GAP_DISC_MODE_GEN;
     // FIXME - use RPA for first parameter
-    rc = ble_gap_adv_start(own_addr_type, NULL, BLE_HS_FOREVER, &adv_params, bleprph_gap_event, NULL);
+    rc = ble_gap_adv_start(own_addr_type, NULL, BLE_HS_FOREVER, &adv_params, gap_event, NULL);
     if (rc != 0) {
         DEBUG_MSG("error enabling advertisement; rc=%d\n", rc);
         return;
@@ -717,11 +459,7 @@ static void ble_host_task(void *param)
     DEBUG_MSG("BLE task running\n");
     nimble_port_run(); // This function will return only when nimble_port_stop() is executed.
 
-    nimble_port_deinit();          // teardown nimble datastructures
+    DEBUG_MSG("BLE task exiting\n");
-    nimble_port_freertos_deinit(); // delete the task
-
-    auto ret = esp_nimble_hci_and_controller_deinit();
-    assert(ret == ESP_OK);
 }
 
 void gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, void *arg)
@@ -760,15 +498,15 @@ void reinitBluetooth()
     DEBUG_MSG("Starting bluetooth\n");
     esp_log_level_set("BTDM_INIT", ESP_LOG_VERBOSE);
 
-    if (!bluetoothPhoneAPI) {
+    auto isFirstTime = !bluetoothPhoneAPI;
+
+    if (isFirstTime) {
         bluetoothPhoneAPI = new BluetoothPhoneAPI();
         bluetoothPhoneAPI->init();
     }
 
-    // FIXME - if waking from light sleep, only esp_nimble_hci_init
+    // FIXME - if waking from light sleep, only esp_nimble_hci_init?
-    // FIXME - why didn't this version work?
+    auto res = esp_nimble_hci_and_controller_init(); // : esp_nimble_hci_init();
-    auto res = esp_nimble_hci_and_controller_init();
-    // auto res = esp_nimble_hci_init();
     // DEBUG_MSG("BLE result %d\n", res);
     assert(res == ESP_OK);
 
@@ -808,5 +546,3 @@ void reinitBluetooth()
 
     nimble_port_freertos_init(ble_host_task);
 }
-
-#endif