Skip to content

Conversation

@kevinkk525
Copy link
Contributor

@kevinkk525 kevinkk525 commented Apr 16, 2020

This PR fixes all known bugs with wait_for by using the shield() implementation added by #5928

However, a small change in core.py was needed to suppress the exception_handler from being called when an exception in the awaited task is being thrown. This happens because shield() detaches wait_for and the awaited task from each other and reverses the cancellation order to match CPython's mechanism.
This change will have to be implemented in _uasyncio Task too but I have not checked that.

This also fixes the issue that the awaited task could just ignore the cancellation.

This PR deprecates #5918 which only partially fixes wait_for.

Fully fix wait_for by using uasyncio.shield() to correct the cancellation order.
for catching exceptions and cancellation order
@dpgeorge
Copy link
Member

dpgeorge commented Dec 1, 2020

Sorry for neglecting this. The approach here uses shield() which needs to create another task (so 2 extra tasks are needed to implement wait-for) and also shield needs additional storage in a task to store the shielded variable.

See #6667 for an alternative approach which does not have these overheads.

@dpgeorge
Copy link
Member

dpgeorge commented Dec 2, 2020

Thanks again for contributing this. It has been fixed differently in b505971

@dpgeorge dpgeorge closed this Dec 2, 2020
@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Dec 2, 2020
@kevinkk525 kevinkk525 deleted the patch-3 branch December 2, 2020 10:36
tannewt added a commit to tannewt/circuitpython that referenced this pull request Feb 18, 2022
This allows board code to override the default pull up reset state.

It is useful for pins that are already externally connected, pulled
or otherwise used by the board.

Fixes micropython#5931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants