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

unix port socket.settimeout not implemented #2379

Closed
danni opened this Issue Sep 2, 2016 · 14 comments

Comments

Projects
None yet
5 participants
@danni
Copy link

danni commented Sep 2, 2016

socket.settimeout() isn't implemented in the UNIX port, which makes it extremely difficult to test code using socket timeouts on UNIX.

(This is especially fun because the ESP8266 doesn't have uselect, I have no platform independent way to regain control.)

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Sep 2, 2016

This situation is known to maintainers, you're the first one to bring it from community (to help assess its priority). We discussed this situation with @dpgeorge , and I believe he even had timeout code for unix port. My reply at that time was however that providing different ways to achieve the functionality isn't too good an idea, and adding select() (well, poll()) support to modlwip is the right way.

Feel free to explain your usecase, why you need timeouts in unix port, then impact on code size of their implementation, and this feature should be configurable to be able to maintain small code size.

@danni

This comment has been minimized.

Copy link

danni commented Sep 2, 2016

Reasoning came from wanting a platform independent way to sleep waiting for IO and uselect not being available on all platforms (preferred choice... well after asyncio). Adding settimeout to the UNIX port seemed easier than adding select to the ESP port as a first cut, to see if my code works.

Also I was surprised by not having it in the UNIX port when it's on the chips (the docs could be clearer in what functions are in what ports). Is code size such a big deal on the UNIX port?

@danni

This comment has been minimized.

Copy link

danni commented Sep 2, 2016

More specifically socket.io needs to ping the server occasionally https://github.com/danni/uwebsockets

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Sep 3, 2016

Also I was surprised by not having it in the UNIX port when it's on the chips (the docs could be clearer in what functions are in what ports).

"chips" are standalone, thing-in-itself systems, so if something is not available for them, it may be complicated to add it. Unix port OTOH is an open-end system, similar to CPython. Large part of its functionality is implemented in Python in standard library: https://github.com/micropython/micropython-lib . Nobody implemented timeout support in micropython-lib (because nobody needed it), that's it.

Is code size such a big deal on the UNIX port?

These and related questions are covered in https://github.com/micropython/micropython/wiki/ContributorGuidelines .

@danni

This comment has been minimized.

Copy link

danni commented Sep 3, 2016

I did look at trying to extend the socket object with timeout support in pure Python, but it needed a lot of knowledge from socket.h to run on both Linux and OSX. Also I had assumed that there would be some intention of having the ports be reasonably equivalent in terms of methods available.

Somewhat related, but is there a push to unify the docs between the ports, and have an "Availability: Pyboard, ESP8266, ...". The multiple sets of docs for the ports caused me no end of confusion starting out. Especially writing code on the train without the chip handy, and then finding out it wasn't going to run.

@danni

This comment has been minimized.

Copy link

danni commented Sep 3, 2016

I'm happy for you to reject this request/PR if you definitely don't want to grow the size of the UNIX port, because I agree select is better. I think I can get a kludey select going on the ESP pretty easily to retain some platform independence. However in this case, maybe settimeout should just be deprecated on all platforms with "Use select"?

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Sep 3, 2016

I did look at trying to extend the socket object with timeout support in pure Python, but it needed a lot of knowledge from socket.h to run on both Linux and OSX.

It can be implemented using uselect.poll().

Also I had assumed that there would be some intention of having the ports be reasonably equivalent in terms of methods available.

Yes, using the more powerful apparatus, not less powerful. uselect.poll can timeout on multiple sockets in parallel, settimeout() only on one.

Somewhat related, but is there a push to unify the docs between the ports, and have an "Availability: Pyboard, ESP8266, ...".

Yes, even more, not to have "availability". That means actually make everything work across ports. It's very easy to hack some adhoc code, it's hard to design consistent full-featured interface for different hardware, and then actually implement it across various hardware. So, it's ongoing work.

@dpgeorge

This comment has been minimized.

Copy link
Contributor

dpgeorge commented Sep 3, 2016

The multiple sets of docs for the ports caused me no end of confusion starting out.

This is a recurring problem that users are having. Maybe we need to have just one set of docs (ie no separate builds for separate ports), and stuff that's completely port specific (quickref and tutorials, but even those can and should have generic versions) goes in individual sections that are accessible from the main index page. I think this would be easier to maintain, and easier for users to use.

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Sep 3, 2016

I'm happy for you to reject this request/PR if you definitely don't want to grow the size of the UNIX port, because I agree select is better. I think I can get a kludey select going on the ESP pretty easily to retain some platform independence. However in this case, maybe settimeout should just be deprecated on all platforms with "Use select"?

uselect.poll should be standard interface, implemented on all ports. Beyond that, it's up to maintainers of particular ports what to have more.

I think I can get a kludey

While people will go this way - doing kludgy things on individual ports, there will be the situation like you describe. It take a bit more effort and forward-looking thinking to get ahead of that. I'm saying that as someone who contributes to MicroPython almost daily last 2.5 years.

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Sep 3, 2016

Maybe we need to have

What we need to do about the docs as a next step is to have proper section ordering in ToC. Look at ToC-based documentation format, like PDF, to see how funny sections are ordered.

@deshipu

This comment has been minimized.

Copy link
Contributor

deshipu commented Sep 3, 2016

Maybe we need to have just one set of docs (ie no separate builds for separate ports), and stuff that's completely port specific (quickref and tutorials, but even those can and should have generic versions) goes in individual sections that are accessible from the main index page.

This would also encourage more compatibility between ports, by making all inconsistencies easy to see, awkward and inconvenient.

If only there was a requirement to write the docs while making decisions about the API...

@pfalcon pfalcon added the notimpl label May 14, 2017

@g-sam

This comment has been minimized.

Copy link

g-sam commented Aug 8, 2017

Plus one for this. Ports should be consistent unless there is a good reason for them not be.

I ran into this because I wanted to have my tests for esp8266 code run on unix.

@pfalcon

This comment has been minimized.

Copy link
Member

pfalcon commented Nov 3, 2017

The issue in the title is now documented, including the suggested alternative at http://docs.micropython.org/en/latest/unix/library/usocket.html?#usocket.socket.settimeout . Given the code size constraints we'd like to maintain, presence of an alternative (more verbose, but also more powerful), and a queue of features we'd like to add, which don't have viable alternatives, I think it's fair to say that timeout support in Unix port won't be implemented on C level. Interested parties are welcome to implement it in https://github.com/micropython/micropython-lib/blob/master/socket/socket.py .

I'm thus closing this.

@pfalcon pfalcon closed this Nov 3, 2017

@pfalcon pfalcon added the docs label Nov 3, 2017

klarose added a commit to klarose/schc-hackathon that referenced this issue Jul 14, 2018

Use uselect.poll rather than settimeout
socket.settimeout is not implemented for every port. The docs recommend
using uselect.poll as the proper cross-platform method:
http://docs.micropython.org/en/latest/unix/library/usocket.html?#usocket.socket.settimeout

Implementing it for unix was suggested here.

micropython/micropython#2379

A version of the unix port with settimeout can be found here, but I
decided not to use it.

klarose/micropython@2349522

klarose added a commit to klarose/schc-hackathon that referenced this issue Jul 14, 2018

Use uselect.poll rather than settimeout
socket.settimeout is not implemented for every port. The docs recommend
using uselect.poll as the proper cross-platform method:
http://docs.micropython.org/en/latest/unix/library/usocket.html?#usocket.socket.settimeout

Implementing it for unix was suggested here.

micropython/micropython#2379

A version of the unix port with settimeout can be found here, but I
decided not to use it.

klarose/micropython@2349522
@dpgeorge

This comment has been minimized.

Copy link
Contributor

dpgeorge commented Oct 17, 2018

socket.settimeout() on the unix port was implemented in 80a2581 and 0c18633

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