added clock stretch yield, [issue 2162] fixed twi::status by Tech-TX · Pull Request #6860 · esp8266/Arduino · GitHub
Skip to content

added clock stretch yield, [issue 2162] fixed twi::status#6860

Merged
devyte merged 1 commit into
esp8266:masterfrom
Tech-TX:bugfix/issue_2162
Dec 1, 2019
Merged

added clock stretch yield, [issue 2162] fixed twi::status#6860
devyte merged 1 commit into
esp8266:masterfrom
Tech-TX:bugfix/issue_2162

Conversation

@Tech-TX

@Tech-TX Tech-TX commented Nov 30, 2019

Copy link
Copy Markdown
Contributor

Finally figured out what I'd done wrong with the earlier commit. No -a in a local commit!

issue #2162

Added devyte's PolledTimeout in a nested loop replacing the original WAIT_CLOCK_STRETCH(). The outer loop exits if SCL goes high or the loop times out. The inner loop does a yield() every 5ms while we're waiting.
Additionally, two small changes to twi::status that add a clock stretch test before we exit if we find SCL stuck low, and the trailing if (!write_start()) at the end is removed as it leaves SDA low, killing the bus.

@devyte devyte added this to the 2.7.0 milestone Dec 1, 2019
@earlephilhower

Copy link
Copy Markdown
Collaborator

@devyte devyte merged commit cc6d346 into esp8266:master Dec 1, 2019
@dok-net

dok-net commented Dec 1, 2019

Copy link
Copy Markdown
Contributor

I propose amending this to, pending #6804 (for corrected timing):

        esp8266::polledTimeout::oneShotFastUs timeout(twi_clockStretchLimit);
        while(!timeout && !SCL_READ())  // loop is stretch duration up to stretch limit
        { 
            optimistic_yield(5000);
        }

Like this, it may fire yield() for the first time earlier, but since yield() runs recurrent scheduled functions (internally resetting the optimistic yield timeout in turn) it's still better and the right way, IMHO.

@dok-net

dok-net commented Dec 1, 2019

Copy link
Copy Markdown
Contributor

@Tech-TX Tech-TX deleted the bugfix/issue_2162 branch December 1, 2019 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants