Skip to content

Match core socket behaviour #178

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fasteddy516
Copy link
Contributor

While porting code from a WiFi-based board (Pico 2 W) to a wired ethernet board (WIZnet W5500-EVB-Pico2) I ran into problems caused by differences in behaviour between CircuitPython core sockets and those provided by this driver. This pull request aims to correct three discrepancies with modifications to adafruit_wiznet5k_socketpool.py:

  1. recv_into() now raises OSError(errno.EAGAIN) when the socket is in non-blocking mode and nothing is received.

  2. Instead of always returning instantly with addr = ("0.0.0.0", 0), accept() now respects the specified socket mode, i.e.:

    • In blocking mode, it blocks until a connection is accepted.
    • If a timeout has been specified, OSError(errno.ETIMEDOUT) is raised when the timeout expires.
    • In non-blocking mode OSError(errno.EAGAIN) is raised if there is no connection to accept.

    This has the added benefit of avoiding the overhead of the WIZnet socket swap dance until an actual connection is established.

  3. close() now calls _disconnect on SOCK_STREAM sockets. I'm not 100% sure on this one, but I noticed while comparing functionally equivalent code between Pico WiFi and WIZnet wired using PuTTy, calling close() on the Pico correctly closed my PuTTy session, but calling close() on the WIZnet just left my PuTTy session hanging. Adding the _disconnect() call in close() made the behaviour between the two match.

For testing 1 and 2 I used the script available here. While testing accept() I used a modified version of @niclas-gr's script from #170 available here.

This pull request resolves the issues (that I am aware of) that I reported in #177, and I believe it resolves #170 as well. The wiznet5k_simpletest.py, wiznet5k_httpserver.py and wiznet5k_aio_post.py examples all ran correctly with these changes in place. (Side note: the api.coindesk.com URL in simpletest appears to no longer be valid.)

@fasteddy516
Copy link
Contributor Author

I had to clean up some style/formatting items flagged by Black and Pylint - next time I'll try to remember to use pre-commit.

One of the flagged items was "too many branches" in recv_into(). My changes to this method were minimal, so I did the quick fix and added too-many-branches to the # pylint: disable= statement for the method. If this isn't acceptable, the method will have to be refactored and some logic extracted into helper methods.

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.

Unexpected connections from 0.0.0.0:0 on ESP32 Ethernet Server
1 participant