begin moving comms glue from the old crufty BLE code to the new cleaner PhoneAPI class

proto

@@ -1 +1 @@
-Subproject commit 0cef75501578a2c4adf63da09fdc34db20b3d862
+Subproject commit 083ba793108c34044e6abc8c94a5f250343b4f32

src/MeshService.cpp

@@ -214,46 +214,33 @@ void MeshService::reloadConfig()
     nodeDB.saveToDisk();
 }
 
-/// Given a ToRadio buffer parse it and properly handle it (setup radio, owner or send packet into the mesh)
+/**
-void MeshService::handleToRadio(std::string s)
+ *  Given a ToRadio buffer parse it and properly handle it (setup radio, owner or send packet into the mesh)
+ * Called by PhoneAPI.handleToRadio.  Note: p is a scratch buffer, this function is allowed to write to it but it can not keep a reference
+ */
+void MeshService::handleToRadio(MeshPacket &p)
 {
-    static ToRadio r; // this is a static scratch object, any data must be copied elsewhere before returning
+    handleIncomingPosition(&p); // If it is a position packet, perhaps set our clock
 
-    if (pb_decode_from_bytes((const uint8_t *)s.c_str(), s.length(), ToRadio_fields, &r)) {
+    if (p.from == 0) // If the phone didn't set a sending node ID, use ours
-        switch (r.which_variant) {
+        p.from = nodeDB.getNodeNum();
-        case ToRadio_packet_tag: {
-            // If our phone is sending a position, see if we can use it to set our RTC
-            MeshPacket &p = r.variant.packet;
-            handleIncomingPosition(&p); // If it is a position packet, perhaps set our clock
 
-            if (p.from == 0) // If the phone didn't set a sending node ID, use ours
+    if (p.id == 0)
-                p.from = nodeDB.getNodeNum();
+        p.id = generatePacketId(); // If the phone didn't supply one, then pick one
 
-            if (p.id == 0)
+    p.rx_time = gps.getValidTime(); // Record the time the packet arrived from the phone
-                p.id = generatePacketId(); // If the phone didn't supply one, then pick one
+                                    // (so we update our nodedb for the local node)
 
-            p.rx_time = gps.getValidTime(); // Record the time the packet arrived from the phone
+    // Send the packet into the mesh
-                                            // (so we update our nodedb for the local node)
 
-            // Send the packet into the mesh
+    sendToMesh(packetPool.allocCopy(p));
 
-            sendToMesh(packetPool.allocCopy(p));
+    bool loopback = false; // if true send any packet the phone sends back itself (for testing)
-
+    if (loopback) {
-            bool loopback = false; // if true send any packet the phone sends back itself (for testing)
+        // no need to copy anymore because handle from radio assumes it should _not_ delete
-            if (loopback) {
+        // packetPool.allocCopy(r.variant.packet);
-                // no need to copy anymore because handle from radio assumes it should _not_ delete
+        handleFromRadio(&p);
-                // packetPool.allocCopy(r.variant.packet);
+        // handleFromRadio will tell the phone a new packet arrived
-                handleFromRadio(&p);
-                // handleFromRadio will tell the phone a new packet arrived
-            }
-            break;
-        }
-        default:
-            DEBUG_MSG("Error: unexpected ToRadio variant\n");
-            break;
-        }
-    } else {
-        DEBUG_MSG("Error: ignoring malformed toradio\n");
     }
 }
 

src/MeshService.h

@@ -54,8 +54,12 @@ class MeshService
     /// Allows the bluetooth handler to free packets after they have been sent
     void releaseToPool(MeshPacket *p) { packetPool.release(p); }
 
-    /// Given a ToRadio buffer (from bluetooth) parse it and properly handle it (setup radio, owner or send packet into the mesh)
+    /**
-    void handleToRadio(std::string s);
+     *  Given a ToRadio buffer parse it and properly handle it (setup radio, owner or send packet into the mesh)
+     * Called by PhoneAPI.handleToRadio.  Note: p is a scratch buffer, this function is allowed to write to it but it can not keep
+     * a reference
+     */
+    void handleToRadio(MeshPacket &p);
 
     /// The radioConfig object just changed, call this to force the hw to change to the new settings
     void reloadConfig();

src/PhoneAPI.cpp

@@ -1,4 +1,6 @@
 #include "PhoneAPI.h"
+#include "MeshService.h"
+#include "NodeDB.h"
 #include <assert.h>
 
 PhoneAPI::PhoneAPI()
@@ -8,12 +10,31 @@ PhoneAPI::PhoneAPI()
     assert(ToRadio_size <= 512);
 }
 
