Skip to content

Socket improvements #31

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

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Socket improvements #31

merged 2 commits into from
Jan 29, 2021

Conversation

xorbit
Copy link
Contributor

@xorbit xorbit commented Jan 27, 2021

  • Fixed socket_available to just report and not flush available UDP
    data when calling multiple times.
  • Fixed socket __exit__ to only disconnect if a TCP (stream) socket.
  • Connect can now accept domain names like CPython versus only IP
    addresses.
  • Added socket send_to, recvfrom, recv_into, recvfrom_into
    methods for better compatibility with CPython socket.
  • Simplified recv and readline socket methods due to
    available doing the right thing for both TCP and UDP.
  • Reformatting to make black happy.
  • Changes made and tested to make generic socket NTP implementation work:
    https://github.com/askpatrickw/Adafruit_CircuitPython_NTP/blob/raw_ntp/adafruit_ntp.py

- Fixed `socket_available` to just report and not flush available UDP
  data when calling multiple times.
- Fixed socket `__exit__` to only disconnect if a TCP (stream) socket.
- Connect can now accept domain names like CPython versus only IP
  addresses.
- Added socket `send_to`, `recvfrom`, `recv_into`, `recvfrom_into`
  methods for better compatibility with CPython socket.
- Simplified `recv` and `readline` socket methods due to
  `available` doing the right thing for both TCP and UDP.
- Reformatting to make black happy.
- Changes made and tested to make generic socket NTP implementation work:
  https://github.com/askpatrickw/Adafruit_CircuitPython_NTP/blob/raw_ntp/adafruit_ntp.py
@xorbit
Copy link
Contributor Author

xorbit commented Jan 27, 2021

You know, this CI is more trouble than it's worth. I check my code with black on my machine yet the CI back seems to have a different idea of what it should look like but doesn't tell me what.

@tannewt
Copy link
Member

tannewt commented Jan 27, 2021

@xorbit I recommend setting up pre-commit if you haven't already. It should manage black and the other checks the CI does. Instructions are here: https://pre-commit.com/#installation

@xorbit
Copy link
Contributor Author

xorbit commented Jan 27, 2021

Thanks for that tip @tannewt !
With help I may learn all these software development tricks yet. I try to keep myself on the hardware side of things mostly but sometimes it's unavoidable to deal with software too.

With pre-commit, black undid a change my system-wide black made yesterday. 😂

@tannewt
Copy link
Member

tannewt commented Jan 28, 2021

Yup! Sorry about that. We've been meaning to add the pre-commit recommendation to the guides but I'm not sure we have yet.

@tannewt
Copy link
Member

tannewt commented Jan 28, 2021

(Will let Brent review and test.) Thanks for the PR! Sorry for the frustrating CI.

@brentru
Copy link
Member

brentru commented Jan 28, 2021

@xorbit I tried wiznet5k_simpleserver.py on my board (with DHCP disabled) and was unable to see a response echo'd back after I ran:
echo -n "hello featherwing" | nc 192.168.10.1 50007

However, I used the wsgi server in the README and had no issues connecting. Same with the simpletest.

Any ideas?

@xorbit
Copy link
Contributor Author

xorbit commented Jan 29, 2021

@xorbit I tried wiznet5k_simpleserver.py on my board (with DHCP disabled) and was unable to see a response echo'd back after I ran:
echo -n "hello featherwing" | nc 192.168.10.1 50007

However, I used the wsgi server in the README and had no issues connecting. Same with the simpletest.

Any ideas?

Odd. I tried it and it seems to work for me. The only difference is that I had DHCP. See this REPL log with debug enabled:

>>> while True:
...     conn, addr = server.accept()
...     print("Connection from {} port {}".format(addr[0], addr[1]))
...     with conn:
...         data= conn.recv()
...         print (data)
...         conn.send(data)
... 
*** Get socket
Allocated socket #1
* Dest is (192.168.1.109, 32896), Next listen socknum is #1
*** Get socket
Allocated socket #1
* Listening on port=50007, ip=192.168.1.135
*** Opening socket 1
* Opening W5k Socket, protocol=33
Connection from 192.168.1.109 port 32896
* socket_available called with protocol 33
Bytes avail. on sock:  17
         * Processing 17 bytes of data
