From 6ad451eb5f9387c77740691601f04a8a4bf2e9a7 Mon Sep 17 00:00:00 2001 From: geeksville Date: Fri, 10 Apr 2020 12:18:48 -0700 Subject: [PATCH] move bluetooth code into something that is architecture specific... because the ESP32 implementation will be different from NRF52 to make this possible I needed to decouple knowlege about bluetooth from our mesh service. Instead mesh service now uses the Obserable pattern to let any interested consumer get notified of important mesh changes (currently that is only bluetooth, but really we should do the same thing for decoupling the GUI 'app' from the mesh service) @girtsf would you mind reviewing my Observer changes? I haven't written C++ code in a long time ;-) --- src/GPS.cpp | 2 +- src/GPS.h | 2 +- src/MeshService.cpp | 21 +++---- src/MeshService.h | 15 +++-- src/Observer.cpp | 11 ---- src/Observer.h | 71 +++++++++++++++++++----- src/PowerFSM.cpp | 1 + src/{ => esp32}/MeshBluetoothService.cpp | 22 +++----- src/{ => esp32}/MeshBluetoothService.h | 0 src/main.cpp | 54 +++--------------- src/main.h | 3 + src/sleep.cpp | 9 ++- src/sleep.h | 1 - 13 files changed, 104 insertions(+), 108 deletions(-) rename src/{ => esp32}/MeshBluetoothService.cpp (96%) rename src/{ => esp32}/MeshBluetoothService.h (100%) diff --git a/src/GPS.cpp b/src/GPS.cpp index 8ca8dd36b..ea04b09f9 100644 --- a/src/GPS.cpp +++ b/src/GPS.cpp @@ -192,7 +192,7 @@ The Unix epoch (or Unix time or POSIX time or Unix timestamp) is the number of s hasValidLocation = (latitude != 0) || (longitude != 0); // bogus lat lon is reported as 0,0 if (hasValidLocation) { wantNewLocation = false; - notifyObservers(); + notifyObservers(NULL); // ublox.powerOff(); } } else // we didn't get a location update, go back to sleep and hope the characters show up diff --git a/src/GPS.h b/src/GPS.h index 5d0d4b876..caf3fc249 100644 --- a/src/GPS.h +++ b/src/GPS.h @@ -10,7 +10,7 @@ * * When new data is available it will notify observers. */ -class GPS : public PeriodicTask, public Observable +class GPS : public PeriodicTask, public Observable { SFE_UBLOX_GPS ublox; diff --git a/src/MeshService.cpp b/src/MeshService.cpp index 85f4856d2..6b9dabdc6 100644 --- a/src/MeshService.cpp +++ b/src/MeshService.cpp @@ -3,7 +3,7 @@ #include #include "GPS.h" -#include "MeshBluetoothService.h" +//#include "MeshBluetoothService.h" #include "MeshService.h" #include "NodeDB.h" #include "Periodic.h" @@ -52,8 +52,7 @@ MeshService service; 4 // max number of packets destined to our queue, we dispatch packets quickly so it doesn't need to be big MeshService::MeshService() - : packetPool(MAX_PACKETS), toPhoneQueue(MAX_RX_TOPHONE), fromRadioQueue(MAX_RX_FROMRADIO), fromNum(0), - radio(packetPool, fromRadioQueue) + : packetPool(MAX_PACKETS), toPhoneQueue(MAX_RX_TOPHONE), fromRadioQueue(MAX_RX_FROMRADIO), radio(packetPool, fromRadioQueue) { // assert(MAX_RX_TOPHONE == 32); // FIXME, delete this, just checking my clever macro } @@ -65,7 +64,7 @@ void MeshService::init() if (!radio.init()) DEBUG_MSG("radio init failed\n"); - gps.addObserver(this); + gpsObserver.observe(&gps); // No need to call this here, our periodic task will fire quite soon // sendOwnerPeriod(); @@ -191,7 +190,7 @@ void MeshService::handleFromRadio() handleFromRadio(mp); } if (oldFromNum != fromNum) // We don't want to generate extra notifies for multiple new packets - bluetoothNotifyFromNum(fromNum); + fromNumChanged.notifyObservers(fromNum); } uint32_t sendOwnerCb() @@ -244,7 +243,7 @@ void MeshService::handleToRadio(std::string s) if (loopback) { MeshPacket *mp = packetPool.allocCopy(r.variant.packet); handleFromRadio(mp); - bluetoothNotifyFromNum(fromNum); // tell the phone a new packet arrived + // handleFromRadio will tell the phone a new packet arrived } break; } @@ -323,8 +322,10 @@ void MeshService::sendOurPosition(NodeNum dest, bool wantReplies) sendToMesh(p); } -void MeshService::onGPSChanged() +void MeshService::onGPSChanged(void *unused) { + DEBUG_MSG("got gps notify\n"); + // Update our local node info with our position (even if we don't decide to update anyone else) MeshPacket *p = allocForSending(); p->payload.which_variant = SubPacket_position_tag; @@ -354,9 +355,3 @@ void MeshService::onGPSChanged() releaseToPool(p); } } - -void MeshService::onNotify(Observable *o) -{ - DEBUG_MSG("got gps notify\n"); - onGPSChanged(); -} diff --git a/src/MeshService.h b/src/MeshService.h index 884529237..30e03017c 100644 --- a/src/MeshService.h +++ b/src/MeshService.h @@ -13,8 +13,10 @@ * Top level app for this service. keeps the mesh, the radio config and the queue of received packets. * */ -class MeshService : private Observer +class MeshService { + CallbackObserver gpsObserver = CallbackObserver(this, &MeshService::onGPSChanged); + MemoryPool packetPool; /// received packets waiting for the phone to process them @@ -28,11 +30,13 @@ class MeshService : private Observer PointerQueue fromRadioQueue; /// The current nonce for the newest packet which has been queued for the phone - uint32_t fromNum; + uint32_t fromNum = 0; public: MeshRadio radio; + Observable fromNumChanged; + MeshService(); void init(); @@ -76,14 +80,13 @@ class MeshService : private Observer void sendToMesh(MeshPacket *p); /// Called when our gps position has changed - updates nodedb and sends Location message out into the mesh - void onGPSChanged(); - - virtual void onNotify(Observable *o); + void onGPSChanged(void *arg); /// handle all the packets that just arrived from the mesh radio void handleFromRadio(); - /// Handle a packet that just arrived from the radio. We will either eventually enqueue the message to the phone or return it to the free pool + /// Handle a packet that just arrived from the radio. We will either eventually enqueue the message to the phone or return it + /// to the free pool void handleFromRadio(MeshPacket *p); /// handle a user packet that just arrived on the radio, return NULL if we should not process this packet at all diff --git a/src/Observer.cpp b/src/Observer.cpp index 468f5ade9..1025f8bc0 100644 --- a/src/Observer.cpp +++ b/src/Observer.cpp @@ -1,13 +1,2 @@ #include "Observer.h" -Observer::~Observer() -{ - if (observed) - observed->removeObserver(this); - observed = NULL; -} - -void Observer::observe(Observable *o) -{ - o->addObserver(this); -} \ No newline at end of file diff --git a/src/Observer.h b/src/Observer.h index b4de9d83b..b20fe818d 100644 --- a/src/Observer.h +++ b/src/Observer.h @@ -4,38 +4,83 @@ #include -class Observable; +template class Observable; -class Observer +/** + * An observer which can be mixed in as a baseclass. Implement onNotify as a method in your class. + */ +template class Observer { - Observable *observed; + Observable *observed; public: Observer() : observed(NULL) {} virtual ~Observer(); - void observe(Observable *o); + void observe(Observable *o); private: - friend class Observable; + friend class Observable; - virtual void onNotify(Observable *o) = 0; + protected: + virtual void onNotify(T arg) = 0; }; -class Observable +/** + * An observer that calls an arbitrary method + */ +template class CallbackObserver : public Observer { - std::list observers; + typedef void (Callback::*ObserverCallback)(T arg); + + Callback *objPtr; + ObserverCallback method; public: - void notifyObservers() + CallbackObserver(Callback *_objPtr, ObserverCallback _method) : objPtr(_objPtr), method(_method) {} + + protected: + virtual void onNotify(T arg) { (objPtr->*method)(arg); } +}; + +/** + * An observable class that will notify observers anytime notifyObservers is called. Argument type T can be any type, but for + * performance reasons a pointer or word sized object is recommended. + */ +template class Observable +{ + std::list *> observers; + + public: + /** + * Tell all observers about a change, observers can process arg as they wish + */ + void notifyObservers(T arg) { - for (std::list::const_iterator iterator = observers.begin(); iterator != observers.end(); ++iterator) { - (*iterator)->onNotify(this); + for (typename std::list *>::const_iterator iterator = observers.begin(); iterator != observers.end(); + ++iterator) { + (*iterator)->onNotify(arg); } } - void addObserver(Observer *o) { observers.push_back(o); } + private: + friend class Observer; - void removeObserver(Observer *o) { observers.remove(o); } + // Not called directly, instead call observer.observe + void addObserver(Observer *o) { observers.push_back(o); } + + void removeObserver(Observer *o) { observers.remove(o); } }; + +template Observer::~Observer() +{ + if (observed) + observed->removeObserver(this); + observed = NULL; +} + +template void Observer::observe(Observable *o) +{ + o->addObserver(this); +} \ No newline at end of file diff --git a/src/PowerFSM.cpp b/src/PowerFSM.cpp index ce7005ffa..d484efe62 100644 --- a/src/PowerFSM.cpp +++ b/src/PowerFSM.cpp @@ -7,6 +7,7 @@ #include "main.h" #include "screen.h" #include "sleep.h" +#include "target_specific.h" static void sdsEnter() { diff --git a/src/MeshBluetoothService.cpp b/src/esp32/MeshBluetoothService.cpp similarity index 96% rename from src/MeshBluetoothService.cpp rename to src/esp32/MeshBluetoothService.cpp index 16af14921..d1bd273b6 100644 --- a/src/MeshBluetoothService.cpp +++ b/src/esp32/MeshBluetoothService.cpp @@ -204,7 +204,7 @@ class FromRadioCharacteristic : public CallbackCharacteristic } }; -class FromNumCharacteristic : public CallbackCharacteristic +class FromNumCharacteristic : public CallbackCharacteristic, public Observer { public: FromNumCharacteristic() @@ -212,6 +212,7 @@ class FromNumCharacteristic : public CallbackCharacteristic BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY) { + observe(&service.fromNumChanged); } void onRead(BLECharacteristic *c) @@ -219,6 +220,13 @@ class FromNumCharacteristic : public CallbackCharacteristic BLEKeepAliveCallbacks::onRead(c); DEBUG_MSG("FIXME implement fromnum read\n"); } + + /// If the mesh service tells us fromNum has changed, tell the phone + virtual void onNotify(uint32_t newValue) + { + setValue(newValue); + notify(); + } }; class MyNodeInfoCharacteristic : public ProtobufCharacteristic @@ -244,18 +252,6 @@ class MyNodeInfoCharacteristic : public ProtobufCharacteristic FromNumCharacteristic *meshFromNumCharacteristic; -/** - * Tell any bluetooth clients that the number of rx packets has changed - */ -void bluetoothNotifyFromNum(uint32_t newValue) -{ - if (meshFromNumCharacteristic) { - // if bt not running ignore - meshFromNumCharacteristic->setValue(newValue); - meshFromNumCharacteristic->notify(); - } -} - BLEService *meshService; /* diff --git a/src/MeshBluetoothService.h b/src/esp32/MeshBluetoothService.h similarity index 100% rename from src/MeshBluetoothService.h rename to src/esp32/MeshBluetoothService.h diff --git a/src/main.cpp b/src/main.cpp index 910d45cfe..0c0958a63 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -21,9 +21,7 @@ */ -#include "BluetoothUtil.h" #include "GPS.h" -#include "MeshBluetoothService.h" #include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" @@ -39,6 +37,10 @@ #include #include +#ifndef NO_ESP32 +#include "BluetoothUtil.h" +#endif + #ifdef TBEAM_V10 #include "axp20x.h" AXP20X_Class axp; @@ -59,8 +61,6 @@ static meshtastic::PowerStatus powerStatus; bool ssd1306_found; bool axp192_found; -bool bluetoothOn; - // ----------------------------------------------------------------------------- // Application // ----------------------------------------------------------------------------- @@ -266,49 +266,6 @@ void setup() setCPUFast(false); // 80MHz is fine for our slow peripherals } -void initBluetooth() -{ - DEBUG_MSG("Starting bluetooth\n"); - - // FIXME - we are leaking like crazy - // AllocatorScope scope(btPool); - - // Note: these callbacks might be coming in from a different thread. - BLEServer *serve = initBLE( - [](uint32_t pin) { - powerFSM.trigger(EVENT_BLUETOOTH_PAIR); - screen.startBluetoothPinScreen(pin); - }, - []() { screen.stopBluetoothPinScreen(); }, getDeviceName(), HW_VENDOR, xstr(APP_VERSION), - xstr(HW_VERSION)); // FIXME, use a real name based on the macaddr - createMeshBluetoothService(serve); - - // Start advertising - this must be done _after_ creating all services - serve->getAdvertising()->start(); -} - -void setBluetoothEnable(bool on) -{ - if (on != bluetoothOn) { - DEBUG_MSG("Setting bluetooth enable=%d\n", on); - - bluetoothOn = on; - if (on) { - Serial.printf("Pre BT: %u heap size\n", ESP.getFreeHeap()); - // ESP_ERROR_CHECK( heap_trace_start(HEAP_TRACE_LEAKS) ); - initBluetooth(); - } else { - // We have to totally teardown our bluetooth objects to prevent leaks - stopMeshBluetoothService(); // Must do before shutting down bluetooth - deinitBLE(); - destroyMeshBluetoothService(); // must do after deinit, because it frees our service - Serial.printf("Shutdown BT: %u heap size\n", ESP.getFreeHeap()); - // ESP_ERROR_CHECK( heap_trace_stop() ); - // heap_trace_dump(); - } - } -} - uint32_t ledBlinker() { static bool ledOn; @@ -352,7 +309,10 @@ void loop() ledPeriodic.loop(); // axpDebugOutput.loop(); + +#ifndef NO_ESP32 loopBLE(); +#endif // for debug printing // service.radio.radioIf.canSleep(); diff --git a/src/main.h b/src/main.h index 9842ea96f..c8c379715 100644 --- a/src/main.h +++ b/src/main.h @@ -9,3 +9,6 @@ extern bool isUSBPowered; // Global Screen singleton. extern meshtastic::Screen screen; + +// Return a human readable string of the form "Meshtastic_ab13" +const char *getDeviceName(); diff --git a/src/sleep.cpp b/src/sleep.cpp index e204ea5f5..2a3dcda24 100644 --- a/src/sleep.cpp +++ b/src/sleep.cpp @@ -1,7 +1,5 @@ #include "sleep.h" -#include "BluetoothUtil.h" #include "GPS.h" -#include "MeshBluetoothService.h" #include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" @@ -11,9 +9,14 @@ #include "esp_pm.h" #include "main.h" #include "rom/rtc.h" +#include "target_specific.h" #include #include +#ifndef NO_ESP32 +#include "BluetoothUtil.h" +#endif + #ifdef TBEAM_V10 #include "axp20x.h" extern AXP20X_Class axp; @@ -105,7 +108,9 @@ void doDeepSleep(uint64_t msecToWake) // not using wifi yet, but once we are this is needed to shutoff the radio hw // esp_wifi_stop(); +#ifndef NO_ESP32 BLEDevice::deinit(false); // We are required to shutdown bluetooth before deep or light sleep +#endif screen.setOn(false); // datasheet says this will draw only 10ua diff --git a/src/sleep.h b/src/sleep.h index 49976516b..129728f50 100644 --- a/src/sleep.h +++ b/src/sleep.h @@ -5,7 +5,6 @@ void doDeepSleep(uint64_t msecToWake); esp_sleep_wakeup_cause_t doLightSleep(uint64_t msecToWake); -void setBluetoothEnable(bool on); void setGPSPower(bool on); // Perform power on init that we do on each wake from deep sleep