From 35dada683a32538536c8af3f2bf13766b502f1c9 Mon Sep 17 00:00:00 2001 From: Girts Date: Sun, 15 Mar 2020 17:42:48 -0700 Subject: [PATCH 1/5] run CI on pull requests as well --- .github/workflows/main.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 54530c2d..43cfd4fd 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,5 +1,7 @@ name: Continuous Integration -on: push +on: + - push + - pull_request jobs: main: From 79f1346359828cf7e8275a13eaf76102bebfed44 Mon Sep 17 00:00:00 2001 From: Girts Folkmanis Date: Sun, 15 Mar 2020 17:43:42 -0700 Subject: [PATCH 2/5] underp include paths in lock.h Had the casing wrong, but could get away with it on a mac. --- src/lock.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lock.h b/src/lock.h index bffaeee3..77f8af70 100644 --- a/src/lock.h +++ b/src/lock.h @@ -1,7 +1,7 @@ #pragma once -#include -#include +#include +#include namespace meshtastic { From 90ecdf229e168f834746401cdd9e712b0ae17e42 Mon Sep 17 00:00:00 2001 From: Girts Folkmanis Date: Sun, 15 Mar 2020 19:27:42 -0700 Subject: [PATCH 3/5] add locks to PeriodicTask --- src/PeriodicTask.cpp | 16 +++++++++------- src/PeriodicTask.h | 20 +++++++++++++------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/PeriodicTask.cpp b/src/PeriodicTask.cpp index b9410c69..99115faf 100644 --- a/src/PeriodicTask.cpp +++ b/src/PeriodicTask.cpp @@ -1,23 +1,25 @@ #include "PeriodicTask.h" #include "Periodic.h" -PeriodicTask::PeriodicTask(uint32_t initialPeriod) : period(initialPeriod) -{ -} +PeriodicTask::PeriodicTask(uint32_t initialPeriod) : period(initialPeriod) {} /// call this from loop void PeriodicTask::loop() { - uint32_t now = millis(); - if (period && (now - lastMsec) >= period) { + meshtastic::LockGuard lg(&lock); + uint32_t now = millis(); + if (!period || (now - lastMsec) < period) { + return; + } lastMsec = now; - doTask(); } + // Release the lock in case the task wants to change the period. + doTask(); } void Periodic::doTask() { uint32_t p = callback(); setPeriod(p); -} \ No newline at end of file +} diff --git a/src/PeriodicTask.h b/src/PeriodicTask.h index 59659e2d..f4a35a2c 100644 --- a/src/PeriodicTask.h +++ b/src/PeriodicTask.h @@ -1,11 +1,12 @@ #pragma once -#include -#include "configuration.h" +#include + +#include "lock.h" /** * A base class for tasks that want their doTask() method invoked periodically - * + * * FIXME: currently just syntatic sugar for polling in loop (you must call .loop), but eventually * generalize with the freertos scheduler so we can save lots of power by having everything either in * something like this or triggered off of an irq. @@ -15,9 +16,10 @@ class PeriodicTask uint32_t lastMsec = 0; uint32_t period = 1; // call soon after creation -public: - uint32_t periodMsec; + // Protects the above variables. + meshtastic::Lock lock; + public: virtual ~PeriodicTask() {} PeriodicTask(uint32_t initialPeriod = 1); @@ -26,8 +28,12 @@ public: virtual void loop(); /// Set a new period in msecs (can be called from doTask or elsewhere and the scheduler will cope) - void setPeriod(uint32_t p) { period = p; } + void setPeriod(uint32_t p) + { + meshtastic::LockGuard lg(&lock); + period = p; + } -protected: + protected: virtual void doTask() = 0; }; From 7a4a1af3322daf380b1f35f2d9eb2e687eb6219a Mon Sep 17 00:00:00 2001 From: Girts Folkmanis Date: Sun, 15 Mar 2020 17:52:01 -0700 Subject: [PATCH 4/5] TypedQueue: make functions return bools instead of BaseType_t Minor cleanup to hide away some FreeRTOS bits. Note: I believe src/CustomRF95.cpp:62 had a bug where it had the condition inverted. --- src/CustomRF95.cpp | 5 ++--- src/MemoryPool.h | 6 ++---- src/MeshService.cpp | 2 +- src/PointerQueue.h | 4 ++-- src/TypedQueue.h | 30 ++++++++++++++++-------------- 5 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/CustomRF95.cpp b/src/CustomRF95.cpp index 9bce33f3..57e094e9 100644 --- a/src/CustomRF95.cpp +++ b/src/CustomRF95.cpp @@ -126,8 +126,7 @@ void CustomRF95::handleInterrupt() // parsing was successful, queue for our recipient mp->has_payload = true; - int res = rxDest.enqueueFromISR(mp, &higherPriWoken); // NOWAIT - fixme, if queue is full, delete older messages - assert(res == pdTRUE); + assert(rxDest.enqueueFromISR(mp, &higherPriWoken)); // NOWAIT - fixme, if queue is full, delete older messages } clearRxBuf(); // This message accepted and cleared @@ -185,4 +184,4 @@ void CustomRF95::startSend(MeshPacket *txp) int res = RH_RF95::send(radiobuf, numbytes); assert(res); -} \ No newline at end of file +} diff --git a/src/MemoryPool.h b/src/MemoryPool.h index 7b2b6514..2997d26d 100644 --- a/src/MemoryPool.h +++ b/src/MemoryPool.h @@ -66,16 +66,14 @@ public: /// Return a buffer for use by others void release(T *p) { - int res = dead.enqueue(p, 0); - assert(res == pdTRUE); + assert(dead.enqueue(p, 0)); assert(p >= buf && (p - buf) < maxElements); // sanity check to make sure a programmer didn't free something that didn't come from this pool } /// Return a buffer from an ISR, if higherPriWoken is set to true you have some work to do ;-) void releaseFromISR(T *p, BaseType_t *higherPriWoken) { - int res = dead.enqueueFromISR(p, higherPriWoken); - assert(res == pdTRUE); + assert(dead.enqueueFromISR(p, higherPriWoken)); assert(p >= buf && (p - buf) < maxElements); // sanity check to make sure a programmer didn't free something that didn't come from this pool } }; diff --git a/src/MeshService.cpp b/src/MeshService.cpp index a09b8018..2662b618 100644 --- a/src/MeshService.cpp +++ b/src/MeshService.cpp @@ -173,7 +173,7 @@ void MeshService::handleFromRadio(MeshPacket *mp) if (d) releaseToPool(d); } - assert(toPhoneQueue.enqueue(mp, 0) == pdTRUE); // FIXME, instead of failing for full queue, delete the oldest mssages + assert(toPhoneQueue.enqueue(mp, 0)); // FIXME, instead of failing for full queue, delete the oldest mssages if (mp->payload.want_response) sendNetworkPing(mp->from); diff --git a/src/PointerQueue.h b/src/PointerQueue.h index 884e523c..5dc57bd5 100644 --- a/src/PointerQueue.h +++ b/src/PointerQueue.h @@ -18,7 +18,7 @@ public: { T *p; - return this->dequeue(&p, maxWait) == pdTRUE ? p : NULL; + return this->dequeue(&p, maxWait) ? p : nullptr; } // returns a ptr or null if the queue was empty @@ -26,6 +26,6 @@ public: { T *p; - return this->dequeueFromISR(&p, higherPriWoken) == pdTRUE ? p : NULL; + return this->dequeueFromISR(&p, higherPriWoken) ? p : nullptr; } }; diff --git a/src/TypedQueue.h b/src/TypedQueue.h index 36f07fab..57c51eac 100644 --- a/src/TypedQueue.h +++ b/src/TypedQueue.h @@ -1,18 +1,22 @@ #pragma once -#include -#include +#include +#include + +#include +#include /** - * A wrapper for freertos queues. Note: each element object must be quite small, so T should be only - * pointer types or ints + * A wrapper for freertos queues. Note: each element object should be small + * and POD (Plain Old Data type) as elements are memcpied by value. */ template class TypedQueue { + static_assert(std::is_pod::value, "T must be pod"); QueueHandle_t h; -public: + public: TypedQueue(int maxElements) { h = xQueueCreate(maxElements, sizeof(T)); @@ -34,24 +38,22 @@ public: return uxQueueMessagesWaiting(h) == 0; } - // pdTRUE for success else failure - BaseType_t enqueue(T x, TickType_t maxWait = portMAX_DELAY) + bool enqueue(T x, TickType_t maxWait = portMAX_DELAY) { - return xQueueSendToBack(h, &x, maxWait); + return xQueueSendToBack(h, &x, maxWait) == pdTRUE; } - BaseType_t enqueueFromISR(T x, BaseType_t *higherPriWoken) + bool enqueueFromISR(T x, BaseType_t *higherPriWoken) { - return xQueueSendToBackFromISR(h, &x, higherPriWoken); + return xQueueSendToBackFromISR(h, &x, higherPriWoken) == pdTRUE; } - // pdTRUE for success else failure - BaseType_t dequeue(T *p, TickType_t maxWait = portMAX_DELAY) + bool dequeue(T *p, TickType_t maxWait = portMAX_DELAY) { - return xQueueReceive(h, p, maxWait); + return xQueueReceive(h, p, maxWait) == pdTRUE; } - BaseType_t dequeueFromISR(T *p, BaseType_t *higherPriWoken) + bool dequeueFromISR(T *p, BaseType_t *higherPriWoken) { return xQueueReceiveFromISR(h, p, higherPriWoken); } From 8cabb3ea3d5e9f9cff92b4ad21abc46e4ddb8586 Mon Sep 17 00:00:00 2001 From: Girts Folkmanis Date: Sun, 15 Mar 2020 19:29:35 -0700 Subject: [PATCH 5/5] add .clang-format file Tried to infer the style from existing files. --- .clang-format | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 00000000..a0e63872 --- /dev/null +++ b/.clang-format @@ -0,0 +1,6 @@ +Language: Cpp +IndentWidth: 4 +ColumnLimit: 130 +PointerAlignment: Right +BreakBeforeBraces: Linux +AllowShortFunctionsOnASingleLine: Inline