* socket_available called with protocol 33
b'hello featherwing'
*** Disconnecting socket #0
*** Closing socket #0
*** Get socket
Allocated socket #0
* Dest is (192.168.1.109, 32896), Next listen socknum is #0
*** Get socket
Allocated socket #0
* Listening on port=50007, ip=192.168.1.135
*** Opening socket 0
* Opening W5k Socket, protocol=33
Connection from 192.168.1.109 port 32896
* socket_available called with protocol 33
Bytes avail. on sock:  8
         * Processing 8 bytes of data
* socket_available called with protocol 33
b'hello m4'
*** Disconnecting socket #1
*** Closing socket #1

So I wondered if it could be a difference because I was using DHCP, so I redid the test without it:

>>> socket.set_interface(eth)
>>> server = socket.socket()
*** Get socket
Allocated socket #0
>>> server.bind(("192.168.1.136", 50007))
>>> server.listen()
* Listening on port=50007, ip=192.168.1.136
*** Opening socket 0
* Opening W5k Socket, protocol=33
>>> while True:
...     conn, addr = server.accept()
...     print ("Connection from {} port {}".format(addr[0], addr[1]))
...     with conn:
...         data = conn.recv()
...         print(data)
...         conn.send(data)
... 
*** Get socket
Allocated socket #1
* Dest is (192.168.1.109, 53456), Next listen socknum is #1
*** Get socket
Allocated socket #1
* Listening on port=50007, ip=192.168.1.136
*** Opening socket 1
* Opening W5k Socket, protocol=33
Connection from 192.168.1.109 port 53456
* socket_available called with protocol 33
Bytes avail. on sock:  13
         * Processing 13 bytes of data
* socket_available called with protocol 33
b'hello m4-shim'
*** Disconnecting socket #0
*** Closing socket #0

Not sure why it's different from your test.

@brentru
Copy link
Member

brentru commented Jan 29, 2021

@xorbit It might be an issue with my local network. Thanks for providing hardware debug logs.

@brentru brentru merged commit 735bb43 into adafruit:master Jan 29, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 30, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_CharLCD to 3.3.6 from 3.3.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_CharLCD#56 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 4.3.0 from 4.2.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#102 from adafruit/dherrada-patch-1
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#101 from kattni/cleanup-deprecated-code
  > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#99 from mw46d/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_IRRemote to 4.0.3 from 4.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_IRRemote#43 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_LSM303DLH_Mag to 1.1.5 from 1.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_LSM303DLH_Mag#11 from askpatrickw/fix-repo-url-typo

Updating https://github.com/adafruit/Adafruit_CircuitPython_MPR121 to 2.1.5 from 2.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_MPR121#30 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_Nunchuk to 1.0.0 from 0.3.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Nunchuk#24 from jfurcean/add-values-property

Updating https://github.com/adafruit/Adafruit_CircuitPython_RFM9x to 2.1.2 from 2.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_RFM9x#59 from AdamLeyshon/add_snr_property

Updating https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k to 1.9.0 from 1.8.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Wiznet5k#31 from xorbit/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_AdafruitIO to 5.1.0 from 5.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_AdafruitIO#62 from brentru/create-group-feed
  > Merge pull request adafruit/Adafruit_CircuitPython_AdafruitIO#61 from brentru/small-feed-names
  > Merge pull request adafruit/Adafruit_CircuitPython_AdafruitIO#60 from askpatrickw/fix-repo-url-typos

Updating https://github.com/adafruit/Adafruit_CircuitPython_AVRprog to 1.3.4 from 1.3.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_AVRprog#20 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_Motor to 3.2.5 from 3.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Motor#51 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_SimpleIO to 2.1.3 from 2.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_SimpleIO#58 from askpatrickw/fix-repo-typo
  > Added pre-commit-config file

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_datetime
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.

3 participants