Skip to content

Shorter socket exit timeout #97

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Mar 20, 2023
Merged

Shorter socket exit timeout #97

merged 8 commits into from
Mar 20, 2023

Conversation

BiffoBear
Copy link
Contributor

@BiffoBear BiffoBear commented Feb 15, 2023 •
edited
Loading

#closes 89, 90 Decreased socket.socket.__exit__ timeout from 1000 seconds to 19 seconds to match the Wiznet5k hardware defaults.

This was linked to issues Feb 17, 2023
Socket gets stuck in __exit__ #89
Closed
adafruit_wiznet5k_socket.py has hardcoded 1000 second timeouts #90
Closed
@tekktrik tekktrik requested a review from a team February 17, 2023 14:56
@e28eta
Copy link
Contributor

e28eta commented Mar 6, 2023

I don't think this'll close #89, but it does look good to me for #90. I filed them as separate issues because I think they're different problems.

@dhalbert
Copy link
Contributor

dhalbert commented Mar 7, 2023

I don't think this'll close #89, but it does look good to me for #90. I filed them as separate issues because I think they're different problems.

@BiffoBear Do you have a comment on this?

@BiffoBear
Copy link
Contributor Author

I don't think this'll close #89, but it does look good to me for #90. I filed them as separate issues because I think they're different problems.

@BiffoBear Do you have a comment on this?

Good point well made. The documents are a little ambiguous, so I've gone through the C source code from the Wiznet drivers.

socket.__exit__() now follows the logic of the Wiznet driver for disconnecting and closing. I've taken the opportunity to remove calls to deprecated socket.socket() methods, as they just call the WIZNET5K methods anyway.

Cheers,
Martin

@BiffoBear
Copy link
Contributor Author

But please note that the default hardware timeout for a TCP socket is about 19 seconds.

FoamyGuy
FoamyGuy approved these changes Mar 20, 2023
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested this version successfully with the simpleserver / client and simpletest requests examples on Feather ESP232-S2 TFT.

Thank you @BiffoBear

@FoamyGuy FoamyGuy merged commit d2637d7 into adafruit:main Mar 20, 2023
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 21, 2023
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@2415fa6)
26f0a86 
Updating https://github.com/adafruit/Adafruit_CircuitPython_VL53L1X to 1.1.10 from 1.1.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL53L1X#15 from jposada202020/adding_version_metadata
  > Add upload url to release action

Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 2.4.0 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#97 from BiffoBear/shorter_socket_exit_timeout
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#99 from BiffoBear/fix_out_of_sockets
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#101 from BiffoBear/fix_socket.close()_behaviour

Updating https://github.com/adafruit/Adafruit_CircuitPython_HID to 5.3.4 from 5.3.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_HID#113 from dhalbert/fix-led-status
  > Add upload url to release action
  > Add .venv to .gitignore
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
@BiffoBear BiffoBear deleted the shorter_socket_exit_timeout branch March 22, 2023 15:21
@FoamyGuy FoamyGuy mentioned this pull request Mar 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoamyGuy FoamyGuy FoamyGuy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

adafruit_wiznet5k_socket.py has hardcoded 1000 second timeouts Socket gets stuck in __exit__
4 participants
@BiffoBear @e28eta @dhalbert @FoamyGuy