Can't get library really working

[emelianov]

Probably i missing something but was able to get code below running only by patching esp8266\libraries\ESP8266WiFi\src\include\ClientContext.h (i know, delay doesn't implement early exit but it's just test)

diff --git a/libraries/ESP8266WiFi/src/include/ClientContext.h b/libraries/ESP8266WiFi/src/include/ClientContext.h
index 2f28915..55c4651 100644
--- a/libraries/ESP8266WiFi/src/include/ClientContext.h
+++ b/libraries/ESP8266WiFi/src/include/ClientContext.h
@@ -20,6 +20,7 @@
  */
 #ifndef CLIENTCONTEXT_H
 #define CLIENTCONTEXT_H
+#include <Schedule.h>

 class ClientContext;
 class WiFiClient;
@@ -131,7 +132,11 @@ public:
         _connect_pending = 1;
         _op_start_time = millis();
         // This delay will be interrupted by esp_schedule in the connect callback
-        delay(_timeout_ms);
+       //delay(_timeout_ms);
+       for (uint16_t i = 0; i < _timeout_ms; i++) {
+               delay(1);
+               run_scheduled_functions();
+       }
         _connect_pending = 0;
         if (!_pcb) {
             DEBUGV(":cabrt\r\n");

That case connect() is success. Don't perform more detailed checks.

#include <SPI.h>
#include <ESP8266WiFi.h>

#include <W5500lwIP.h>
#include <Schedule.h>

Wiznet5500lwIP eth(SPI, D4);
byte mac[] = {0x00, 0xAA, 0xBB, 0xCC, 0xDE, 0x02};

void setup ()
{
  Serial.begin(115200);

  SPI.begin();
  SPI.setBitOrder(MSBFIRST);
  SPI.setDataMode(SPI_MODE0);
  SPI.setFrequency(40000000);
  int present = eth.begin(mac);
  Serial.println("present= " + String(present, HEX));

  WiFi.mode(WIFI_OFF);

  while (!eth.connected()) {
    Serial.print(".");
    delay(1000);
    run_scheduled_functions();
  }
  Serial.println(eth.localIP());
  eth.setDefault(); // use ethernet for default route
  
  WiFiClient* client;
  client = new WiFiClient();
  Serial.println(client->connect(IPAddress(192, 168, 30, 133), 80));
}

void loop ()
{
    yield();
}

So it looks like that's enough to make run_scheduled_functions() called from yield()/delay() is that possible?

[d-a-v]

So it looks like that's enough to make run_scheduled_functions() called from yield()/delay() is that possible?

This is the purpose of the 1-2 months old patches in the core.
delay() and yield() both call esp_yield() which calls run_scheduled_functions()

With your patch you call it more often (2*timeout_ms) times instead of once.
This particular line you changed has changed on May 19 and I haven't been working on ethernet since then.
esp8266/Arduino@912c0db#diff-3f3ed90c696a8411853b28574edb12d8
So it appears this change has broken ethernet.

Can you check the old fashion, with esp_yield() instead of delay() without your loop ?

Thanks for debugging this !

[d-a-v]

Sorry I was not talking about the same delay() but they are very similar (in the same file, the other delay() recently updated).
You don't have to try anything (except trying to not call run_scheduled_functions()) and thanks again for debugging this issue.

Your fix is good and I think also is worth being applied to the other function.

To make it not a test, just add a test on _connect_pending:

       for (decltype(_timeout_ms) i = 0; _connect_pending && i < _timeout_ms; i++) {
               // let a chance to recurrent scheduled functions (ethernet)
               delay(1);
       }

Similarly on the other delay() with the same comment (the one I'm talking about above) should become:

       for (decltype(_timeout_ms) i = 0; _send_waiting && i < _timeout_ms; i++) {
               // let a chance to recurrent scheduled functions (ethernet)
               delay(1);
       }

It those changes work for you, would you propose a PR on the arduino core with them and a link to this issue ?

[emelianov]

I've looked dipper to Arduino core code and it looks like it's better to refactor delay(). Main idea is to call esp_yield() periodically during delay not only once. I'll try to create PR after comprehensive testing.
In combination with #7 it should make your library working with no WiFi* classes modification.

Nevertheless above mentioned

for (decltype(_timeout_ms) i = 0; _connect_pending && i < _timeout_ms; i++) {
               // let a chance to recurrent scheduled functions (ethernet)
               delay(1);
       }

modifications are also required. I'll create separate PR for them too.

[emelianov]

Just built rather complex project (client/server both) with ethernet (WiFi off) -- it's working. No issue except connect delay and wdt reset on first start (coldstart?).
Thanks for your great work on ethernet implementation!

[d-a-v]

except connect delay and wdt reset on first start (coldstart?).

This wdreset should be identified.

Thanks for your great work on ethernet implementation!

As you probably saw it, it's rather simple and only connecting lwIP with a new link layer.
The main issue is the need for proper transparent polling, which you are also currently addressing.

What's left is

• delay() refactoring esp8266/Arduino#6212
• w5100 driver
• simplify recurrent scheduled function in arduino core (some overhead can be removed)
• move this code into esp8266/arduino

And afterward

• PPP
• NAT in lwIP

[emelianov]

except connect delay and wdt reset on first start (coldstart?).

This wdreset should be identified.

Steps to reproduce WDT reset:

1. Connect to Wi-Fi
2. Disconnect from Wi-Fi (WiFi.mode(WIFI_OFF) )
3. Connect to Ehernet
4. Initialize mDNS.
   At step 4 got wdtreset.
   As firmware restores previousWi-Fi connection state on boot getting the same error on first boot with WiFi.mode(WIFI_OFF) (without explicit Wi-Fi connection from code).
   It's not looks like common situation, so can be left as is.

Hope to have some spare time to play with W5500 interrupt usage next days. But i'm not sure it can be really helpful.

[emelianov]

David,
By the way is the any particular reason to not implement handlePacket() using os_timer API (os_timer_arm) ?

[d-a-v]

I will try to reproduce ASAP (currently finishing to update recurrent scheduled functions in the arduno core).

By the way is the any particular reason to not implement handlePacket() using os_timer API (os_timer_arm) ?

Yes because minimum timing resolution is 1ms and I don't know whether performances need lower value yet.
Yes because I try to stay out from SYS stack (Ticker can use os_timer_arm and use scheduled functions. But in that case every timer create a new scheduled function, and they re executed only on loop()).
Yes because I know that current lwIP configuration is not protected against concurrency.

#define SYS_ARCH_DECL_PROTECT(x)
#define SYS_ARCH_PROTECT(x)
#define SYS_ARCH_UNPROTECT(x)

Once it works reliably in CONT stack (= not in ISR) we can try to make it work from interrupts (timer or external int) without recurrent/scheduled functions (= in ISR).