Skip to content

Proposed changes from review #6

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 18 additions & 8 deletions cores/esp8266/Schedule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ typedef std::function<bool(void)> 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;
Expand All @@ -32,26 +32,26 @@ 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)
{
result = new scheduled_fn_t;
++sCount;
}
result->mNext = nullptr;
Copy link
Author

Choose a reason for hiding this comment

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

otherwise null pointer access if SCHEDULED_FN_MAX_COUNT is exhausted

return result;
}

static void recycle_fn_unsafe(scheduled_fn_t* fn)
{
fn->mFunc = mFuncT();
fn->mFunc = nullptr; // special overload in c++ std lib
Copy link
Author

Choose a reason for hiding this comment

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

probably for performance, so use this style instead of move assignment

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<bool(void)>&& fn, uint32_t repeat_us)
{
assert(repeat_us < decltype(scheduled_fn_t::callNow)::neverExpires); //~26800000us (26.8s)

Expand All @@ -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<bool(void)>& fn, uint32_t repeat_us)
{
return schedule_function_us(std::function<bool(void)>(fn), repeat_us);
}

IRAM_ATTR // called from ISR
bool schedule_function(const std::function<void(void)>& fn)
bool schedule_function(std::function<void(void)>&& fn)
{
return schedule_function_us([fn]() { fn(); return false; }, 0);
}

bool ICACHE_RAM_ATTR schedule_function(const std::function<void(void)>& fn)
{
return schedule_function_us([fn](){ fn(); return false; }, 0);
return schedule_function(std::function<void(void)>(fn));
}

void run_scheduled_functions()
Expand Down
3 changes: 2 additions & 1 deletion cores/esp8266/Schedule.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <functional>

#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.
Expand All @@ -17,11 +16,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<void(void)>&& fn);
Copy link
Author

Choose a reason for hiding this comment

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

Pending PR esp8266#6129

bool schedule_function(const std::function<void(void)>& fn);

// 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);
Copy link
Author

Choose a reason for hiding this comment

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

Pending PR esp8266#6129

bool schedule_function_us(const std::function<bool(void)>& fn, uint32_t repeat_us);

// Run all scheduled functions.
Expand Down