+void PhoneAPI::init()
+{
+    observe(&service.fromNumChanged);
+}
+
 /**
  * Handle a ToRadio protobuf
  */
-void PhoneAPI::handleToRadio(const char *buf, size_t len)
+void PhoneAPI::handleToRadio(const uint8_t *buf, size_t bufLength)
 {
-    // FIXME
+    if (pb_decode_from_bytes(buf, bufLength, ToRadio_fields, &toRadioScratch)) {
+        switch (toRadioScratch.which_variant) {
+        case ToRadio_packet_tag: {
+            // If our phone is sending a position, see if we can use it to set our RTC
+            MeshPacket &p = toRadioScratch.variant.packet;
+            service.handleToRadio(p);
+            break;
+        }
+        default:
+            DEBUG_MSG("Error: unexpected ToRadio variant\n");
+            break;
+        }
+    } else {
+        DEBUG_MSG("Error: ignoring malformed toradio\n");
+    }
 }
 
 /**
@@ -21,9 +42,28 @@ void PhoneAPI::handleToRadio(const char *buf, size_t len)
  *
  * We assume buf is at least FromRadio_size bytes long.
  */
-bool PhoneAPI::getFromRadio(char *buf)
+size_t PhoneAPI::getFromRadio(uint8_t *buf)
 {
-    return false; // FIXME
+    if (!available())
+        return false;
+
+    // Do we have a message from the mesh?
+    if (packetForPhone) {
+        // Encapsulate as a FromRadio packet
+        memset(&fromRadioScratch, 0, sizeof(fromRadioScratch));
+        fromRadioScratch.which_variant = FromRadio_packet_tag;
+        fromRadioScratch.variant.packet = *packetForPhone;
+
+        size_t numbytes = pb_encode_to_bytes(buf, sizeof(FromRadio_size), FromRadio_fields, &fromRadioScratch);
+        DEBUG_MSG("delivering toPhone packet to phone %d bytes\n", numbytes);
+
+        service.releaseToPool(packetForPhone); // we just copied the bytes, so don't need this buffer anymore
+        packetForPhone = NULL;
+        return numbytes;
+    }
+
+    DEBUG_MSG("toPhone queue is empty\n");
+    return 0;
 }
 
 /**
@@ -31,6 +71,8 @@ bool PhoneAPI::getFromRadio(char *buf)
  */
 bool PhoneAPI::available()
 {
+    packetForPhone = service.getForPhone();
+
     return true; // FIXME
 }
 
@@ -38,9 +80,33 @@ bool PhoneAPI::available()
 // The following routines are only public for now - until the rev1 bluetooth API is removed
 //
 
-void PhoneAPI::handleSetOwner(const User &o) {}
+void PhoneAPI::handleSetOwner(const User &o)
+{
+    int changed = 0;
 
-void PhoneAPI::handleSetRadio(const RadioConfig &r) {}
+    if (*o.long_name) {
+        changed |= strcmp(owner.long_name, o.long_name);
+        strcpy(owner.long_name, o.long_name);
+    }
+    if (*o.short_name) {
+        changed |= strcmp(owner.short_name, o.short_name);
+        strcpy(owner.short_name, o.short_name);
+    }
+    if (*o.id) {
+        changed |= strcmp(owner.id, o.id);
+        strcpy(owner.id, o.id);
+    }
+
+    if (changed) // If nothing really changed, don't broadcast on the network or write to flash
+        service.reloadOwner();
+}
+
+void PhoneAPI::handleSetRadio(const RadioConfig &r)
+{
+    radioConfig = r;
+
+    service.reloadConfig();
+}
 
 /**
  * The client wants to start a new set of config reads
@@ -50,4 +116,11 @@ void PhoneAPI::handleWantConfig(uint32_t nonce) {}
 /**
  * Handle a packet that the phone wants us to send.  It is our responsibility to free the packet to the pool
  */
-void PhoneAPI::handleToRadioPacket(MeshPacket *p) {}
+void PhoneAPI::handleToRadioPacket(MeshPacket *p) {}
+
+/// If the mesh service tells us fromNum has changed, tell the phone
+int PhoneAPI::onNotify(uint32_t newValue)
+{
+    onNowHasData(newValue);
+    return 0;
+}

src/PhoneAPI.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "Observer.h"
 #include "mesh-pb-constants.h"
 #include "mesh.pb.h"
 #include <string>
@@ -8,10 +9,13 @@
  * Provides our protobuf based API which phone/PC clients can use to talk to our device
  * over UDP, bluetooth or serial.
  *
+ * Subclass to customize behavior for particular type of transport (BLE, UDP, TCP, serial)
+ *
  * Eventually there should be once instance of this class for each live connection (because it has a bit of state
  * for that connection)
  */
 class PhoneAPI
+    : public Observer<uint32_t> // FIXME, we shouldn't be inheriting from Observer, instead use CallbackObserver as a member
 {
     enum State {
         STATE_SEND_NOTHING, // Initial state, don't send anything until the client starts asking for config
@@ -27,22 +31,34 @@ class PhoneAPI
     /**
      * Each packet sent to the phone has an incrementing count
      */
-    uint32_t fromRadioNum = 0; 
+    uint32_t fromRadioNum = 0;
+
+    /// We temporarily keep the packet here between the call to available and getFromRadio
+    MeshPacket *packetForPhone = NULL;
+
+    /// Our fromradio packet while it is being assembled
+    FromRadio fromRadioScratch;
+
+    ToRadio toRadioScratch; // this is a static scratch object, any data must be copied elsewhere before returning
 
   public:
     PhoneAPI();
 
+    /// Do late init that can't happen at constructor time
+    void init();
+
     /**
      * Handle a ToRadio protobuf
      */
-    void handleToRadio(const char *buf, size_t len);
+    void handleToRadio(const uint8_t *buf, size_t len);
 
     /**
-     * Get the next packet we want to send to the phone, or NULL if no such packet is available.
+     * Get the next packet we want to send to the phone
      *
      * We assume buf is at least FromRadio_size bytes long.
+     * Returns number of bytes in the FromRadio packet (or 0 if no packet available)
      */
-    bool getFromRadio(char *buf);
+    size_t getFromRadio(uint8_t *buf);
 
     /**
      * Return true if we have data available to send to the phone
@@ -57,7 +73,6 @@ class PhoneAPI
     void handleSetRadio(const RadioConfig &r);
 
   protected:
-
     /**
      * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies)
      */
@@ -73,4 +88,7 @@ class PhoneAPI
      * Handle a packet that the phone wants us to send.  It is our responsibility to free the packet to the pool
      */
     void handleToRadioPacket(MeshPacket *p);
+
+    /// If the mesh service tells us fromNum has changed, tell the phone
+    virtual int onNotify(uint32_t newValue);
 };

src/esp32/MeshBluetoothService.cpp

@@ -6,19 +6,41 @@
 #include <esp_gatt_defs.h>
 
 #include "CallbackCharacteristic.h"
+#include "GPS.h"
 #include "MeshService.h"
 #include "NodeDB.h"
+#include "PhoneAPI.h"
 #include "PowerFSM.h"
 #include "configuration.h"
 #include "mesh-pb-constants.h"
 #include "mesh.pb.h"
 
-#include "GPS.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(_max(_max(_max(ToRadio_size, RadioConfig_size), User_size), MyNodeInfo_size), FromRadio_size)];
 
+static CallbackCharacteristic *meshFromNumCharacteristic;
+
+BLEService *meshService;
+
+class BluetoothPhoneAPI : public PhoneAPI
+{
+    /**
+     * Subclasses can use this as a hook to provide custom notifications for their transport (i.e. bluetooth notifies)
+     */
+    virtual void onNowHasData(uint32_t fromRadioNum)
+    {
+        PhoneAPI::onNowHasData(fromRadioNum);
+
+        if (meshFromNumCharacteristic) { // this ptr might change from sleep to sleep, or even be null
+            meshFromNumCharacteristic->setValue(fromRadioNum);
+            meshFromNumCharacteristic->notify();
+        }
+    }
+};
+
+BluetoothPhoneAPI *bluetoothPhoneAPI;
+
 class ProtobufCharacteristic : public CallbackCharacteristic
 {
     const pb_msgdesc_t *fields;
@@ -114,7 +136,7 @@ class RadioCharacteristic : public ProtobufCharacteristic
     {
         DEBUG_MSG("Writing radio config\n");
         ProtobufCharacteristic::onWrite(c);
-        service.reloadConfig();
+        bluetoothPhoneAPI->handleSetRadio(radioConfig);
     }
 };
 
@@ -135,23 +157,7 @@ class OwnerCharacteristic : public ProtobufCharacteristic
 
         static User o; // if the phone doesn't set ID we are careful to keep ours, we also always keep our macaddr
         if (writeToDest(c, &o)) {
-            int changed = 0;
+            bluetoothPhoneAPI->handleSetOwner(o);
-
-            if (*o.long_name) {
-                changed |= strcmp(owner.long_name, o.long_name);
-                strcpy(owner.long_name, o.long_name);
-            }
-            if (*o.short_name) {
-                changed |= strcmp(owner.short_name, o.short_name);
-                strcpy(owner.short_name, o.short_name);
-            }
-            if (*o.id) {
-                changed |= strcmp(owner.id, o.id);
-                strcpy(owner.id, o.id);
-            }
-
-            if (changed) // If nothing really changed, don't broadcast on the network or write to flash
-                service.reloadOwner();
         }
     }
 };
