Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add regular scheduled functions, now also callable on yield() #6039

Merged
merged 35 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1674c72
add regular scheduled functions, now also callable on `yield()`
d-a-v May 2, 2019
4f09a64
protect critical sections
d-a-v May 3, 2019
b6564c2
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 3, 2019
545dd35
fix emulation on host, use alternate interruopt locking method
d-a-v May 3, 2019
aa4f87e
fix emulation on host
d-a-v May 3, 2019
896b686
critical code protection (wip)
d-a-v May 3, 2019
fac39ed
add IRAM attrs where relevant
d-a-v May 3, 2019
6e48831
add host emulation fake defines
d-a-v May 3, 2019
b9bf95b
fix per https://github.com/esp8266/Arduino/pull/6039#commitcomment-33…
d-a-v May 4, 2019
37128a2
Merge branch 'master' into recurrentscheduledfunctions
earlephilhower May 5, 2019
36ac7dd
wonderful idea with a class and its destructor for interrupt locking
d-a-v May 5, 2019
499d2ea
remove duplicate interrupt lock class
d-a-v May 6, 2019
19955d8
Merge commit '625c3a62c4991347e8298fb5e4021bc6f6df7099' into recurren…
d-a-v May 13, 2019
fc9ff2a
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 14, 2019
97fde51
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 15, 2019
cce44be
Merge branch 'recurrentscheduledfunctions' of github.com:d-a-v/Arduin…
d-a-v May 16, 2019
83ab383
remove ScheduledFunctions per https://github.com/esp8266/Arduino/pull…
d-a-v May 16, 2019
8f1953e
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 16, 2019
d2e5fbc
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 18, 2019
58cfb7b
Merge branch 'master' into recurrentscheduledfunctions
d-a-v May 20, 2019
c042640
restore FIFO
d-a-v May 20, 2019
9854ad0
fix comments per review
d-a-v May 21, 2019
e42d104
Merge branch 'recurrentscheduledfunctions' of github.com:d-a-v/Arduin…
d-a-v May 22, 2019
c064a58
uodates per review
d-a-v May 22, 2019
8f022c6
fixes per review
d-a-v May 22, 2019
e2ad457
fixes per review
d-a-v May 22, 2019
16c7e62
fix inverted logic missed from review
d-a-v May 22, 2019
7982a7f
fix per review https://github.com/d-a-v/Arduino/pull/6 (1/2)
d-a-v May 23, 2019
65603a3
fix dangling pointer per https://github.com/d-a-v/Arduino/pull/6 last…
d-a-v May 23, 2019
24474c8
pass lambdas with const refs
d-a-v May 23, 2019
d9c2270
Proposed changes from review
dok-net May 22, 2019
df839c2
Initial count not applied anymore
dok-net May 23, 2019
40dbcc1
Merge pull request #6 from dok-net/d-a-v/recurrentscheduledfunctions
d-a-v May 23, 2019
8e06c30
cosmetics
d-a-v May 23, 2019
a98cf9d
Merge branch 'master' into recurrentscheduledfunctions
earlephilhower May 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cores/esp8266/Esp.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ uint32_t EspClass::getCycleCount()
__asm__ __volatile__("esync; rsr %0,ccount":"=a" (ccount));
return ccount;
}
#endif

#endif // !defined(CORE_MOCK)

extern EspClass ESP;

Expand Down
6 changes: 0 additions & 6 deletions cores/esp8266/FunctionalInterrupt.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <FunctionalInterrupt.h>
#include <Schedule.h>
#include "Arduino.h"
#include <ScheduledFunctions.h>

// Duplicate typedefs from core_esp8266_wiring_digital_c
typedef void (*voidFuncPtr)(void);
Expand All @@ -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)
{
Expand Down Expand Up @@ -54,10 +52,6 @@ void attachInterrupt(uint8_t pin, std::function<void(void)> intRoutine, int mode

void attachScheduledInterrupt(uint8_t pin, std::function<void(InterruptInfo)> scheduledIntRoutine, int mode)
{
if (!scheduledInterrupts)
{
scheduledInterrupts = new ScheduledFunctions(32);
}
InterruptInfo* ii = new InterruptInfo;

FunctionInfo* fi = new FunctionInfo;
Expand Down
2 changes: 0 additions & 2 deletions cores/esp8266/FunctionalInterrupt.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <stddef.h>
#include <stdint.h>
#include <functional>
#include <ScheduledFunctions.h>

extern "C" {
#include "c_types.h"
Expand All @@ -29,7 +28,6 @@ struct ArgStructure {
FunctionInfo* functionInfo = nullptr;
};

static ScheduledFunctions* scheduledInterrupts;
void attachInterrupt(uint8_t pin, std::function<void(void)> intRoutine, int mode);
void attachScheduledInterrupt(uint8_t pin, std::function<void(InterruptInfo)> scheduledIntRoutine, int mode);

Expand Down
12 changes: 7 additions & 5 deletions cores/esp8266/PolledTimeout.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -162,7 +162,7 @@ class timeoutTemplate
return expiredOneShot();
}

ICACHE_RAM_ATTR
IRAM_ATTR // fast
operator bool()
{
return expired();
Expand All @@ -178,13 +178,15 @@ class timeoutTemplate
return _timeout != alwaysExpired;
}

IRAM_ATTR // called from ISR
void reset(const timeType newUserTimeout)
{
reset();
_timeout = TimePolicyT::toTimeTypeUnit(newUserTimeout);
_neverExpires = (newUserTimeout < 0) || (newUserTimeout > timeMax());
}

IRAM_ATTR // called from ISR
void reset()
{
_start = TimePolicyT::time();
Expand All @@ -208,7 +210,7 @@ class timeoutTemplate

private:

ICACHE_RAM_ATTR
IRAM_ATTR // fast
bool checkExpired(const timeType internalUnit) const
{
// canWait() is not checked here
Expand All @@ -218,7 +220,7 @@ class timeoutTemplate

protected:

ICACHE_RAM_ATTR
IRAM_ATTR // fast
bool expiredRetrigger()
{
if (!canWait())
Expand All @@ -234,7 +236,7 @@ class timeoutTemplate
return false;
}

ICACHE_RAM_ATTR
IRAM_ATTR // fast
bool expiredOneShot() const
{
// returns "always expired" or "has expired"
Expand Down
63 changes: 52 additions & 11 deletions cores/esp8266/Schedule.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
#include "Schedule.h"
#include "PolledTimeout.h"
#include "interrupts.h"

typedef std::function<bool(void)> mFuncT;

struct scheduled_fn_t
{
scheduled_fn_t* mNext;
std::function<void(void)> mFunc;
mFuncT mFunc;
esp8266::polledTimeout::periodicFastUs callNow;

scheduled_fn_t(): callNow(esp8266::polledTimeout::periodicFastUs::alwaysExpired) { }
};

static scheduled_fn_t* sFirst = 0;
Expand All @@ -14,7 +21,10 @@ 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) {
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -35,6 +45,8 @@ static scheduled_fn_t* get_fn() {

static void recycle_fn(scheduled_fn_t* fn)
{
InterruptLock lockAllInterruptsInThisScope;
d-a-v marked this conversation as resolved.
Show resolved Hide resolved

if (!sLastUnused) {
sFirstUnused = fn;
}
Expand All @@ -45,8 +57,11 @@ static void recycle_fn(scheduled_fn_t* fn)
sLastUnused = fn;
}

bool schedule_function(std::function<void(void)> fn)
IRAM_ATTR // called from ISR
bool schedule_function_us(mFuncT fn, uint32_t repeat_us)
{
InterruptLock lockAllInterruptsInThisScope;
d-a-v marked this conversation as resolved.
Show resolved Hide resolved

scheduled_fn_t* item = get_fn();
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
if (!item) {
return false;
Expand All @@ -60,19 +75,45 @@ bool schedule_function(std::function<void(void)> fn)
sLast->mNext = item;
}
sLast = item;

if (repeat_us)
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
item->callNow.reset(repeat_us);
d-a-v marked this conversation as resolved.
Show resolved Hide resolved

return true;
}

IRAM_ATTR // called from ISR
bool schedule_function(std::function<void(void)> fn)
{
return schedule_function_us([&fn](){ fn(); return false; }, 0);
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
}

void run_scheduled_functions()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO: This linked-list implementation is not - probably never was - preemption safe, generally a compiler will keep the values of all the pointers in registers, even on the single-core ESP8266 an IRQ will not flush the registers but just push them to the stack and restore them, therefore any IRQ that's scheduling functions fails during an ongoing run_scheduled_functions(). Blocking IRQs during the complete execution of run_scheduled_functions makes it thread/IRQ safe, but I don't think this is permissible from an IRQ performance POV.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.
We don't want to lock while executing the scheduled function themselvses.
One solution is to tag variables with volatile.
Another one is to build a local copy of the list while being locked, then unlock and run that list.

Copy link
Collaborator Author

@d-a-v d-a-v May 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I must step back on this.
Locking IRQ is the right thing to do when the value of a variable can be modified in a TAS block.
Even if there are registers holding some variables, they will not be changed while in a locked block because an IRQ won't occur in that block, regardless whether the variable is cached in a register.

I think the compiler will not / must not optimize a variable into registers when is it not declared locally.

d-a-v marked this conversation as resolved.
Show resolved Hide resolved
{
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<void(void)>();
recycle_fn(item);
scheduled_fn_t* lastRecurring = nullptr;
scheduled_fn_t* toCall = sFirst;
while (toCall) {
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
scheduled_fn_t* item = toCall;
toCall = item->mNext;
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
if (item->callNow)
{
if (item->mFunc())
{
lastRecurring = item;
}
else
{
{
InterruptLock lockAllInterruptsInThisScope;
if (sFirst == item)
sFirst = item->mNext;
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
if (sLast == item)
sLast = lastRecurring;
}

item->mFunc = mFuncT();
recycle_fn(item);
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
15 changes: 10 additions & 5 deletions cores/esp8266/Schedule.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,25 @@
#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 stabilizing.
// Function signatures may change, queue must stay FIFO.
// 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.
// 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<void(void)> fn);

// Run all scheduled functions.
// Run given function periodically about every <repeat_us> microseconds until it returns false.
// Note that it may be more than <repeat_us> 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<bool(void)> fn, uint32_t repeat_us);
devyte marked this conversation as resolved.
Show resolved Hide resolved

// 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();
Expand Down
117 changes: 0 additions & 117 deletions cores/esp8266/ScheduledFunctions.cpp

This file was deleted.

Loading