From 0a6059ba13486d9ebb6b085536ead45d73046a60 Mon Sep 17 00:00:00 2001 From: grcasanova Date: Sun, 5 Jul 2020 23:11:40 +0200 Subject: [PATCH] refactored threading-related classes, code broken --- .gitignore | 11 ++- src/WorkerThread.cpp | 49 ------------ src/WorkerThread.h | 96 ------------------------ src/concurrency/Lock.cpp | 23 ++++++ src/{lock.h => concurrency/Lock.h} | 22 +----- src/concurrency/LockGuard.cpp | 15 ++++ src/concurrency/LockGuard.h | 21 ++++++ src/concurrency/NotifiedWorkerThread.cpp | 19 +++++ src/concurrency/NotifiedWorkerThread.h | 47 ++++++++++++ src/concurrency/Thread.cpp | 16 ++++ src/concurrency/Thread.h | 34 +++++++++ src/concurrency/WorkerThread.cpp | 25 ++++++ src/concurrency/WorkerThread.h | 30 ++++++++ src/debug.cpp | 20 ----- src/debug.h | 10 --- src/lock.cpp | 50 ------------ 16 files changed, 243 insertions(+), 245 deletions(-) delete mode 100644 src/WorkerThread.cpp delete mode 100644 src/WorkerThread.h create mode 100644 src/concurrency/Lock.cpp rename src/{lock.h => concurrency/Lock.h} (59%) create mode 100644 src/concurrency/LockGuard.cpp create mode 100644 src/concurrency/LockGuard.h create mode 100644 src/concurrency/NotifiedWorkerThread.cpp create mode 100644 src/concurrency/NotifiedWorkerThread.h create mode 100644 src/concurrency/Thread.cpp create mode 100644 src/concurrency/Thread.h create mode 100644 src/concurrency/WorkerThread.cpp create mode 100644 src/concurrency/WorkerThread.h delete mode 100644 src/debug.cpp delete mode 100644 src/debug.h delete mode 100644 src/lock.cpp diff --git a/.gitignore b/.gitignore index b26acad66..c6d44fad0 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,13 @@ main/credentials.h !.vscode/settings.json !.vscode/tasks.json !.vscode/extensions.json -*.code-workspace \ No newline at end of file +*.code-workspace + +.DS_Store +Thumbs.db +.autotools +.built +.context +.cproject +.idea/* +.vagrant diff --git a/src/WorkerThread.cpp b/src/WorkerThread.cpp deleted file mode 100644 index 3c7ea4c9a..000000000 --- a/src/WorkerThread.cpp +++ /dev/null @@ -1,49 +0,0 @@ -#include "WorkerThread.h" -#include "debug.h" -#include - -#ifdef configUSE_PREEMPTION - -void Thread::start(const char *name, size_t stackSize, uint32_t priority) -{ - auto r = xTaskCreate(callRun, name, stackSize, this, priority, &taskHandle); - assert(r == pdPASS); -} - -void Thread::callRun(void *_this) -{ - ((Thread *)_this)->doRun(); -} - -void WorkerThread::doRun() -{ - while (!wantExit) { - block(); - -#ifdef DEBUG_STACK - static uint32_t lastPrint = 0; - if (millis() - lastPrint > 10 * 1000L) { - lastPrint = millis(); - meshtastic::printThreadInfo("net"); - } -#endif - - loop(); - } -} - -/** - * Notify this thread so it can run - */ -void NotifiedWorkerThread::notify(uint32_t v, eNotifyAction action) -{ - xTaskNotify(taskHandle, v, action); -} - -void NotifiedWorkerThread::block() -{ - xTaskNotifyWait(0, // don't clear notification on entry - clearOnRead, ¬ification, portMAX_DELAY); // Wait forever -} - -#endif \ No newline at end of file diff --git a/src/WorkerThread.h b/src/WorkerThread.h deleted file mode 100644 index ab1864869..000000000 --- a/src/WorkerThread.h +++ /dev/null @@ -1,96 +0,0 @@ -#include -#include "freertosinc.h" - -#ifdef HAS_FREE_RTOS - -class Thread -{ - protected: - TaskHandle_t taskHandle = NULL; - - /** - * set this to true to ask thread to cleanly exit asap - */ - volatile bool wantExit = false; - - public: - void start(const char *name, size_t stackSize = 1024, uint32_t priority = tskIDLE_PRIORITY); - - virtual ~Thread() { vTaskDelete(taskHandle); } - - uint32_t getStackHighwaterMark() { return uxTaskGetStackHighWaterMark(taskHandle); } - - protected: - /** - * The method that will be called when start is called. - */ - virtual void doRun() = 0; - - private: - static void callRun(void *_this); -}; - -/** - * This wraps threading (FreeRTOS for now) with a blocking API intended for efficiently converting onlyschool arduino loop() code. - * - * Use as a mixin base class for the classes you want to convert. - * - * https://www.freertos.org/RTOS_Task_Notification_As_Mailbox.html - */ -class WorkerThread : public Thread -{ - protected: - /** - * A method that should block execution - either waiting ona queue/mutex or a "task notification" - */ - virtual void block() = 0; - - virtual void loop() = 0; - - /** - * The method that will be called when start is called. - */ - virtual void doRun(); -}; - -/** - * A worker thread that waits on a freertos notification - */ -class NotifiedWorkerThread : public WorkerThread -{ - public: - /** - * Notify this thread so it can run - */ - void notify(uint32_t v = 0, eNotifyAction action = eNoAction); - - /** - * Notify from an ISR - * - * This must be inline or IRAM_ATTR on ESP32 - */ - inline void notifyFromISR(BaseType_t *highPriWoken, uint32_t v = 0, eNotifyAction action = eNoAction) - { - xTaskNotifyFromISR(taskHandle, v, action, highPriWoken); - } - - protected: - /** - * The notification that was most recently used to wake the thread. Read from loop() - */ - uint32_t notification = 0; - - /** - * What notification bits should be cleared just after we read and return them in notification? - * - * Defaults to clear all of them. - */ - uint32_t clearOnRead = UINT32_MAX; - - /** - * A method that should block execution - either waiting ona queue/mutex or a "task notification" - */ - virtual void block(); -}; - -#endif \ No newline at end of file diff --git a/src/concurrency/Lock.cpp b/src/concurrency/Lock.cpp new file mode 100644 index 000000000..07f5a2ef5 --- /dev/null +++ b/src/concurrency/Lock.cpp @@ -0,0 +1,23 @@ +#include "Lock.h" +#include + +namespace concurrency { + +Lock::Lock() +{ + handle = xSemaphoreCreateBinary(); + assert(handle); + assert(xSemaphoreGive(handle)); +} + +void Lock::lock() +{ + assert(xSemaphoreTake(handle, portMAX_DELAY)); +} + +void Lock::unlock() +{ + assert(xSemaphoreGive(handle)); +} + +} // namespace concurrency diff --git a/src/lock.h b/src/concurrency/Lock.h similarity index 59% rename from src/lock.h rename to src/concurrency/Lock.h index 6a90c61b1..c52b629c0 100644 --- a/src/lock.h +++ b/src/concurrency/Lock.h @@ -2,8 +2,7 @@ #include "freertosinc.h" -namespace meshtastic -{ +namespace concurrency { // Simple wrapper around FreeRTOS API for implementing a mutex lock. class Lock @@ -25,23 +24,8 @@ class Lock void unlock(); private: -#ifdef configUSE_PREEMPTION SemaphoreHandle_t handle; -#endif + }; -// RAII lock guard. -class LockGuard -{ - public: - LockGuard(Lock *lock); - ~LockGuard(); - - LockGuard(const LockGuard &) = delete; - LockGuard &operator=(const LockGuard &) = delete; - - private: - Lock *lock; -}; - -} // namespace meshtastic +} // namespace concurrency diff --git a/src/concurrency/LockGuard.cpp b/src/concurrency/LockGuard.cpp new file mode 100644 index 000000000..56e8ac877 --- /dev/null +++ b/src/concurrency/LockGuard.cpp @@ -0,0 +1,15 @@ +#include "LockGuard.h" + +namespace concurrency { + +LockGuard::LockGuard(Lock *lock) : lock(lock) +{ + lock->lock(); +} + +LockGuard::~LockGuard() +{ + lock->unlock(); +} + +} // namespace concurrency diff --git a/src/concurrency/LockGuard.h b/src/concurrency/LockGuard.h new file mode 100644 index 000000000..6c35982ad --- /dev/null +++ b/src/concurrency/LockGuard.h @@ -0,0 +1,21 @@ +#pragma once + +#include "Lock.h" + +namespace concurrency { + +// RAII lock guard. +class LockGuard +{ + public: + LockGuard(Lock *lock); + ~LockGuard(); + + LockGuard(const LockGuard &) = delete; + LockGuard &operator=(const LockGuard &) = delete; + + private: + Lock *lock; +}; + +} // namespace concurrency diff --git a/src/concurrency/NotifiedWorkerThread.cpp b/src/concurrency/NotifiedWorkerThread.cpp new file mode 100644 index 000000000..7785ecf8c --- /dev/null +++ b/src/concurrency/NotifiedWorkerThread.cpp @@ -0,0 +1,19 @@ +#include "NotifiedWorkerThread.h" + +namespace concurrency { + +/** + * Notify this thread so it can run + */ +void NotifiedWorkerThread::notify(uint32_t v, eNotifyAction action) +{ + xTaskNotify(taskHandle, v, action); +} + +void NotifiedWorkerThread::block() +{ + xTaskNotifyWait(0, // don't clear notification on entry + clearOnRead, ¬ification, portMAX_DELAY); // Wait forever +} + +} // namespace concurrency diff --git a/src/concurrency/NotifiedWorkerThread.h b/src/concurrency/NotifiedWorkerThread.h new file mode 100644 index 000000000..4fd053269 --- /dev/null +++ b/src/concurrency/NotifiedWorkerThread.h @@ -0,0 +1,47 @@ +#pragma once + +#include "WorkerThread.h" + +namespace concurrency { + +/** + * A worker thread that waits on a freertos notification + */ +class NotifiedWorkerThread : public WorkerThread +{ + public: + /** + * Notify this thread so it can run + */ + void notify(uint32_t v = 0, eNotifyAction action = eNoAction); + + /** + * Notify from an ISR + * + * This must be inline or IRAM_ATTR on ESP32 + */ + inline void notifyFromISR(BaseType_t *highPriWoken, uint32_t v = 0, eNotifyAction action = eNoAction) + { + xTaskNotifyFromISR(taskHandle, v, action, highPriWoken); + } + + protected: + /** + * The notification that was most recently used to wake the thread. Read from loop() + */ + uint32_t notification = 0; + + /** + * What notification bits should be cleared just after we read and return them in notification? + * + * Defaults to clear all of them. + */ + uint32_t clearOnRead = UINT32_MAX; + + /** + * A method that should block execution - either waiting ona queue/mutex or a "task notification" + */ + virtual void block(); +}; + +} // namespace concurrency diff --git a/src/concurrency/Thread.cpp b/src/concurrency/Thread.cpp new file mode 100644 index 000000000..39fd8f6ed --- /dev/null +++ b/src/concurrency/Thread.cpp @@ -0,0 +1,16 @@ +#include "Thread.h" + +namespace concurrency { + +void Thread::start(const char *name, size_t stackSize, uint32_t priority) +{ + auto r = xTaskCreate(callRun, name, stackSize, this, priority, &taskHandle); + assert(r == pdPASS); +} + +void Thread::callRun(void *_this) +{ + ((Thread *)_this)->doRun(); +} + +} // namespace concurrency diff --git a/src/concurrency/Thread.h b/src/concurrency/Thread.h new file mode 100644 index 000000000..e8a710ebf --- /dev/null +++ b/src/concurrency/Thread.h @@ -0,0 +1,34 @@ +#pragma once + +#include "freertosinc.h" + +namespace concurrency { + +class Thread +{ + protected: + TaskHandle_t taskHandle = NULL; + + /** + * set this to true to ask thread to cleanly exit asap + */ + volatile bool wantExit = false; + + public: + void start(const char *name, size_t stackSize = 1024, uint32_t priority = tskIDLE_PRIORITY); + + virtual ~Thread() { vTaskDelete(taskHandle); } + + uint32_t getStackHighwaterMark() { return uxTaskGetStackHighWaterMark(taskHandle); } + + protected: + /** + * The method that will be called when start is called. + */ + virtual void doRun() = 0; + + private: + static void callRun(void *_this); +}; + +} // namespace concurrency diff --git a/src/concurrency/WorkerThread.cpp b/src/concurrency/WorkerThread.cpp new file mode 100644 index 000000000..3287c44f6 --- /dev/null +++ b/src/concurrency/WorkerThread.cpp @@ -0,0 +1,25 @@ +#include "WorkerThread.h" +#include "debug.h" + +namespace concurrency { + +void WorkerThread::doRun() +{ + while (!wantExit) { + block(); + +#ifdef DEBUG_STACK + static uint32_t lastPrint = 0; + if (millis() - lastPrint > 10 * 1000L) { + lastPrint = millis(); + uint32_t taskHandle = reinterpret_cast(xTaskGetCurrentTaskHandle()); + DEBUG_MSG("printThreadInfo(%s) task: %" PRIx32 " core id: %u min free stack: %u\n", "thread", taskHandle, xPortGetCoreID(), + uxTaskGetStackHighWaterMark(nullptr)); + } +#endif + + loop(); + } +} + +} // namespace concurrency diff --git a/src/concurrency/WorkerThread.h b/src/concurrency/WorkerThread.h new file mode 100644 index 000000000..0ae3060a0 --- /dev/null +++ b/src/concurrency/WorkerThread.h @@ -0,0 +1,30 @@ +#pragma once + +#include "Thread.h" + +namespace concurrency { + +/** + * This wraps threading (FreeRTOS for now) with a blocking API intended for efficiently converting onlyschool arduino loop() code. + * + * Use as a mixin base class for the classes you want to convert. + * + * https://www.freertos.org/RTOS_Task_Notification_As_Mailbox.html + */ +class WorkerThread : public Thread +{ + protected: + /** + * A method that should block execution - either waiting ona queue/mutex or a "task notification" + */ + virtual void block() = 0; + + virtual void loop() = 0; + + /** + * The method that will be called when start is called. + */ + virtual void doRun(); +}; + +} // namespace concurrency diff --git a/src/debug.cpp b/src/debug.cpp deleted file mode 100644 index 9d8f19f09..000000000 --- a/src/debug.cpp +++ /dev/null @@ -1,20 +0,0 @@ -#include "debug.h" - -#include - -#include "freertosinc.h" -#include "configuration.h" - -namespace meshtastic -{ - -void printThreadInfo(const char *extra) -{ -#ifndef NO_ESP32 - uint32_t taskHandle = reinterpret_cast(xTaskGetCurrentTaskHandle()); - DEBUG_MSG("printThreadInfo(%s) task: %" PRIx32 " core id: %u min free stack: %u\n", extra, taskHandle, xPortGetCoreID(), - uxTaskGetStackHighWaterMark(nullptr)); -#endif -} - -} // namespace meshtastic diff --git a/src/debug.h b/src/debug.h deleted file mode 100644 index 74a1875e4..000000000 --- a/src/debug.h +++ /dev/null @@ -1,10 +0,0 @@ -#pragma once - -namespace meshtastic -{ - -/// Dumps out which core we are running on, and min level of remaining stack -/// seen. -void printThreadInfo(const char *extra); - -} // namespace meshtastic diff --git a/src/lock.cpp b/src/lock.cpp deleted file mode 100644 index 4a8d28cf5..000000000 --- a/src/lock.cpp +++ /dev/null @@ -1,50 +0,0 @@ -#include "lock.h" - -#include - -namespace meshtastic -{ - -#ifdef configUSE_PREEMPTION -Lock::Lock() -{ - handle = xSemaphoreCreateBinary(); - assert(handle); - assert(xSemaphoreGive(handle)); -} - -void Lock::lock() -{ - assert(xSemaphoreTake(handle, portMAX_DELAY)); -} - -void Lock::unlock() -{ - assert(xSemaphoreGive(handle)); -} -#else -Lock::Lock() -{ -} - -void Lock::lock() -{ -} - -void Lock::unlock() -{ -} -#endif - - -LockGuard::LockGuard(Lock *lock) : lock(lock) -{ - lock->lock(); -} - -LockGuard::~LockGuard() -{ - lock->unlock(); -} - -} // namespace meshtastic