mikaelpatel / Arduino-Scheduler

Portable Cooperative Multi-tasking Scheduler for Arduino
https://mikaelpatel.github.io/Arduino-Scheduler/
163 stars 41 forks source link

Multi-tasking support for esp8266 #11

Closed anmaped closed 7 years ago

anmaped commented 7 years ago

It requires integration with Arduino esp8266 runtime.

There are space for several tasks but we need to manage the heap end address that is statically assigned in the Arduino esp8266. I have tested it with several tasks without any issue. Note that a task is created to run loop() since we need to return the loop_task() to allow that the runtime of the esp8266 SDK runs.

Issues/Improvements:

mikaelpatel commented 7 years ago

I guess that you have not regression tested this on any other configuration (i.e. boards). There are some changes that I would avoid especially shed() and macro. Might need to explain this so that I understand the issue.

anmaped commented 7 years ago

@mikaelpatel Yes. I have tested on AVR (atmega328p). Take a look here to understand better why the previous version is unsafe. The elaboration order of the construction of the Scheduler object and others is an issue.

mikaelpatel commented 7 years ago

I understood that you was concerned with the order of construction but I could not see how that would apply in this case. Could you elaborate? The constructor is empty as this is more or less a name space. All data is static.

anmaped commented 7 years ago

The constructor is implicit. It initializes the variables (even they are statically assigned). If you use the uninitialized variables then the scheduler will crash (there is no elaboration order in c++, by default it is undefined). They are elaborated as the code is compiled.

For instance, assuming that we are using a class that calls a scheduler class function there is 50% of chance to crash depending on the compilation order. Using direct calls on setup or loop does not make any difference and there is no issue. The problem is when you use other classes that calls an uninitialized scheduler.

Static initialization order is undefined, and the most elegant way around it is to wrap the initialization in a function.

anmaped commented 7 years ago

I want to increase the stack safety. Maybe instructing the compiler to do that. Do you have any idea ?

mikaelpatel commented 7 years ago

The constructor will not initiate static class data. That is a misconception. The constructor is only responsible for the construction of instances. Static class data is initiated as normal static data.

In any case I understand your concern but the risk in this case is very small to none. I thought you had a test case, found an error, that you had fixed with this construct. A small test program that shows this error would convince me.

mikaelpatel commented 7 years ago

Could you define "stack safety". Fill with a predefined pattern. Increase head room?

anmaped commented 7 years ago

Probably you misunderstood my point. I have found it on esp8266 because the value of the s_top. Imagine what happen with s_top undefined value.

uint8_t stack[s_top - frame];
if (s_main.stack == NULL) s_main.stack = stack;

Note that in esp8266 the code is more elaborated, more code more things. On AVR I never found it, but it exists, and there is chance for it.

mikaelpatel commented 7 years ago

I think your assumption is that Scheduler::start() is called by a constructor in another class. That could cause the issues you are describing. If Scheduler::start() is restricted to setup() function(s) I believe we can simply ignore this. But again I would like to see a test case that generates the issue.

I would also like to confirm that you agree that the class static data is not initiated by a constructor.

anmaped commented 7 years ago

I will confirm it (I don't know what the SDK of the esp8266 do, we only feed a function as the user_init() and I don't know who calls it since it's a closed SDK).

Do you have experience with GCC flags -fstack-check and -fstack-limit-symbol=__stack_limit ? I need to see what the compiler do for Xtensa architecture. This combined with coloring that can be tested at each yield is a robust approach though.