avoid circular #include dependence for PolledTimeout

[d-a-v]

edit / change:
This PR removes #include <Arduino.h> from PolledTimeout.h thus avoid circular dependencies in some cases.
(cf. last line from OP below)
Title is changed accordingly.

OP:
Using delay() in the main loop is not efficient in the esp8266 arduino core.
PolledTimeout has beed added a while ago to help users avoiding it.

PolledTimeout is not included by default and being hidden in a deep namespace hierarchy makes its access difficult to the average arduino user.

This PR brings it at top level. It is now easier for a user to understand and remember how to transform

void loop ()
{
   handleWebServer();
   doSomething();
   //delay(1000); // breaks webserver!  <= wrong
   delay(1000); // reduces webserver performances!
}

to

void loop ()
{
    handleWebServer();

    static periodicMs doItNow(1000);
    if (doItNow) {
        doSomething();
    }
}

or

periodicMs doItNow(1000);

void loop ()
{
    handleWebServer();
    if (doItNow) {
        doSomething();
    }
}

All other useful classes in PolledTimeout are also available.
Documentation for PolledTimeoud is in an example (updated in this PR)

Also adding ::resetAndSetExpired([period]) to mark a periodic to be triggered at next check, and optionally change its period.

All other changes are due to circular inclusions with Arduino.h.

[dok-net]

I see multiple issues with this PR, in that it works against PRs from me - either those still pending, or by undoing performance improvements in already merges PRs.
Simply speaking, this PR suffers a separation-of-concern failure, that makes it kind of hard to address in short time.

[dok-net]

To start off, though, PolledTimeout is good for stackless coroutine programming, but replacing delay(), intended in Arduino to be shimmed by multitasking libraries, prominently here from my POV the CoopTask implementation, breaks that cooperative multitasking model of stackful coroutines.
That said, running with CoopTask, the code sample

void loop ()
{
   handleWebServer();
   doSomething();
   delay(1000); // breaks webserver!
}

incorrectly claims that delay() breaks the webserver, because the webserver runs just fine as a CoopTask task.
Please also remember that the work done for Scheduler.h|cpp allows coroutines to run during delay() suspensions.

[dok-net]

The change from clockCyclesPerMicrosecond() to esp_get_cpu_freq_mhz(), OTTOH, disables the compile-time constexpr work I've done. I think this is merged already. Likewise, the relocating of millis() etc. may be a serious merge obstacle to long standing pending PRs.

[d-a-v]

Dear dok-net, I'm sorry to know that you see so many issues in this trivial PR.

it works against PRs from me by undoing performance improvements in already merged PRs

Can you name them and show how performances are degraded ?

[the example] incorrectly claims that delay() breaks the webserver, because the webserver runs just fine as a CoopTask task.

Lots of snippets seen in issues of this repository use that kind of delay in the main loop, including examples (that also need to be fixed). The delay() as shown above reduces webserver performances (which are already low) when cooptask library is not used, which is the default. This is then not an incorrect claim - I can change to "breaks webserver peformances!" if it can help with your interpretation.

The change from clockCyclesPerMicrosecond() to esp_get_cpu_freq_mhz()

That is a rename I can make in this PR, not a problem.

Likewise, the relocating of millis() etc. may be a serious merge obstacle to long standing pending PRs.

This is a supposition from your part. millis() was in Arduino.h, now in features.h included by PolledTimeout.h included by Esp.h included by Arduino.h.
I'm using that change from a long time in my work, including by using third-party libraries. Not saying it is perfect though. And if millis() not directly in Arduino.h is really an issue, that can be easily solved.

This PR suffers a separation-of-concern failure that makes it kind of hard to address in short time.

It is targetted for version 3.0.0 so there is plenty of time. You seem to see a general failure or much complexity where this PR simply adds an include file and imports a global conflictless namespace. I'm sorry to admit that I have a general failure in trying to understand what "separation-of-concern failure" means here.

You are free to explain to not use polledTimeout in your library's documentation. But you cannot claim that polledTimeout is counter-productive nor not needed in the provided example in OP. It was also originally made for that purpose.

[dok-net]

Dear @d-a-v, I have no strong feelings about the central change underlying this PR:

using namespace esp8266::polledTimeout;

What does cause me some concern, though, is that there are a handful of changes in this PR, that are unrelated to that namespace matter. That is what I refer to as separation of concern issue. If you could split it into individual PRs, it would make it easier to discuss the issues.

To get you started on an obvious problem, I think there's an undetected compile-time failure caused by one of the changes. Where you add an implementation of uint8_t getCpuFreqMHz() in cores/esp8266/Esp.h, you're ignoring the existing implementation in Esp.cpp.

[dok-net]

Furthermore, the example code change you give in this PR, labelled as "transformation", is in fact a completely different style of programming for all except the most trivial "Hello world" program code. I'm in no way passing judgement on whether commonplace AVR Arduino-like code using delay(), stateful/stackful coroutines using delay() in multiple tasks (loop-like functions) running off an AVR Arduino-compatible MT extension, or stackless coroutines running either off PolledTimeout or Schedule.h are "better suited" in any conceivable use case. It's just that this cannot be described as a transformation to unsuspecting users, and it's also a portability issue.

[d-a-v]

@dok-net I'm sure I have seen this or even done this on AVR before, not you ?

    // declaration: unsigned long for lastTime and intervalMs
    if (millis() - lastTime > intervalMs) {
        lastTime += intervalMs;
        digitalWrite(LED_BUILTIN, 1 - digitalRead(LED_BUILTIN));
    }

That makes me think that we could port the polledTimeout library to be arduino compatible on all architectures, so we could on a pro mini

    // declaration: periodicMs doItNow(intervalMs);
    if (doItNow) {
        digitalWrite(LED_BUILTIN, 1 - digitalRead(LED_BUILTIN));
    }

What do you think ?
It allows to easily make two leds blink at different frequencies compared to the usage of delay(), and with only one stack. Nice no ?

[dok-net]

@d-a-v No objections, just note that this never had anything to do with delay in the first place. It's a different programming paradigm.

[TD-er]

What's striking me in this PR is that it is calling using namespace in a .h file.
I get that's more or less the intent of this PR, but in general calling using namespace in a .h file makes the success of compiling depending on the order of includes which in general is a bad idea.
What's wrong with adding the namespace as a prefix to the objects you try to create? If it is too much of a hassle, then call using namespace in the .cpp file or for that matter in the .ino

[d-a-v]

polledTimeout is a helper, in that sense it is not strictly necessary.
Its namespace is unnecessary long and difficult to remember on a everyday use basis.

The goal of this PR is to make it easy to use, as presented in the OP.

[TD-er]

I can understand why it may be a good thing, so I won't object as long as you do consider the implications of calling using namespace in a .h file.
It will be included in Arduino.h so it is somewhat less likely it will cause build issues, but it might.

Still there may be name conflicts if some function has the same name (or function signature) in existing projects which also occur in this name space.

[d-a-v]

Yes, there is a risk of a name collision with {periodic,oneShot}{Ms,Fast{Ms,Us,Ns}}, but I'm confident.

If the using this-quite-unique-namespace is too much bothering, the PR can be changed to bring the above class names to global, and to not break backward compatibility the namespace can be recreated when #including PolledTImeout.h.

[d-a-v]

@devyte I'll let you verify changes following your review.
I didn't split core_esp8266_features.h into another core_esp8266_timing.h, I let that for another distinct commit.