From 1656c8d0cb64e7afc6371504c2378d69f982ec14 Mon Sep 17 00:00:00 2001 From: geeksville Date: Mon, 25 May 2020 07:48:36 -0700 Subject: [PATCH] use my Timer class on all platforms, it works better than the freertos version --- docs/software/nrf52-TODO.md | 6 +++-- src/OSTimer.cpp | 12 ++++----- src/OSTimer.h | 10 -------- src/PeriodicTask.h | 9 ++++++- src/mesh/RadioLibInterface.cpp | 45 +++++++++++----------------------- src/mesh/RadioLibInterface.h | 11 +++++++-- src/mesh/SX1262Interface.cpp | 6 ++--- src/nrf52/main-nrf52.cpp | 5 +++- 8 files changed, 48 insertions(+), 56 deletions(-) diff --git a/docs/software/nrf52-TODO.md b/docs/software/nrf52-TODO.md index 99477d761..2e469f576 100644 --- a/docs/software/nrf52-TODO.md +++ b/docs/software/nrf52-TODO.md @@ -7,8 +7,6 @@ Minimum items needed to make sure hardware is good. - write UC1701 wrapper -- scheduleOSCallback doesn't work yet - it is way too fast (causes rapid polling of busyTx, high power draw etc...) -- find out why we reboot while debugging - seems to be power? try using external supply - DONE install a hardfault handler for null ptrs (if one isn't already installed) - test my hackedup bootloader on the real hardware - Use the PMU driver on real hardware @@ -43,6 +41,7 @@ Needed to be fully functional at least at the same level of the ESP32 boards. At - change packet numbers to be 32 bits - check datasheet about sx1262 temperature compensation +- enable brownout detection and watchdog - stop polling for GPS characters, instead stay blocked on read in a thread - turn back on in-radio destaddr checking for RF95 - figure out what the correct current limit should be for the sx1262, currently we just use the default 100 @@ -63,6 +62,7 @@ Nice ideas worth considering someday... - Use flego to me an iOS/linux app? https://felgo.com/doc/qt/qtbluetooth-index/ or - Use flutter to make an iOS/linux app? https://github.com/Polidea/FlutterBleLib - enable monitor mode debuggin (need to use real jlink): https://devzone.nordicsemi.com/nordic/nordic-blog/b/blog/posts/monitor-mode-debugging-with-j-link-and-gdbeclipse +- Improve efficiency of PeriodicTimer by only checking the next queued timer event, and carefully sorting based on schedule - make a Mfg Controller and device under test classes as examples of custom app code for third party devs. Make a post about this. Use a custom payload type code. Have device under test send a broadcast with max hopcount of 0 for the 'mfgcontroller' payload type. mfg controller will read SNR and reply. DOT will declare failure/success and switch to the regular app screen. - Hook Segger RTT to the nordic logging framework. https://devzone.nordicsemi.com/nordic/nordic-blog/b/blog/posts/debugging-with-real-time-terminal - Use nordic logging for DEBUG_MSG @@ -123,6 +123,8 @@ Nice ideas worth considering someday... - customize the bootloader to use proper button bindings - remove the MeshRadio wrapper - we don't need it anymore, just do everything in RadioInterface subclasses. - DONE use SX126x::startReceiveDutyCycleAuto to save power by sleeping and briefly waking to check for preamble bits. Change xmit rules to have more preamble bits. +- scheduleOSCallback doesn't work yet - it is way too fast (causes rapid polling of busyTx, high power draw etc...) +- find out why we reboot while debugging - it was bluetooth/softdevice ``` diff --git a/src/OSTimer.cpp b/src/OSTimer.cpp index 0da3b7d28..0978163ce 100644 --- a/src/OSTimer.cpp +++ b/src/OSTimer.cpp @@ -1,21 +1,21 @@ #include "OSTimer.h" #include "configuration.h" -#ifdef NO_ESP32 - /** * Schedule a callback to run. The callback must _not_ block, though it is called from regular thread level (not ISR) * - * NOTE! xTimerPend... seems to ignore the time passed in on ESP32 - I haven't checked on NRF52 + * NOTE! xTimerPend... seems to ignore the time passed in on ESP32 and on NRF52 + * The reason this didn't work is bcause xTimerPednFunctCall really isn't a timer function at all - it just means run the callback + * from the timer thread the next time you have spare cycles. * * @return true if successful, false if the timer fifo is too full. - */ + bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec) { return xTimerPendFunctionCall(callback, param1, param2, pdMS_TO_TICKS(delayMsec)); -} +} */ -#else +#ifndef NO_ESP32 // Super skanky quick hack to use hardware timers of the ESP32 static hw_timer_t *timer; diff --git a/src/OSTimer.h b/src/OSTimer.h index cdf2386a6..37415f3a6 100644 --- a/src/OSTimer.h +++ b/src/OSTimer.h @@ -4,15 +4,5 @@ typedef void (*PendableFunction)(void *pvParameter1, uint32_t ulParameter2); -/** - * Schedule a callback to run. The callback must _not_ block, though it is called from regular thread level (not ISR) - * - * NOTE! ESP32 implementation is busted - always waits 0 ticks - * - * @return true if successful, false if the timer fifo is too full. - */ - bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec); - - /// Uses a hardware timer, but calls the handler in _interrupt_ context bool scheduleHWCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec); \ No newline at end of file diff --git a/src/PeriodicTask.h b/src/PeriodicTask.h index 9d2a06b6d..951b8cbbb 100644 --- a/src/PeriodicTask.h +++ b/src/PeriodicTask.h @@ -1,6 +1,7 @@ #pragma once #include "lock.h" +#include #include #include @@ -66,7 +67,13 @@ class PeriodicTask * Set a new period in msecs (can be called from doTask or elsewhere and the scheduler will cope) * While zero this task is disabled and will not run */ - void setPeriod(uint32_t p) { period = p; } + void setPeriod(uint32_t p) + { + lastMsec = millis(); // reset starting from now + period = p; + } + + uint32_t getPeriod() const { return period; } /** * Syntatic sugar for suspending tasks diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 1471e3a98..44bcc0413 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -11,12 +11,18 @@ static SPISettings spiSettings(4000000, MSBFIRST, SPI_MODE0); RadioLibInterface::RadioLibInterface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOLIB_PIN_TYPE rst, RADIOLIB_PIN_TYPE busy, SPIClass &spi, PhysicalLayer *_iface) - : module(cs, irq, rst, busy, spi, spiSettings), iface(_iface) + : PeriodicTask(0), module(cs, irq, rst, busy, spi, spiSettings), iface(_iface) { assert(!instance); // We assume only one for now instance = this; } +bool RadioLibInterface::init() +{ + setup(); // init our timer + return RadioInterface::init(); +} + #ifndef NO_ESP32 // ESP32 doesn't use that flag #define YIELD_FROM_ISR(x) portYIELD_FROM_ISR() @@ -194,48 +200,25 @@ void RadioLibInterface::loop() } } -#ifndef NO_ESP32 -#define USE_HW_TIMER -#else -// Not needed on NRF52 -#define IRAM_ATTR -#endif - -void IRAM_ATTR RadioLibInterface::timerCallback(void *p1, uint32_t p2) +void RadioLibInterface::doTask() { - RadioLibInterface *t = (RadioLibInterface *)p1; - - t->timerRunning = false; + disable(); // Don't call this callback again // We use without overwrite, so that if there is already an interrupt pending to be handled, that gets handle properly (the // ISR handler will restart our timer) -#ifndef USE_HW_TIMER - t->notify(TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); -#else - BaseType_t xHigherPriorityTaskWoken; - instance->notifyFromISR(&xHigherPriorityTaskWoken, TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); - /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE. - The macro used to do this is dependent on the port and may be called - portEND_SWITCHING_ISR. */ - YIELD_FROM_ISR(xHigherPriorityTaskWoken); -#endif + notify(TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); } void RadioLibInterface::startTransmitTimer(bool withDelay) { // If we have work to do and the timer wasn't already scheduled, schedule it now - if (!timerRunning && !txQueue.isEmpty()) { - timerRunning = true; + if (getPeriod() == 0 && !txQueue.isEmpty()) { uint32_t delay = - !withDelay ? 0 : random(MIN_TX_WAIT_MSEC, MAX_TX_WAIT_MSEC); // See documentation for loop() wrt these values + !withDelay ? 1 : random(MIN_TX_WAIT_MSEC, MAX_TX_WAIT_MSEC); // See documentation for loop() wrt these values // DEBUG_MSG("xmit timer %d\n", delay); -#ifdef USE_HW_TIMER - bool okay = scheduleHWCallback(timerCallback, this, 0, delay); -#else - bool okay = scheduleOSCallback(timerCallback, this, 0, delay); -#endif - assert(okay); + + setPeriod(delay); } } diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h index 5f9149d71..1a0e5f450 100644 --- a/src/mesh/RadioLibInterface.h +++ b/src/mesh/RadioLibInterface.h @@ -1,5 +1,6 @@ #pragma once +#include "PeriodicTask.h" #include "RadioInterface.h" #include @@ -11,13 +12,12 @@ #define INTERRUPT_ATTR #endif -class RadioLibInterface : public RadioInterface +class RadioLibInterface : public RadioInterface, private PeriodicTask { /// Used as our notification from the ISR enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX, TRANSMIT_DELAY_COMPLETED }; volatile PendingISR pending = ISR_NONE; - volatile bool timerRunning = false; /** * Raw ISR handler that just calls our polymorphic method @@ -104,7 +104,14 @@ class RadioLibInterface : public RadioInterface static void timerCallback(void *p1, uint32_t p2); + virtual void doTask(); + protected: + /// Initialise the Driver transport hardware and software. + /// Make sure the Driver is properly configured before calling init(). + /// \return true if initialisation succeeded. + virtual bool init(); + /** * Convert our modemConfig enum into wf, sf, etc... * diff --git a/src/mesh/SX1262Interface.cpp b/src/mesh/SX1262Interface.cpp index 84a969765..dadd98d0f 100644 --- a/src/mesh/SX1262Interface.cpp +++ b/src/mesh/SX1262Interface.cpp @@ -113,9 +113,9 @@ void SX1262Interface::startReceive() /** Could we send right now (i.e. either not actively receving or transmitting)? */ bool SX1262Interface::isActivelyReceiving() { - return false; // FIXME - // FIXME this is not correct - often always true - need to add an extra conditional - // return lora.getPacketLength() > 0; + // return false; // FIXME + // FIXME this is not correct? - often always true - need to add an extra conditional + return lora.getPacketLength() > 0; } bool SX1262Interface::sleep() diff --git a/src/nrf52/main-nrf52.cpp b/src/nrf52/main-nrf52.cpp index e0d1ac5e6..6e0b486e0 100644 --- a/src/nrf52/main-nrf52.cpp +++ b/src/nrf52/main-nrf52.cpp @@ -43,12 +43,15 @@ void getMacAddr(uint8_t *dmac) NRF52Bluetooth *nrf52Bluetooth; +// FIXME, turn off soft device access for debugging +static bool isSoftDeviceAllowed = false; + static bool bleOn = false; void setBluetoothEnable(bool on) { if (on != bleOn) { if (on) { - if (!nrf52Bluetooth) { + if (!nrf52Bluetooth && isSoftDeviceAllowed) { nrf52Bluetooth = new NRF52Bluetooth(); nrf52Bluetooth->setup(); }