micropython / micropython-esp32

Old port of MicroPython to the ESP32 -- new port is at https://github.com/micropython/micropython
MIT License
678 stars 218 forks source link

esp32/modsocket.c: read()/write() now return None for nonblocking #105

Closed MrSurly closed 7 years ago

MrSurly commented 7 years ago

Per #103

Tested with actual device, connecting to a simple socketserver server running on a PC.

dpgeorge commented 7 years ago

These functions need to return ETIMEDOUT if the timeout>0 and there was nothing. That is:

Does this PR give such behaviour?

MrSurly commented 7 years ago

Fixed, but:

Should it be returning ETIMEDOUT or MP_ETIMEDOUT? I used the former, per your question, but on ESP32 ETIMEDOUT has a value of 116 (normally 110) for newlib, though lwip defines it as 110. Using MP_ETIMEDOUT results in this error message, rather than the more generic error seen in the example below.

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 110] ETIMEDOUT
>>> import socket
I (122327) modsocket: Initializing
>>> s = socket.socket()
>>> addr = socket.getaddrinfo("172.16.23.157", 9999)[0][-1]
>>> s.connect(addr)
>>> I (129217) wifi: pm start, type:0

s.setblocking(False)
>>> print(s.read(5))
None
>>> s.settimeout(0)
>>> print(s.read(5))
None
>>> s.settimeout(1)
>>> print(s.read(5))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: 116
>>> s.write('hello')
5
>>> print(s.read(5))
b'HELLO'
>>> s.setblocking(False)
>>> s.write('there')
5
>>> print(s.read(5))
b'THERE'
>>> s.write('mango')
5
>>> s.setblocking(True)
>>> print(s.read(5))
b'MANGO'
>>> print(s.read(5))
dpgeorge commented 7 years ago

Should it be returning ETIMEDOUT or MP_ETIMEDOUT?

The code should use the MP_Exxx constants everywhere so the values are the same across all ports. Can you please update the PR to use them?

MrSurly commented 7 years ago

The code should use the MP_Exxx constants everywhere so the values are the same across all ports. Can you please update the PR to use them?

Done.

dpgeorge commented 7 years ago

Thanks, squashed and merged in 62d40e8b9e5b62ed27445a8f50688537575d97d1