Skip to content
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

Handle EAGAIN on write (BlockingIOError: [Errno 11] Resource temporar… #227

Closed
wants to merge 1 commit into from

Conversation

wolfmanjm
Copy link

@wolfmanjm wolfmanjm commented May 3, 2017

…ily unavailable)

As the pyserial-asyncio uses non blocking mode posix serial needs to handle the EAGAIN error which is a non fatal error that is often seen in non blocking mode.

Without it we get this error in asyncio mode when writing a lot of data..

protocol: <__main__.SerialConnection object at 0xb6d0bd8c>
transport: SerialTransport(<_UnixSelectorEventLoop running=True closed=False debug=False>, <__main__.SerialConnection object at 0xb6d0bd8c>, Serial<id=0xb6d0b9ac, open=True>(port='/dev/ttyACM0', baudrate=115200, bytesize=8, parity='N', stopbits=1, timeout=0, xonxoff=False, rtscts=False, dsrdtr=False))
Traceback (most recent call last):
  File "/usr/local/lib/python3.4/dist-packages/pyserial-3.3-py3.4.egg/serial/serialposix.py", line 534, in write
    n = os.write(self.fd, d)
BlockingIOError: [Errno 11] Resource temporarily unavailable

Fixes this issue pyserial/pyserial-asyncio#20

@zsquareplusc
Copy link
Member

OK but the fix only works for Python 3, that Exception name is not present in Python 2.7 (I need to review #225 too).

@wolfmanjm
Copy link
Author

oh I am sorry I forgot to test on 2.7 as asyncio only works on 3... I'll see what I can do for 2.7.
thank you

@zsquareplusc
Copy link
Member

I've ported the code from the socket handler here and saw that EINTR was not handled for Py 2.7 while it was for 3.x.

So you do not need to provide an updated patch, but thanks for finding the issue.

@wolfmanjm
Copy link
Author

@zsquareplusc Thank you, however that fix is for read. The error I get is with write not the read.
So this still needs to be added to the write method as well as my patch does.

zsquareplusc added a commit that referenced this pull request May 4, 2017
- catch all the errno's that BlockingIOError would cover
- catch EINTR

fixes #227
@zsquareplusc
Copy link
Member

OK, i think it makes sense to catch the same errors in read and write, pushed a change with that.

@wolfmanjm
Copy link
Author

wolfmanjm commented May 4, 2017

Hi @zsquareplusc I just tested your fix.. however I now get this error... Seems that it still is not handling non blocking mode correctly on writes.
I'll try to debug further. My original fix did not get this error.

protocol: <__main__.SerialConnection object at 0xb6c8c40c>
transport: SerialTransport(<_UnixSelectorEventLoop running=True closed=False debug=False>, <__main__.SerialConnection object at 0xb6c8c40c>, Serial<id=0xb718fb0c, open=True>(port='/dev/ttyACM0', baudrate=115200, bytesize=8, parity='N', stopbits=1, timeout=0, xonxoff=False, rtscts=False, dsrdtr=False))
Traceback (most recent call last):
  File "/home/morris/.local/lib/python3.4/site-packages/pyserial_asyncio-0.4-py3.4.egg/serial_asyncio/__init__.py", line 120, in write
    n = self._serial.write(data)
  File "/home/morris/.local/lib/python3.4/site-packages/pyserial-3.3-py3.4.egg/serial/serialposix.py", line 577, in write
    raise writeTimeoutError
serial.serialutil.SerialTimeoutException: Write timeout

@wolfmanjm
Copy link
Author

ok the issue appears to be here...

return self.target_time is not None and self.time_left() <= 0

even though the Timeout is set to 0 ie non blocking this

self.target_time = self.TIME() + duration
sets the target_time when it should be set to None. this causes a bogus timeout error for non blocking writes.

@wolfmanjm
Copy link
Author

Changing

self.target_time = self.TIME() + self.duration

to
if duration is not None and duration != 0:
seems to fix this issue, but I am not convinced that is correct, what do you think?

@wolfmanjm
Copy link
Author

wolfmanjm commented May 5, 2017

Possibly the correct fix is to do what was there originally and not call in

if timeout.expired():

           if timeout.expired():
                raise writeTimeoutError

unless there is an exception.

or possibly not, as checking timeout for a non blocking port seems inconsistent and possibly incorrect, and in this case would still generate a bogus timeout exception.
I think my original suggestion is more correct as setting timeout = 0 means non blocking and no timeouts.

zsquareplusc added a commit that referenced this pull request May 5, 2017
@zsquareplusc
Copy link
Member

It was probably broken before that change too... (your fix had a separate except clause that skipped the test). It seems that exceptions in non-blocking I/O would have triggered the faulty write error.

The timeout object returns True for expired() if the timeout is 0 (non-blocking I/O). This is fine for read() where it just breaks out of the read loop, but not good for write() where it would throw an exception (the return for non-blocking I/O is separate here). The fix is to skip the test for non-blocking I/O.

@wolfmanjm
Copy link
Author

SKip the test for non blocking write in posixserial or in Timeout? I am not clear where to exactly fix this :)

@wolfmanjm
Copy link
Author

Ahh ok never mind I see your fix, thank you I'll test it ASAP.

@wolfmanjm
Copy link
Author

Thank you...

I tested the latest and it works perfectly.

@wolfmanjm wolfmanjm deleted the fix/EAGAIN-on-write branch May 5, 2017 03:21
n-eq pushed a commit to n-eq/pyserial that referenced this pull request Aug 22, 2017
- catch all the errno's that BlockingIOError would cover
- and catch EINTR in for both blocks, for Py 2.7 and 3.x

fixes pyserial#227
n-eq pushed a commit to n-eq/pyserial that referenced this pull request Aug 22, 2017
- catch all the errno's that BlockingIOError would cover
- catch EINTR

fixes pyserial#227
n-eq pushed a commit to n-eq/pyserial that referenced this pull request Aug 22, 2017
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.

None yet

2 participants