Skip to content

Commit

Permalink
Fix: WiFiClient::flush() yields but can be called from events (#5254)
Browse files Browse the repository at this point in the history
Fix bug introduced by #5167 which replaced delay() by yield().
That should have been esp_yield() which is the one delay()
calls and is safe from either SYS or CONT contexts.

Fixes #5237.
  • Loading branch information
d-a-v authored and earlephilhower committed Oct 17, 2018
1 parent ad7cb63 commit e549355
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion libraries/ESP8266WiFi/src/include/ClientContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ class ClientContext
last_sent = millis();
}

yield();
esp_yield(); // from sys or os context

if ((state() != ESTABLISHED) || (sndbuf == TCP_SND_BUF)) {
break;
Expand Down

6 comments on commit e549355

@ascillato
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi

@d-a-v, @earlephilhower

We have done several tests and we found that this last commit make the device unresponsive to webserver.

Reverting to ad7cb63 all works again.

Please, could you revert this last commit?

@earlephilhower
Copy link
Collaborator

@earlephilhower earlephilhower commented on e549355 Oct 18, 2018

Choose a reason for hiding this comment

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

@ascillato sorry to hear that. Can you try using

esp_schedule();
esp_yield();

instead of going back to delay() and see if that clears your issue up?

yield() is not allowed in SYS context (which can also use this routine). -edit- typo here <---

@ascillato
Copy link
Contributor

Choose a reason for hiding this comment

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

will test

@earlephilhower
Copy link
Collaborator

Choose a reason for hiding this comment

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

D'oh, my suggested code is actually the exact same as delay(0); So ignore that, try delay(0), please.

@ascillato
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, will try

@ascillato
Copy link
Contributor

@ascillato ascillato commented on e549355 Oct 18, 2018

Choose a reason for hiding this comment

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

Hi,

delay(0) works fine !!!!

Tested webserver and OTA URL. All fine.

Please sign in to comment.