From 1674c720c38909ae7bab4a013e7f7d49fba85362 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Thu, 2 May 2019 16:09:12 +0200 Subject: [PATCH 01/23] add regular scheduled functions, now also callable on `yield()` added bool schedule_function_us(std::function fn, uint32_t repeat_us) lambda must return true to be not removed from the schedule function list if repeat_us is 0, then the function is called only once. Legacy schedule_function() is preserved Linked list management is simplified This addition allows network drivers like ethernet chips on lwIP to be regularly called - even if some user code loops on receiving data without getting out from main loop (callable from yield()) - without the need to call the driver handling function (transparent) This may be also applicable with common libraries (mDNS, Webserver, ) --- cores/esp8266/FunctionalInterrupt.cpp | 2 + cores/esp8266/FunctionalInterrupt.h | 1 - cores/esp8266/Schedule.cpp | 62 +++++++++++++-------------- cores/esp8266/Schedule.h | 12 ++++-- cores/esp8266/core_esp8266_main.cpp | 5 ++- 5 files changed, 42 insertions(+), 40 deletions(-) diff --git a/cores/esp8266/FunctionalInterrupt.cpp b/cores/esp8266/FunctionalInterrupt.cpp index 2633c63182..f98e1953f6 100644 --- a/cores/esp8266/FunctionalInterrupt.cpp +++ b/cores/esp8266/FunctionalInterrupt.cpp @@ -3,6 +3,8 @@ #include "Arduino.h" #include +static ScheduledFunctions* scheduledInterrupts; + // Duplicate typedefs from core_esp8266_wiring_digital_c typedef void (*voidFuncPtr)(void); typedef void (*voidFuncPtrArg)(void*); diff --git a/cores/esp8266/FunctionalInterrupt.h b/cores/esp8266/FunctionalInterrupt.h index a6d53188ae..260a868a87 100644 --- a/cores/esp8266/FunctionalInterrupt.h +++ b/cores/esp8266/FunctionalInterrupt.h @@ -29,7 +29,6 @@ struct ArgStructure { FunctionInfo* functionInfo = nullptr; }; -static ScheduledFunctions* scheduledInterrupts; void attachInterrupt(uint8_t pin, std::function intRoutine, int mode); void attachScheduledInterrupt(uint8_t pin, std::function scheduledIntRoutine, int mode); diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 27b9731954..e7f14e4e68 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -1,16 +1,19 @@ #include "Schedule.h" +#include "PolledTimeout.h" + +typedef std::function mFuncT; struct scheduled_fn_t { scheduled_fn_t* mNext; - std::function mFunc; + mFuncT mFunc; + esp8266::polledTimeout::periodicFastUs callNow; + + scheduled_fn_t(): callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { } }; static scheduled_fn_t* sFirst = 0; -static scheduled_fn_t* sLast = 0; - static scheduled_fn_t* sFirstUnused = 0; -static scheduled_fn_t* sLastUnused = 0; static int sCount = 0; @@ -20,9 +23,6 @@ static scheduled_fn_t* get_fn() { if (sFirstUnused) { result = sFirstUnused; sFirstUnused = result->mNext; - if (sFirstUnused == NULL) { - sLastUnused = NULL; - } } // if no unused items, and count not too high, allocate a new one else if (sCount != SCHEDULED_FN_MAX_COUNT) { @@ -35,44 +35,40 @@ static scheduled_fn_t* get_fn() { static void recycle_fn(scheduled_fn_t* fn) { - if (!sLastUnused) { - sFirstUnused = fn; - } - else { - sLastUnused->mNext = fn; - } - fn->mNext = NULL; - sLastUnused = fn; + fn->mNext = sFirstUnused; + sFirstUnused = fn; } -bool schedule_function(std::function fn) +bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { scheduled_fn_t* item = get_fn(); if (!item) { return false; } item->mFunc = fn; - item->mNext = NULL; - if (!sFirst) { - sFirst = item; - } - else { - sLast->mNext = item; - } - sLast = item; + item->mNext = sFirst; + sFirst = item; + if (repeat_us) + item->callNow.reset(repeat_us); return true; } +bool schedule_function(std::function fn) +{ + return schedule_function_us([&fn](){ fn(); return false; }, 0); +} + void run_scheduled_functions() { - scheduled_fn_t* rFirst = sFirst; - sFirst = NULL; - sLast = NULL; - while (rFirst) { - scheduled_fn_t* item = rFirst; - rFirst = item->mNext; - item->mFunc(); - item->mFunc = std::function(); - recycle_fn(item); + scheduled_fn_t* toCall = sFirst; + while (toCall) { + scheduled_fn_t* item = toCall; + toCall = item->mNext; + if (item->callNow && !item->mFunc()) { + if (sFirst == item) + sFirst = item->mNext; + item->mFunc = mFuncT(); + recycle_fn(item); + } } } diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index 3399972945..ce62cfd229 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -6,12 +6,13 @@ #define SCHEDULED_FN_MAX_COUNT 32 #define SCHEDULED_FN_INITIAL_COUNT 4 -// Warning -// This API is not considered stable. +// Warning +// This API is not considered stable. // Function signatures will change. // You have been warned. -// Run given function next time `loop` function returns, +// Run given function ONCE next time `loop` function returns, +// or `yield` is called, // or `run_scheduled_functions` is called. // Use std::bind to pass arguments to a function, or call a class member function. // Note: there is no mechanism for cancelling scheduled functions. @@ -19,7 +20,10 @@ // Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT. bool schedule_function(std::function fn); -// Run all scheduled functions. +// run given function every at least microseconds until it returns false +bool schedule_function_us(std::function fn, uint32_t repeat_us); + +// Run all scheduled functions. // Use this function if your are not using `loop`, or `loop` does not return // on a regular basis. void run_scheduled_functions(); diff --git a/cores/esp8266/core_esp8266_main.cpp b/cores/esp8266/core_esp8266_main.cpp index 5d0fb54e65..7ab1e56bf7 100644 --- a/cores/esp8266/core_esp8266_main.cpp +++ b/cores/esp8266/core_esp8266_main.cpp @@ -91,13 +91,15 @@ extern "C" void esp_yield() { } extern "C" void esp_schedule() { + // always on CONT stack here + run_scheduled_functions(); ets_post(LOOP_TASK_PRIORITY, 0, 0); } extern "C" void __yield() { if (cont_can_yield(g_pcont)) { esp_schedule(); - esp_yield(); + cont_yield(g_pcont); //esp_yield(); } else { panic(); @@ -122,7 +124,6 @@ static void loop_wrapper() { setup_done = true; } loop(); - run_scheduled_functions(); esp_schedule(); } From 4f09a64a6034b82794ec447f51a407233094f51d Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 3 May 2019 03:56:07 +0200 Subject: [PATCH 02/23] protect critical sections --- cores/esp8266/Schedule.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index e7f14e4e68..fce2c5df5e 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -46,8 +46,12 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) return false; } item->mFunc = fn; + + noInterrupts(); item->mNext = sFirst; sFirst = item; + interrupts(); + if (repeat_us) item->callNow.reset(repeat_us); return true; @@ -64,9 +68,13 @@ void run_scheduled_functions() while (toCall) { scheduled_fn_t* item = toCall; toCall = item->mNext; - if (item->callNow && !item->mFunc()) { + if (item->callNow && !item->mFunc()) + { + noInterrupts(); if (sFirst == item) sFirst = item->mNext; + interrupts(); + item->mFunc = mFuncT(); recycle_fn(item); } From 545dd350c900a17dc5aa8a2c9e1c8efdf8b49ff1 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Fri, 3 May 2019 11:05:01 +0200 Subject: [PATCH 03/23] fix emulation on host, use alternate interruopt locking method --- cores/esp8266/Arduino.h | 11 +++++++++++ cores/esp8266/Schedule.cpp | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index 34e5fb8fce..1e445859f4 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -146,6 +146,16 @@ void ets_intr_unlock(); #define __STRINGIFY(a) #a #endif +#if CORE_MOCK + +#define xt_rsil(level) (level) +#define xt_wsr_ps(state) do { (void)(state); } while (0) + +#define interrupts() do { (void)0; } while (0) +#define noInterrupts() do { (void)0; } while (0) + +#else // !CORE_MOCK + // these low level routines provide a replacement for SREG interrupt save that AVR uses // but are esp8266 specific. A normal use pattern is like // @@ -165,6 +175,7 @@ void ets_intr_unlock(); #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) +#endif // !CORE_MOCK #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index fce2c5df5e..49908878a5 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -47,10 +47,10 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) } item->mFunc = fn; - noInterrupts(); + uint32_t savedPS = xt_rsil(0); // noInterrupts(); item->mNext = sFirst; sFirst = item; - interrupts(); + xt_wsr_ps(savedPS); // interrupts(); if (repeat_us) item->callNow.reset(repeat_us); @@ -70,10 +70,10 @@ void run_scheduled_functions() toCall = item->mNext; if (item->callNow && !item->mFunc()) { - noInterrupts(); + uint32_t savedPS = xt_rsil(0); // noInterrupts(); if (sFirst == item) sFirst = item->mNext; - interrupts(); + xt_wsr_ps(savedPS); // interrupts(); item->mFunc = mFuncT(); recycle_fn(item); From aa4f87ec40f7b72b866ce5abadd4bb4e5a68b0d2 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Fri, 3 May 2019 11:55:55 +0200 Subject: [PATCH 04/23] fix emulation on host --- cores/esp8266/Arduino.h | 12 ------------ tests/host/common/Arduino.h | 19 ++----------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index 1e445859f4..a2594add09 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -146,16 +146,6 @@ void ets_intr_unlock(); #define __STRINGIFY(a) #a #endif -#if CORE_MOCK - -#define xt_rsil(level) (level) -#define xt_wsr_ps(state) do { (void)(state); } while (0) - -#define interrupts() do { (void)0; } while (0) -#define noInterrupts() do { (void)0; } while (0) - -#else // !CORE_MOCK - // these low level routines provide a replacement for SREG interrupt save that AVR uses // but are esp8266 specific. A normal use pattern is like // @@ -175,8 +165,6 @@ void ets_intr_unlock(); #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) -#endif // !CORE_MOCK - #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) #define microsecondsToClockCycles(a) ( (a) * clockCyclesPerMicrosecond() ) diff --git a/tests/host/common/Arduino.h b/tests/host/common/Arduino.h index 9b75fc114c..e6c14ff52c 100644 --- a/tests/host/common/Arduino.h +++ b/tests/host/common/Arduino.h @@ -143,26 +143,11 @@ extern "C" { #define __STRINGIFY(a) #a #endif - // these low level routines provide a replacement for SREG interrupt save that AVR uses - // but are esp8266 specific. A normal use pattern is like - // - //{ - // uint32_t savedPS = xt_rsil(1); // this routine will allow level 2 and above - // // do work here - // xt_wsr_ps(savedPS); // restore the state - //} - // - // level (0-15), interrupts of the given level and above will be active - // level 15 will disable ALL interrupts, - // level 0 will enable ALL interrupts, - // -#define xt_rsil(level) (__extension__({uint32_t state; __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (state)); state;})) -#define xt_wsr_ps(state) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (state) : "memory") - +#define xt_rsil(level) (level) +#define xt_wsr_ps(state) do { (void)(state); } while (0) #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) - #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) #define microsecondsToClockCycles(a) ( (a) * clockCyclesPerMicrosecond() ) From 896b6864ddbc9821da8ac7f8c99408d72a5d8431 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 3 May 2019 23:02:20 +0200 Subject: [PATCH 05/23] critical code protection (wip) --- cores/esp8266/Arduino.h | 10 ++++++++++ cores/esp8266/Schedule.cpp | 15 ++++++++++----- tests/host/common/c_types.h | 3 +++ tools/sdk/include/c_types.h | 3 +++ 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index a2594add09..3e5ad8b7a2 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -165,6 +165,16 @@ void ets_intr_unlock(); #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) +#define lockEnterVar(varSave, level) __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (varSave)) +#define lockLeaveVar(varSave) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (varSave) : "memory") + +#define lockDeclM() uint32_t intrState +#define lockEnterM() lockEnterVar(intrState, 0) +#define lockLeaveM() lockLeaveVar(intrState) + +#define lockEnter() lockDeclM(); lockEnterM() +#define lockLeave() lockLeaveM() + #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) #define microsecondsToClockCycles(a) ( (a) * clockCyclesPerMicrosecond() ) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 49908878a5..7db02399c2 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -17,6 +17,7 @@ static scheduled_fn_t* sFirstUnused = 0; static int sCount = 0; +IRAM_ATTR static scheduled_fn_t* get_fn() { scheduled_fn_t* result = NULL; // try to get an item from unused items list @@ -39,24 +40,28 @@ static void recycle_fn(scheduled_fn_t* fn) sFirstUnused = fn; } +IRAM_ATTR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { + lockEnter(); + scheduled_fn_t* item = get_fn(); if (!item) { + lockLeave(); return false; } item->mFunc = fn; - - uint32_t savedPS = xt_rsil(0); // noInterrupts(); item->mNext = sFirst; sFirst = item; - xt_wsr_ps(savedPS); // interrupts(); if (repeat_us) item->callNow.reset(repeat_us); + + lockLeave(); return true; } +IRAM_ATTR bool schedule_function(std::function fn) { return schedule_function_us([&fn](){ fn(); return false; }, 0); @@ -70,10 +75,10 @@ void run_scheduled_functions() toCall = item->mNext; if (item->callNow && !item->mFunc()) { - uint32_t savedPS = xt_rsil(0); // noInterrupts(); + lockEnter(); if (sFirst == item) sFirst = item->mNext; - xt_wsr_ps(savedPS); // interrupts(); + lockLeave(); item->mFunc = mFuncT(); recycle_fn(item); diff --git a/tests/host/common/c_types.h b/tests/host/common/c_types.h index 897c2cbc6e..867f2f7726 100644 --- a/tests/host/common/c_types.h +++ b/tests/host/common/c_types.h @@ -102,6 +102,9 @@ typedef enum { #define ICACHE_RODATA_ATTR #endif /* ICACHE_FLASH */ +// counterpart https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp8266-compat.h +#define IRAM_ATTR ICACHE_RAM_ATTR + #define STORE_ATTR __attribute__((aligned(4))) #ifndef __cplusplus diff --git a/tools/sdk/include/c_types.h b/tools/sdk/include/c_types.h index 40eb5bedf9..88dc147213 100644 --- a/tools/sdk/include/c_types.h +++ b/tools/sdk/include/c_types.h @@ -93,6 +93,9 @@ typedef enum { #define ICACHE_RAM_ATTR #endif /* ICACHE_FLASH */ +// counterpart https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp8266-compat.h +#define IRAM_ATTR ICACHE_RAM_ATTR + #define STORE_ATTR __attribute__((aligned(4))) #ifndef __cplusplus From fac39ed56845ddb5515336754fad897597897871 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 3 May 2019 23:22:35 +0200 Subject: [PATCH 06/23] add IRAM attrs where relevant --- cores/esp8266/PolledTimeout.h | 12 +++++++----- cores/esp8266/Schedule.cpp | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cores/esp8266/PolledTimeout.h b/cores/esp8266/PolledTimeout.h index 95157d3772..fe9c9ca4bc 100644 --- a/cores/esp8266/PolledTimeout.h +++ b/cores/esp8266/PolledTimeout.h @@ -153,7 +153,7 @@ class timeoutTemplate reset(userTimeout); } - ICACHE_RAM_ATTR + IRAM_ATTR // fast bool expired() { YieldPolicyT::execute(); //in case of DoNothing: gets optimized away @@ -162,7 +162,7 @@ class timeoutTemplate return expiredOneShot(); } - ICACHE_RAM_ATTR + IRAM_ATTR // fast operator bool() { return expired(); @@ -178,6 +178,7 @@ class timeoutTemplate return _timeout != alwaysExpired; } + IRAM_ATTR // called from ISR void reset(const timeType newUserTimeout) { reset(); @@ -185,6 +186,7 @@ class timeoutTemplate _neverExpires = (newUserTimeout < 0) || (newUserTimeout > timeMax()); } + IRAM_ATTR // called from ISR void reset() { _start = TimePolicyT::time(); @@ -208,7 +210,7 @@ class timeoutTemplate private: - ICACHE_RAM_ATTR + IRAM_ATTR // fast bool checkExpired(const timeType internalUnit) const { // canWait() is not checked here @@ -218,7 +220,7 @@ class timeoutTemplate protected: - ICACHE_RAM_ATTR + IRAM_ATTR // fast bool expiredRetrigger() { if (!canWait()) @@ -234,7 +236,7 @@ class timeoutTemplate return false; } - ICACHE_RAM_ATTR + IRAM_ATTR // fast bool expiredOneShot() const { // returns "always expired" or "has expired" diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 7db02399c2..e8937a8bfa 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -17,7 +17,7 @@ static scheduled_fn_t* sFirstUnused = 0; static int sCount = 0; -IRAM_ATTR +IRAM_ATTR // called from ISR static scheduled_fn_t* get_fn() { scheduled_fn_t* result = NULL; // try to get an item from unused items list @@ -40,7 +40,7 @@ static void recycle_fn(scheduled_fn_t* fn) sFirstUnused = fn; } -IRAM_ATTR +IRAM_ATTR // called from ISR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { lockEnter(); @@ -61,7 +61,7 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) return true; } -IRAM_ATTR +IRAM_ATTR // called from ISR bool schedule_function(std::function fn) { return schedule_function_us([&fn](){ fn(); return false; }, 0); From 6e488316c525fe3577238ae19967a11eda533da9 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 3 May 2019 23:50:34 +0200 Subject: [PATCH 07/23] add host emulation fake defines --- tests/host/common/Arduino.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/host/common/Arduino.h b/tests/host/common/Arduino.h index e6c14ff52c..ea96e9af45 100644 --- a/tests/host/common/Arduino.h +++ b/tests/host/common/Arduino.h @@ -145,9 +145,18 @@ extern "C" { #define xt_rsil(level) (level) #define xt_wsr_ps(state) do { (void)(state); } while (0) + #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) - + +#define lockEnterVar(varSave, level) do { (void)(varSave); (void)(level); } while (0) +#define lockLeaveVar(varSave) do { (void)0; } while (0) +#define lockDeclM() do { (void)0; } while (0) +#define lockEnterM() do { (void)0; } while (0) +#define lockLeaveM() do { (void)0; } while (0) +#define lockEnter() do { (void)0; } while (0) +#define lockLeave() do { (void)0; } while (0) + #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) #define microsecondsToClockCycles(a) ( (a) * clockCyclesPerMicrosecond() ) From b9bf95b46eb1268af106d4c0b43d2ea1f7079a14 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Sat, 4 May 2019 14:26:05 +0200 Subject: [PATCH 08/23] fix per https://github.com/esp8266/Arduino/pull/6039#commitcomment-33409191 @mhightower83 --- cores/esp8266/Arduino.h | 2 +- cores/esp8266/Schedule.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index 3e5ad8b7a2..61ba6aa958 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -169,7 +169,7 @@ void ets_intr_unlock(); #define lockLeaveVar(varSave) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (varSave) : "memory") #define lockDeclM() uint32_t intrState -#define lockEnterM() lockEnterVar(intrState, 0) +#define lockEnterM() lockEnterVar(intrState, 15) #define lockLeaveM() lockLeaveVar(intrState) #define lockEnter() lockDeclM(); lockEnterM() diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index e8937a8bfa..42f3ddc8af 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -54,10 +54,11 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) item->mNext = sFirst; sFirst = item; + lockLeave(); + if (repeat_us) item->callNow.reset(repeat_us); - lockLeave(); return true; } From 36ac7dd410456e6943c8ef19be807ff0cba053a2 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Sun, 5 May 2019 23:08:41 +0200 Subject: [PATCH 09/23] wonderful idea with a class and its destructor for interrupt locking --- cores/esp8266/Arduino.h | 9 --------- cores/esp8266/Esp.h | 13 ++++++++++++- cores/esp8266/Schedule.cpp | 18 ++++++++---------- tests/host/common/Arduino.h | 8 -------- tests/host/common/mock.h | 9 +++++++++ 5 files changed, 29 insertions(+), 28 deletions(-) diff --git a/cores/esp8266/Arduino.h b/cores/esp8266/Arduino.h index 61ba6aa958..34e5fb8fce 100644 --- a/cores/esp8266/Arduino.h +++ b/cores/esp8266/Arduino.h @@ -165,15 +165,6 @@ void ets_intr_unlock(); #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) -#define lockEnterVar(varSave, level) __asm__ __volatile__("rsil %0," __STRINGIFY(level) : "=a" (varSave)) -#define lockLeaveVar(varSave) __asm__ __volatile__("wsr %0,ps; isync" :: "a" (varSave) : "memory") - -#define lockDeclM() uint32_t intrState -#define lockEnterM() lockEnterVar(intrState, 15) -#define lockLeaveM() lockLeaveVar(intrState) - -#define lockEnter() lockDeclM(); lockEnterM() -#define lockLeave() lockLeaveM() #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index f717c15881..0f93336733 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -213,7 +213,18 @@ uint32_t EspClass::getCycleCount() __asm__ __volatile__("esync; rsr %0,ccount":"=a" (ccount)); return ccount; } -#endif + +class EspLockInterrupts +{ +public: + // implement automatic x=xt_rsil(15);xt_wsr_ps(x) + EspLockInterrupts () { __asm__ __volatile__("rsil %0,15" : "=a" (saveStatus)); } + ~EspLockInterrupts () { __asm__ __volatile__("wsr %0,ps; isync" :: "a" (saveStatus) : "memory"); } +private: + volatile uint32_t saveStatus; +}; + +#endif // !defined(CORE_MOCK) extern EspClass ESP; diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 42f3ddc8af..a21199d0d2 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -43,19 +43,16 @@ static void recycle_fn(scheduled_fn_t* fn) IRAM_ATTR // called from ISR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { - lockEnter(); + EspLockInterrupts lockAllInterruptsInThisBlock; scheduled_fn_t* item = get_fn(); - if (!item) { - lockLeave(); + if (!item) return false; - } + item->mFunc = fn; item->mNext = sFirst; sFirst = item; - lockLeave(); - if (repeat_us) item->callNow.reset(repeat_us); @@ -76,10 +73,11 @@ void run_scheduled_functions() toCall = item->mNext; if (item->callNow && !item->mFunc()) { - lockEnter(); - if (sFirst == item) - sFirst = item->mNext; - lockLeave(); + { + EspLockInterrupts lockAllInterruptsInThisBlock; + if (sFirst == item) + sFirst = item->mNext; + } item->mFunc = mFuncT(); recycle_fn(item); diff --git a/tests/host/common/Arduino.h b/tests/host/common/Arduino.h index ea96e9af45..02f0d9695f 100644 --- a/tests/host/common/Arduino.h +++ b/tests/host/common/Arduino.h @@ -149,14 +149,6 @@ extern "C" { #define interrupts() xt_rsil(0) #define noInterrupts() xt_rsil(15) -#define lockEnterVar(varSave, level) do { (void)(varSave); (void)(level); } while (0) -#define lockLeaveVar(varSave) do { (void)0; } while (0) -#define lockDeclM() do { (void)0; } while (0) -#define lockEnterM() do { (void)0; } while (0) -#define lockLeaveM() do { (void)0; } while (0) -#define lockEnter() do { (void)0; } while (0) -#define lockLeave() do { (void)0; } while (0) - #define clockCyclesPerMicrosecond() ( F_CPU / 1000000L ) #define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() ) #define microsecondsToClockCycles(a) ( (a) * clockCyclesPerMicrosecond() ) diff --git a/tests/host/common/mock.h b/tests/host/common/mock.h index 067411139f..15a605ae46 100644 --- a/tests/host/common/mock.h +++ b/tests/host/common/mock.h @@ -167,4 +167,13 @@ void mock_stop_spiffs (); // +class EspLockInterrupts +{ +public: + EspLockInterrupts () { } + ~EspLockInterrupts () { } +}; + +// + #endif // __cplusplus From 499d2ea49f948f2407658687d4e9d758465ae464 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Mon, 6 May 2019 02:33:43 +0200 Subject: [PATCH 10/23] remove duplicate interrupt lock class --- cores/esp8266/Esp.h | 10 ---------- cores/esp8266/FunctionalInterrupt.cpp | 2 -- cores/esp8266/FunctionalInterrupt.h | 1 + cores/esp8266/Schedule.cpp | 5 +++-- tests/host/common/mock.h | 11 +---------- 5 files changed, 5 insertions(+), 24 deletions(-) diff --git a/cores/esp8266/Esp.h b/cores/esp8266/Esp.h index 0f93336733..bc241eb392 100644 --- a/cores/esp8266/Esp.h +++ b/cores/esp8266/Esp.h @@ -214,16 +214,6 @@ uint32_t EspClass::getCycleCount() return ccount; } -class EspLockInterrupts -{ -public: - // implement automatic x=xt_rsil(15);xt_wsr_ps(x) - EspLockInterrupts () { __asm__ __volatile__("rsil %0,15" : "=a" (saveStatus)); } - ~EspLockInterrupts () { __asm__ __volatile__("wsr %0,ps; isync" :: "a" (saveStatus) : "memory"); } -private: - volatile uint32_t saveStatus; -}; - #endif // !defined(CORE_MOCK) extern EspClass ESP; diff --git a/cores/esp8266/FunctionalInterrupt.cpp b/cores/esp8266/FunctionalInterrupt.cpp index f98e1953f6..2633c63182 100644 --- a/cores/esp8266/FunctionalInterrupt.cpp +++ b/cores/esp8266/FunctionalInterrupt.cpp @@ -3,8 +3,6 @@ #include "Arduino.h" #include -static ScheduledFunctions* scheduledInterrupts; - // Duplicate typedefs from core_esp8266_wiring_digital_c typedef void (*voidFuncPtr)(void); typedef void (*voidFuncPtrArg)(void*); diff --git a/cores/esp8266/FunctionalInterrupt.h b/cores/esp8266/FunctionalInterrupt.h index 260a868a87..a6d53188ae 100644 --- a/cores/esp8266/FunctionalInterrupt.h +++ b/cores/esp8266/FunctionalInterrupt.h @@ -29,6 +29,7 @@ struct ArgStructure { FunctionInfo* functionInfo = nullptr; }; +static ScheduledFunctions* scheduledInterrupts; void attachInterrupt(uint8_t pin, std::function intRoutine, int mode); void attachScheduledInterrupt(uint8_t pin, std::function scheduledIntRoutine, int mode); diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index a21199d0d2..fb1ed38156 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -1,5 +1,6 @@ #include "Schedule.h" #include "PolledTimeout.h" +#include "interrupts.h" typedef std::function mFuncT; @@ -43,7 +44,7 @@ static void recycle_fn(scheduled_fn_t* fn) IRAM_ATTR // called from ISR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { - EspLockInterrupts lockAllInterruptsInThisBlock; + InterruptLock lockAllInterruptsInThisScope; scheduled_fn_t* item = get_fn(); if (!item) @@ -74,7 +75,7 @@ void run_scheduled_functions() if (item->callNow && !item->mFunc()) { { - EspLockInterrupts lockAllInterruptsInThisBlock; + InterruptLock lockAllInterruptsInThisScope; if (sFirst == item) sFirst = item->mNext; } diff --git a/tests/host/common/mock.h b/tests/host/common/mock.h index 15a605ae46..4d92d33d88 100644 --- a/tests/host/common/mock.h +++ b/tests/host/common/mock.h @@ -131,7 +131,7 @@ void mockUDPSwallow (size_t copied, char* ccinbuf, size_t& ccinbufsize); class UdpContext; void register_udp (int sock, UdpContext* udp = nullptr); -class InterruptLock { }; +//class InterruptLock { }; // @@ -167,13 +167,4 @@ void mock_stop_spiffs (); // -class EspLockInterrupts -{ -public: - EspLockInterrupts () { } - ~EspLockInterrupts () { } -}; - -// - #endif // __cplusplus From 83ab3835dacd10a3fe3911b6fff9a73b77742b66 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 17 May 2019 00:35:21 +0200 Subject: [PATCH 11/23] remove ScheduledFunctions per https://github.com/esp8266/Arduino/pull/6038#issuecomment-488758509 --- cores/esp8266/FunctionalInterrupt.cpp | 6 -- cores/esp8266/FunctionalInterrupt.h | 2 - cores/esp8266/ScheduledFunctions.cpp | 117 -------------------------- cores/esp8266/ScheduledFunctions.h | 51 ----------- 4 files changed, 176 deletions(-) delete mode 100644 cores/esp8266/ScheduledFunctions.cpp delete mode 100644 cores/esp8266/ScheduledFunctions.h diff --git a/cores/esp8266/FunctionalInterrupt.cpp b/cores/esp8266/FunctionalInterrupt.cpp index 2633c63182..fa57e217e5 100644 --- a/cores/esp8266/FunctionalInterrupt.cpp +++ b/cores/esp8266/FunctionalInterrupt.cpp @@ -1,7 +1,6 @@ #include #include #include "Arduino.h" -#include // Duplicate typedefs from core_esp8266_wiring_digital_c typedef void (*voidFuncPtr)(void); @@ -17,7 +16,6 @@ void ICACHE_RAM_ATTR interruptFunctional(void* arg) if (localArg->functionInfo->reqScheduledFunction) { schedule_function(std::bind(localArg->functionInfo->reqScheduledFunction,InterruptInfo(*(localArg->interruptInfo)))); -// scheduledInterrupts->scheduleFunctionReg(std::bind(localArg->functionInfo->reqScheduledFunction,InterruptInfo(*(localArg->interruptInfo))), false, true); } if (localArg->functionInfo->reqFunction) { @@ -54,10 +52,6 @@ void attachInterrupt(uint8_t pin, std::function intRoutine, int mode void attachScheduledInterrupt(uint8_t pin, std::function scheduledIntRoutine, int mode) { - if (!scheduledInterrupts) - { - scheduledInterrupts = new ScheduledFunctions(32); - } InterruptInfo* ii = new InterruptInfo; FunctionInfo* fi = new FunctionInfo; diff --git a/cores/esp8266/FunctionalInterrupt.h b/cores/esp8266/FunctionalInterrupt.h index a6d53188ae..968793e499 100644 --- a/cores/esp8266/FunctionalInterrupt.h +++ b/cores/esp8266/FunctionalInterrupt.h @@ -4,7 +4,6 @@ #include #include #include -#include extern "C" { #include "c_types.h" @@ -29,7 +28,6 @@ struct ArgStructure { FunctionInfo* functionInfo = nullptr; }; -static ScheduledFunctions* scheduledInterrupts; void attachInterrupt(uint8_t pin, std::function intRoutine, int mode); void attachScheduledInterrupt(uint8_t pin, std::function scheduledIntRoutine, int mode); diff --git a/cores/esp8266/ScheduledFunctions.cpp b/cores/esp8266/ScheduledFunctions.cpp deleted file mode 100644 index 25bc58db61..0000000000 --- a/cores/esp8266/ScheduledFunctions.cpp +++ /dev/null @@ -1,117 +0,0 @@ -/* - * ScheduledFunctions.cpp - * - * Created on: 27 apr. 2018 - * Author: Herman - */ -#include "ScheduledFunctions.h" - -std::list ScheduledFunctions::scheduledFunctions; - -ScheduledFunctions::ScheduledFunctions() -:ScheduledFunctions(UINT_MAX) -{ -} - -ScheduledFunctions::ScheduledFunctions(unsigned int reqMax) -{ - maxElements = reqMax; -} - -ScheduledFunctions::~ScheduledFunctions() { -} - -ScheduledRegistration ScheduledFunctions::insertElement(ScheduledElement se, bool front) -{ - if (countElements >= maxElements) - { - return nullptr; - } - else - { - countElements++; - if (front) - { - scheduledFunctions.push_front(se); - return scheduledFunctions.begin()->registration; - } - else - { - scheduledFunctions.push_back(se); - return scheduledFunctions.rbegin()->registration; - } - } -} - -std::list::iterator ScheduledFunctions::eraseElement(std::list::iterator it) -{ - countElements--; - return scheduledFunctions.erase(it); -} - -bool ScheduledFunctions::scheduleFunction(ScheduledFunction sf, bool continuous, bool front) -{ - return (insertElement({this,continuous,nullptr,sf}, front) == nullptr); -} - -bool ScheduledFunctions::scheduleFunction(ScheduledFunction sf) -{ - return scheduleFunction(sf, false, false); -} - -ScheduledRegistration ScheduledFunctions::scheduleFunctionReg (ScheduledFunction sf, bool continuous, bool front) -{ - return insertElement({this,continuous,std::make_shared(1),sf},front); -} - -void ScheduledFunctions::runScheduledFunctions() -{ - auto lastElement = scheduledFunctions.end(); // do not execute elements added during runScheduledFunctions - auto it = scheduledFunctions.begin(); - while (it != lastElement) - { - bool erase = false; - if (it->registration == nullptr) - { - it->function(); - } - else - { - if (it->registration.use_count() > 1) - { - it->function(); - } - else - { - erase = true; - } - } - if ((!it->continuous) || (erase)) - { - it = it->_this->eraseElement(it); - } - else - { - it++; - } - } -} - -void ScheduledFunctions::removeFunction(ScheduledRegistration sr) -{ - auto it = scheduledFunctions.begin(); - bool removed = false; - while ((!removed) && (it != scheduledFunctions.end())) - { - if (it->registration == sr) - { - it = eraseElement(it); - removed = true; - } - else - { - it++; - } - } -} - diff --git a/cores/esp8266/ScheduledFunctions.h b/cores/esp8266/ScheduledFunctions.h deleted file mode 100644 index 0129635364..0000000000 --- a/cores/esp8266/ScheduledFunctions.h +++ /dev/null @@ -1,51 +0,0 @@ -/* - * ScheduledFunctions.h - * - * Created on: 27 apr. 2018 - * Author: Herman - */ -#include "Arduino.h" -#include "Schedule.h" - -#include -#include -#include -#include - -#ifndef SCHEDULEDFUNCTIONS_H_ -#define SCHEDULEDFUNCTIONS_H_ - -typedef std::function ScheduledFunction; -typedef std::shared_ptr ScheduledRegistration; - -class ScheduledFunctions { - -public: - ScheduledFunctions(); - ScheduledFunctions(unsigned int reqMax); - virtual ~ScheduledFunctions(); - - struct ScheduledElement - { - ScheduledFunctions* _this; - bool continuous; - ScheduledRegistration registration; - ScheduledFunction function; - }; - - ScheduledRegistration insertElement(ScheduledElement se, bool front); - std::list::iterator eraseElement(std::list::iterator); - bool scheduleFunction(ScheduledFunction sf, bool continuous, bool front); - bool scheduleFunction(ScheduledFunction sf); - ScheduledRegistration scheduleFunctionReg (ScheduledFunction sf, bool continuous, bool front); - static void runScheduledFunctions(); - void removeFunction(ScheduledRegistration sr); - - - static std::list scheduledFunctions; - unsigned int maxElements; - unsigned int countElements = 0; - -}; - -#endif /* SCHEDULEDFUNCTIONS_H_ */ From c042640f163392dba00d519cf472e25ef040bfe7 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Tue, 21 May 2019 01:01:53 +0200 Subject: [PATCH 12/23] restore FIFO --- cores/esp8266/Schedule.cpp | 58 +++++++++++++++++++++++++++++--------- cores/esp8266/Schedule.h | 7 ++--- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index fb1ed38156..9af4270a99 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -14,17 +14,25 @@ struct scheduled_fn_t }; static scheduled_fn_t* sFirst = 0; +static scheduled_fn_t* sLast = 0; + static scheduled_fn_t* sFirstUnused = 0; +static scheduled_fn_t* sLastUnused = 0; static int sCount = 0; IRAM_ATTR // called from ISR static scheduled_fn_t* get_fn() { + InterruptLock lockAllInterruptsInThisScope; + scheduled_fn_t* result = NULL; // try to get an item from unused items list if (sFirstUnused) { result = sFirstUnused; sFirstUnused = result->mNext; + if (sFirstUnused == NULL) { + sLastUnused = NULL; + } } // if no unused items, and count not too high, allocate a new one else if (sCount != SCHEDULED_FN_MAX_COUNT) { @@ -37,8 +45,16 @@ static scheduled_fn_t* get_fn() { static void recycle_fn(scheduled_fn_t* fn) { - fn->mNext = sFirstUnused; - sFirstUnused = fn; + InterruptLock lockAllInterruptsInThisScope; + + if (!sLastUnused) { + sFirstUnused = fn; + } + else { + sLastUnused->mNext = fn; + } + fn->mNext = NULL; + sLastUnused = fn; } IRAM_ATTR // called from ISR @@ -47,12 +63,18 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) InterruptLock lockAllInterruptsInThisScope; scheduled_fn_t* item = get_fn(); - if (!item) + if (!item) { return false; - + } item->mFunc = fn; - item->mNext = sFirst; - sFirst = item; + item->mNext = NULL; + if (!sFirst) { + sFirst = item; + } + else { + sLast->mNext = item; + } + sLast = item; if (repeat_us) item->callNow.reset(repeat_us); @@ -68,20 +90,30 @@ bool schedule_function(std::function fn) void run_scheduled_functions() { - scheduled_fn_t* toCall = sFirst; + scheduled_fn_t* lastRecurring = nullptr; + scheduled_fn_t* toCall = sFirst; while (toCall) { scheduled_fn_t* item = toCall; toCall = item->mNext; - if (item->callNow && !item->mFunc()) + if (item->callNow) { + if (item->mFunc()) { - InterruptLock lockAllInterruptsInThisScope; - if (sFirst == item) - sFirst = item->mNext; + lastRecurring = item; } + else + { + { + InterruptLock lockAllInterruptsInThisScope; + if (sFirst == item) + sFirst = item->mNext; + if (sLast == item) + sLast = lastRecurring; + } - item->mFunc = mFuncT(); - recycle_fn(item); + item->mFunc = mFuncT(); + recycle_fn(item); + } } } } diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index ce62cfd229..ef0eb1db44 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -6,9 +6,8 @@ #define SCHEDULED_FN_MAX_COUNT 32 #define SCHEDULED_FN_INITIAL_COUNT 4 -// Warning -// This API is not considered stable. -// Function signatures will change. +// This API was not considered stable but is now fossilizing. +// Function signatures may change, queue must stay FIFO. // You have been warned. // Run given function ONCE next time `loop` function returns, @@ -20,7 +19,7 @@ // Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT. bool schedule_function(std::function fn); -// run given function every at least microseconds until it returns false +// Run given function every at least microseconds until it returns false bool schedule_function_us(std::function fn, uint32_t repeat_us); // Run all scheduled functions. From 9854ad085ed6fd37b7799e05ee5b81c685189bef Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 22 May 2019 01:16:19 +0200 Subject: [PATCH 13/23] fix comments per review --- cores/esp8266/Schedule.cpp | 4 ++-- cores/esp8266/Schedule.h | 6 ++++-- tests/host/common/mock.h | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 9af4270a99..8951eb1490 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -90,8 +90,8 @@ bool schedule_function(std::function fn) void run_scheduled_functions() { - scheduled_fn_t* lastRecurring = nullptr; - scheduled_fn_t* toCall = sFirst; + scheduled_fn_t* lastRecurring = nullptr; + scheduled_fn_t* toCall = sFirst; while (toCall) { scheduled_fn_t* item = toCall; toCall = item->mNext; diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index ef0eb1db44..2c59adbf70 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -6,7 +6,7 @@ #define SCHEDULED_FN_MAX_COUNT 32 #define SCHEDULED_FN_INITIAL_COUNT 4 -// This API was not considered stable but is now fossilizing. +// This API was not considered stable but is now stabilizing. // Function signatures may change, queue must stay FIFO. // You have been warned. @@ -19,7 +19,9 @@ // Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT. bool schedule_function(std::function fn); -// Run given function every at least microseconds until it returns false +// Run given function periodically about every microseconds until it returns false. +// Note that it may be more than microseconds between calls if `yield` is not called +// frequently, and therefore should not be used for timing critical operations. bool schedule_function_us(std::function fn, uint32_t repeat_us); // Run all scheduled functions. diff --git a/tests/host/common/mock.h b/tests/host/common/mock.h index 4d92d33d88..e681d64975 100644 --- a/tests/host/common/mock.h +++ b/tests/host/common/mock.h @@ -131,8 +131,6 @@ void mockUDPSwallow (size_t copied, char* ccinbuf, size_t& ccinbufsize); class UdpContext; void register_udp (int sock, UdpContext* udp = nullptr); -//class InterruptLock { }; - // void mock_start_spiffs (const String& fname, size_t size_kb, size_t block_kb = 8, size_t page_b = 512); From c064a58513c341fbb9b08910dcea296394f6fde7 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 22 May 2019 10:54:09 +0200 Subject: [PATCH 14/23] uodates per review --- cores/esp8266/Schedule.cpp | 66 +++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 8951eb1490..54491bfb11 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -1,3 +1,6 @@ + +#include + #include "Schedule.h" #include "PolledTimeout.h" #include "interrupts.h" @@ -22,22 +25,24 @@ static scheduled_fn_t* sLastUnused = 0; static int sCount = 0; IRAM_ATTR // called from ISR -static scheduled_fn_t* get_fn() { - InterruptLock lockAllInterruptsInThisScope; - - scheduled_fn_t* result = NULL; +static scheduled_fn_t* get_fn_unsafe() +{ + scheduled_fn_t* result = nullptr; // try to get an item from unused items list - if (sFirstUnused) { + if (sFirstUnused) + { result = sFirstUnused; - sFirstUnused = result->mNext; - if (sFirstUnused == NULL) { - sLastUnused = NULL; + sFirstUnused = sFirstUnused->mNext; + if (sFirstUnused == nullptr) + { + sLastUnused = nullptr; } } // if no unused items, and count not too high, allocate a new one - else if (sCount != SCHEDULED_FN_MAX_COUNT) { + else if (sCount != SCHEDULED_FN_MAX_COUNT) + { result = new scheduled_fn_t; - result->mNext = NULL; + result->mNext = nullptr; ++sCount; } return result; @@ -47,38 +52,40 @@ static void recycle_fn(scheduled_fn_t* fn) { InterruptLock lockAllInterruptsInThisScope; - if (!sLastUnused) { + if (!sLastUnused) + { sFirstUnused = fn; } - else { + else + { sLastUnused->mNext = fn; } - fn->mNext = NULL; + fn->mNext = nullptr; sLastUnused = fn; } IRAM_ATTR // called from ISR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { + assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //26800000us (26.8s) + InterruptLock lockAllInterruptsInThisScope; - scheduled_fn_t* item = get_fn(); - if (!item) { + scheduled_fn_t* item = get_fn_unsafe(); + if (!item) return false; - } - item->mFunc = fn; - item->mNext = NULL; - if (!sFirst) { - sFirst = item; - } - else { - sLast->mNext = item; - } - sLast = item; if (repeat_us) item->callNow.reset(repeat_us); + item->mFunc = fn; + item->mNext = nullptr; + if (sFirst) + sLast->mNext = item; + else + sFirst = item; + sLast = item; + return true; } @@ -90,9 +97,16 @@ bool schedule_function(std::function fn) void run_scheduled_functions() { + // Note to the reader: + // There is no exposed API to remove a scheduled function: + // Scheduled functions are removed in this function, and + // its purpose is that it is never called from an interrupt / + // always called from cont stack. + scheduled_fn_t* lastRecurring = nullptr; scheduled_fn_t* toCall = sFirst; - while (toCall) { + while (toCall) + { scheduled_fn_t* item = toCall; toCall = item->mNext; if (item->callNow) From 8f022c628d701472057f7dd723a79cced12ea818 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 22 May 2019 11:02:08 +0200 Subject: [PATCH 15/23] fixes per review --- cores/esp8266/Schedule.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 54491bfb11..78e10df501 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -34,17 +34,15 @@ static scheduled_fn_t* get_fn_unsafe() result = sFirstUnused; sFirstUnused = sFirstUnused->mNext; if (sFirstUnused == nullptr) - { sLastUnused = nullptr; - } } // if no unused items, and count not too high, allocate a new one else if (sCount != SCHEDULED_FN_MAX_COUNT) { result = new scheduled_fn_t; - result->mNext = nullptr; ++sCount; } + result->mNext = nullptr; return result; } @@ -79,7 +77,6 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) item->callNow.reset(repeat_us); item->mFunc = fn; - item->mNext = nullptr; if (sFirst) sLast->mNext = item; else @@ -120,7 +117,7 @@ void run_scheduled_functions() { InterruptLock lockAllInterruptsInThisScope; if (sFirst == item) - sFirst = item->mNext; + sFirst = sFirst->mNext; if (sLast == item) sLast = lastRecurring; } From e2ad457eaa4fca7f5addd26087d1dfbfe7b53192 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 22 May 2019 11:04:15 +0200 Subject: [PATCH 16/23] fixes per review --- cores/esp8266/Schedule.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 78e10df501..6fbde2ab80 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -105,7 +105,7 @@ void run_scheduled_functions() while (toCall) { scheduled_fn_t* item = toCall; - toCall = item->mNext; + toCall = toCall->mNext; if (item->callNow) { if (item->mFunc()) From 16c7e62383d63dbf40f7c9b4125bffe1cae7ee2f Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Wed, 22 May 2019 11:13:20 +0200 Subject: [PATCH 17/23] fix inverted logic missed from review --- cores/esp8266/Schedule.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 6fbde2ab80..6720fde01d 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -50,14 +50,10 @@ static void recycle_fn(scheduled_fn_t* fn) { InterruptLock lockAllInterruptsInThisScope; - if (!sLastUnused) - { - sFirstUnused = fn; - } - else - { + if (sLastUnused) sLastUnused->mNext = fn; - } + else + sFirstUnused = fn; fn->mNext = nullptr; sLastUnused = fn; } @@ -65,7 +61,7 @@ static void recycle_fn(scheduled_fn_t* fn) IRAM_ATTR // called from ISR bool schedule_function_us(mFuncT fn, uint32_t repeat_us) { - assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //26800000us (26.8s) + assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //~26800000us (26.8s) InterruptLock lockAllInterruptsInThisScope; @@ -96,9 +92,9 @@ void run_scheduled_functions() { // Note to the reader: // There is no exposed API to remove a scheduled function: - // Scheduled functions are removed in this function, and - // its purpose is that it is never called from an interrupt / - // always called from cont stack. + // Scheduled functions are removed only from this function, and + // its purpose is that it is never called from an interrupt + // (always on cont stack). scheduled_fn_t* lastRecurring = nullptr; scheduled_fn_t* toCall = sFirst; From 7982a7f305fefe7557b7ca22991ff406239676b1 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Thu, 23 May 2019 10:11:09 +0200 Subject: [PATCH 18/23] fix per review https://github.com/d-a-v/Arduino/pull/6 (1/2) --- cores/esp8266/Schedule.cpp | 47 +++++++++++++++----------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 6720fde01d..0add60a61f 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -16,11 +16,10 @@ struct scheduled_fn_t scheduled_fn_t(): callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { } }; -static scheduled_fn_t* sFirst = 0; -static scheduled_fn_t* sLast = 0; +static scheduled_fn_t* sFirst = nullptr; +static scheduled_fn_t* sLast = nullptr; -static scheduled_fn_t* sFirstUnused = 0; -static scheduled_fn_t* sLastUnused = 0; +static scheduled_fn_t* sUnused = nullptr; static int sCount = 0; @@ -29,15 +28,13 @@ static scheduled_fn_t* get_fn_unsafe() { scheduled_fn_t* result = nullptr; // try to get an item from unused items list - if (sFirstUnused) + if (sUnused) { - result = sFirstUnused; - sFirstUnused = sFirstUnused->mNext; - if (sFirstUnused == nullptr) - sLastUnused = nullptr; + result = sUnused; + sUnused = sUnused->mNext; } // if no unused items, and count not too high, allocate a new one - else if (sCount != SCHEDULED_FN_MAX_COUNT) + else if (sCount < SCHEDULED_FN_MAX_COUNT) { result = new scheduled_fn_t; ++sCount; @@ -46,16 +43,11 @@ static scheduled_fn_t* get_fn_unsafe() return result; } -static void recycle_fn(scheduled_fn_t* fn) +static void recycle_fn_unsafe(scheduled_fn_t* fn) { - InterruptLock lockAllInterruptsInThisScope; - - if (sLastUnused) - sLastUnused->mNext = fn; - else - sFirstUnused = fn; - fn->mNext = nullptr; - sLastUnused = fn; + fn->mFunc = mFuncT(); + fn->mNext = sUnused; + sUnused = fn; } IRAM_ATTR // called from ISR @@ -110,16 +102,13 @@ void run_scheduled_functions() } else { - { - InterruptLock lockAllInterruptsInThisScope; - if (sFirst == item) - sFirst = sFirst->mNext; - if (sLast == item) - sLast = lastRecurring; - } - - item->mFunc = mFuncT(); - recycle_fn(item); + InterruptLock lockAllInterruptsInThisScope; + if (sFirst == item) + sFirst = sFirst->mNext; + if (sLast == item) + sLast = lastRecurring; + + recycle_fn_unsafe(item); } } } From 65603a3b927bbb9507a8ac3322406f9e08a7b0cc Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Thu, 23 May 2019 10:32:28 +0200 Subject: [PATCH 19/23] fix dangling pointer per https://github.com/d-a-v/Arduino/pull/6 last point - thanks! --- cores/esp8266/Schedule.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 0add60a61f..de886021c1 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -103,8 +103,12 @@ void run_scheduled_functions() else { InterruptLock lockAllInterruptsInThisScope; + if (sFirst == item) sFirst = sFirst->mNext; + else if (lastRecurring) + lastRecurring->mNext = item->mNext; + if (sLast == item) sLast = lastRecurring; From 24474c8d8845dacb0ff391d0f8a2f83194d82298 Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Thu, 23 May 2019 10:53:45 +0200 Subject: [PATCH 20/23] pass lambdas with const refs --- cores/esp8266/Schedule.cpp | 6 +++--- cores/esp8266/Schedule.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index de886021c1..99d27d0e55 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -51,7 +51,7 @@ static void recycle_fn_unsafe(scheduled_fn_t* fn) } IRAM_ATTR // called from ISR -bool schedule_function_us(mFuncT fn, uint32_t repeat_us) +bool schedule_function_us(const mFuncT& fn, uint32_t repeat_us) { assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //~26800000us (26.8s) @@ -75,9 +75,9 @@ bool schedule_function_us(mFuncT fn, uint32_t repeat_us) } IRAM_ATTR // called from ISR -bool schedule_function(std::function fn) +bool schedule_function(const std::function& fn) { - return schedule_function_us([&fn](){ fn(); return false; }, 0); + return schedule_function_us([fn](){ fn(); return false; }, 0); } void run_scheduled_functions() diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index 2c59adbf70..62b5d84f84 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -17,12 +17,12 @@ // Note: there is no mechanism for cancelling scheduled functions. // Keep that in mind when binding functions to objects which may have short lifetime. // Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT. -bool schedule_function(std::function fn); +bool schedule_function(const std::function& fn); // Run given function periodically about every microseconds until it returns false. // Note that it may be more than microseconds between calls if `yield` is not called // frequently, and therefore should not be used for timing critical operations. -bool schedule_function_us(std::function fn, uint32_t repeat_us); +bool schedule_function_us(const std::function& fn, uint32_t repeat_us); // Run all scheduled functions. // Use this function if your are not using `loop`, or `loop` does not return From d9c227056e67fea00c6dff178c4791b7858ea324 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Wed, 22 May 2019 22:03:11 +0200 Subject: [PATCH 21/23] Proposed changes from review --- cores/esp8266/Schedule.cpp | 26 ++++++++++++++++++-------- cores/esp8266/Schedule.h | 2 ++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index 99d27d0e55..ceb5ca064c 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -9,11 +9,11 @@ typedef std::function mFuncT; struct scheduled_fn_t { - scheduled_fn_t* mNext; + scheduled_fn_t* mNext = nullptr; mFuncT mFunc; esp8266::polledTimeout::periodicFastUs callNow; - scheduled_fn_t(): callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { } + scheduled_fn_t() : callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { } }; static scheduled_fn_t* sFirst = nullptr; @@ -32,6 +32,7 @@ static scheduled_fn_t* get_fn_unsafe() { result = sUnused; sUnused = sUnused->mNext; + result->mNext = nullptr; } // if no unused items, and count not too high, allocate a new one else if (sCount < SCHEDULED_FN_MAX_COUNT) @@ -39,19 +40,18 @@ static scheduled_fn_t* get_fn_unsafe() result = new scheduled_fn_t; ++sCount; } - result->mNext = nullptr; return result; } static void recycle_fn_unsafe(scheduled_fn_t* fn) { - fn->mFunc = mFuncT(); + fn->mFunc = nullptr; // special overload in c++ std lib fn->mNext = sUnused; sUnused = fn; } -IRAM_ATTR // called from ISR -bool schedule_function_us(const mFuncT& fn, uint32_t repeat_us) +IRAM_ATTR // (not only) called from ISR +bool schedule_function_us(std::function&& fn, uint32_t repeat_us) { assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //~26800000us (26.8s) @@ -74,10 +74,20 @@ bool schedule_function_us(const mFuncT& fn, uint32_t repeat_us) return true; } +bool ICACHE_RAM_ATTR schedule_function_us(const std::function& fn, uint32_t repeat_us) +{ + return schedule_function_us(std::function(fn), repeat_us); +} + IRAM_ATTR // called from ISR -bool schedule_function(const std::function& fn) +bool schedule_function(std::function&& fn) +{ + return schedule_function_us([fn]() { fn(); return false; }, 0); +} + +bool ICACHE_RAM_ATTR schedule_function(const std::function& fn) { - return schedule_function_us([fn](){ fn(); return false; }, 0); + return schedule_function(std::function(fn)); } void run_scheduled_functions() diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index 62b5d84f84..26da90519e 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -17,11 +17,13 @@ // Note: there is no mechanism for cancelling scheduled functions. // Keep that in mind when binding functions to objects which may have short lifetime. // Returns false if the number of scheduled functions exceeds SCHEDULED_FN_MAX_COUNT. +//bool schedule_function(std::function&& fn); bool schedule_function(const std::function& fn); // Run given function periodically about every microseconds until it returns false. // Note that it may be more than microseconds between calls if `yield` is not called // frequently, and therefore should not be used for timing critical operations. +//bool schedule_function_us(std::function&& fn, uint32_t repeat_us); bool schedule_function_us(const std::function& fn, uint32_t repeat_us); // Run all scheduled functions. From df839c2a37e7ebf2838011e3d8d03c7ac35685ef Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Thu, 23 May 2019 12:04:33 +0200 Subject: [PATCH 22/23] Initial count not applied anymore --- cores/esp8266/Schedule.h | 1 - 1 file changed, 1 deletion(-) diff --git a/cores/esp8266/Schedule.h b/cores/esp8266/Schedule.h index 26da90519e..bda3ebf48e 100644 --- a/cores/esp8266/Schedule.h +++ b/cores/esp8266/Schedule.h @@ -4,7 +4,6 @@ #include #define SCHEDULED_FN_MAX_COUNT 32 -#define SCHEDULED_FN_INITIAL_COUNT 4 // This API was not considered stable but is now stabilizing. // Function signatures may change, queue must stay FIFO. From 8e06c30c9039c1782c71fbb1739d68f05514a4cf Mon Sep 17 00:00:00 2001 From: David Gauchard Date: Thu, 23 May 2019 12:42:38 +0200 Subject: [PATCH 23/23] cosmetics --- cores/esp8266/Schedule.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cores/esp8266/Schedule.cpp b/cores/esp8266/Schedule.cpp index ceb5ca064c..bf968743e2 100644 --- a/cores/esp8266/Schedule.cpp +++ b/cores/esp8266/Schedule.cpp @@ -74,7 +74,8 @@ bool schedule_function_us(std::function&& fn, uint32_t repeat_us) return true; } -bool ICACHE_RAM_ATTR schedule_function_us(const std::function& fn, uint32_t repeat_us) +IRAM_ATTR // (not only) called from ISR +bool schedule_function_us(const std::function& fn, uint32_t repeat_us) { return schedule_function_us(std::function(fn), repeat_us); } @@ -85,7 +86,8 @@ bool schedule_function(std::function&& fn) return schedule_function_us([fn]() { fn(); return false; }, 0); } -bool ICACHE_RAM_ATTR schedule_function(const std::function& fn) +IRAM_ATTR // called from ISR +bool schedule_function(const std::function& fn) { return schedule_function(std::function(fn)); }