@@ -166,7 +172,7 @@ class ToRadioCharacteristic : public CallbackCharacteristic
         BLEKeepAliveCallbacks::onWrite(c);
         DEBUG_MSG("Got on write\n");
 
-        service.handleToRadio(c->getValue());
+        bluetoothPhoneAPI->handleToRadio(c->getData(), c->getValue().length());
     }
 };
 
@@ -180,31 +186,17 @@ class FromRadioCharacteristic : public CallbackCharacteristic
     void onRead(BLECharacteristic *c)
     {
         BLEKeepAliveCallbacks::onRead(c);
-        MeshPacket *mp = service.getForPhone();
+        size_t numBytes = bluetoothPhoneAPI->getFromRadio(trBytes);
 
         // Someone is going to read our value as soon as this callback returns.  So fill it with the next message in the queue
         // or make empty if the queue is empty
-        if (!mp) {
+        if (numBytes) {
-            DEBUG_MSG("toPhone queue is empty\n");
+            c->setValue(trBytes, numBytes);
-            c->setValue((uint8_t *)"", 0);
-        } else {
-            static FromRadio fRadio;
-
-            // Encapsulate as a FromRadio packet
-            memset(&fRadio, 0, sizeof(fRadio));
-            fRadio.which_variant = FromRadio_packet_tag;
-            fRadio.variant.packet = *mp;
-
-            size_t numbytes = pb_encode_to_bytes(trBytes, sizeof(trBytes), FromRadio_fields, &fRadio);
-            DEBUG_MSG("delivering toPhone packet to phone %d bytes\n", numbytes);
-            c->setValue(trBytes, numbytes);
-
-            service.releaseToPool(mp); // we just copied the bytes, so don't need this buffer anymore
         }
     }
 };
 
-class FromNumCharacteristic : public CallbackCharacteristic, public Observer<uint32_t>
+class FromNumCharacteristic : public CallbackCharacteristic
 {
   public:
     FromNumCharacteristic()
@@ -212,7 +204,7 @@ class FromNumCharacteristic : public CallbackCharacteristic, public Observer<uin
                                                                              BLECharacteristic::PROPERTY_READ |
                                                                              BLECharacteristic::PROPERTY_NOTIFY)
     {
-        observe(&service.fromNumChanged);
+        // observe(&service.fromNumChanged);
     }
 
     void onRead(BLECharacteristic *c)
@@ -220,14 +212,6 @@ class FromNumCharacteristic : public CallbackCharacteristic, public Observer<uin
         BLEKeepAliveCallbacks::onRead(c);
         DEBUG_MSG("FIXME implement fromnum read\n");
     }
-
-    /// If the mesh service tells us fromNum has changed, tell the phone
-    virtual int onNotify(uint32_t newValue)
-    {
-        setValue(newValue);
-        notify();
-        return 0;
-    }
 };
 
 class MyNodeInfoCharacteristic : public ProtobufCharacteristic
@@ -251,15 +235,17 @@ class MyNodeInfoCharacteristic : public ProtobufCharacteristic
     }
 };
 
-FromNumCharacteristic *meshFromNumCharacteristic;
-
-BLEService *meshService;
-
 /*
 See bluetooth-api.md for documentation.
  */
 BLEService *createMeshBluetoothService(BLEServer *server)
 {
+    // Only create our phone API object once
+    if (!bluetoothPhoneAPI) {
+        bluetoothPhoneAPI = new BluetoothPhoneAPI();
+        bluetoothPhoneAPI->init();
+    }
+
     // Create the BLE Service, we need more than the default of 15 handles
     BLEService *service = server->createService(BLEUUID("6ba1b218-15a8-461f-9fa8-5dcae273eafd"), 30, 0);