Skip to content
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

Fix optimistic_yield to not yield on each call after x µs #6804

Merged
merged 9 commits into from
Feb 20, 2020

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Nov 18, 2019

Before, once an invocation of loop() runs for x µs, each call to optimistic_yield() does really yield().
Now, only yield every x µs between optimistic_yield() or yield() calls.

Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

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

I wonder if we should use instead the very light esp_get_cycle_count() here

@dok-net
Copy link
Contributor Author

dok-net commented Nov 19, 2019

@d-a-v I've used the ESP.get… wrappers instead... do you approve of the possible overflowing for large interval numbers? Adding corrective code probably ruins any advantage from not using micros()?
I have also changed Schedule.cpp from using polledTimeout, which introduced a completely separate yield() cadence, that I consider wasteful now, to using optimistic_yield(), which exists specificially for use in library's loops(). Works very well in my async test (scanning WiFi networks), same in multitasking with CoopTask. Also no regression with EspSoftwareSerial on an 9600bps external device, none in SW serial to internal HW loopback test at 153600bps 8E2, either.

@dok-net dok-net changed the title Fix optimistic_yield to not yield on every call after x ms Fix optimistic_yield to not yield on every call after x µs Nov 19, 2019
@dok-net dok-net force-pushed the optimistic_yield_recurrency branch 4 times, most recently from c0bc8d3 to fa55a4f Compare November 21, 2019 21:05
@dok-net dok-net force-pushed the optimistic_yield_recurrency branch 2 times, most recently from d8a07c1 to 6c50e74 Compare December 1, 2019 08:12
@dok-net dok-net force-pushed the optimistic_yield_recurrency branch 2 times, most recently from 2448189 to 2a15fbc Compare December 2, 2019 10:06
@dok-net dok-net changed the title Fix optimistic_yield to not yield on every call after x µs Fix optimistic_yield to not yield on each call after x µs Dec 4, 2019
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

Seems like a very good fix of a long-standing problem. My only concern is the change in Schedule.cpp which seems to be orthogonal to the optimistic_yield change and replaces an esp_schedule/cont_yield with a yield...

cores/esp8266/Schedule.cpp Outdated Show resolved Hide resolved
cores/esp8266/Arduino.h Outdated Show resolved Hide resolved
@devyte devyte added this to the 2.7.0 milestone Dec 11, 2019
@devyte devyte self-assigned this Dec 18, 2019
@dok-net dok-net force-pushed the optimistic_yield_recurrency branch 3 times, most recently from 4b6b570 to 7e5dd70 Compare February 5, 2020 11:25
cores/esp8266/core_esp8266_main.cpp Outdated Show resolved Hide resolved
cores/esp8266/core_esp8266_main.cpp Outdated Show resolved Hide resolved
cores/esp8266/core_esp8266_main.cpp Outdated Show resolved Hide resolved
@earlephilhower earlephilhower merged commit 0a58172 into esp8266:master Feb 20, 2020
@dok-net dok-net deleted the optimistic_yield_recurrency branch February 22, 2020 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants