ev3dev / ev3dev-lang-python

Pure python bindings for ev3dev
MIT License
425 stars 144 forks source link

OSError exception [Errno 4] EINTR when running motors in EV3 #727

Open orioldelos opened 4 years ago

orioldelos commented 4 years ago
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                  Version         Architecture    Description
+++-=====================-===============-===============-================================================
ii  micropython-ev3dev2   2.0.0           all             Python language bindings for ev3dev for MicroPyt
ii  python3-ev3dev        1.2.0           all             Python language bindings for ev3dev
ii  python3-ev3dev2       2.0.0           all             Python language bindings for ev3dev

I have the following code runing on a thread in micropython with the only intention of moving the motor back and forth:

 while not self.timeToStop:
            try:
                self.sonarmotor.on_to_position(motorspeed,sweepangle/2)
                self.sonarmotor.on_to_position(motorspeed,-sweepangle/2)
            except OSError as e:
                type, value, traceback = sys.exc_info()
                logging.info("------------------------------------------Motor Exception: ")
                logging.info("Type:" + str(type))
                logging.info("Value:" + str(value))
                logging.info(str(traceback))

I was forced to handle the exception to avoid the aplication from aborting each time an OSError exception [Errno 4] EINTR was thrown.

I believe this might be a bug. Am I running the latest versions of Ev3dev2?

dlech commented 4 years ago

If you are using MicroPython instead of regular Python, it does not implement PEP 475

orioldelos commented 4 years ago

If I understand correctly, with micropython, if a system call fails, the underlying code will not retry to make the call and instead it will throw an OS exception. Therefore the only work around is to handle the exception the best you can?

WasabiFan commented 4 years ago

Yeah, as it is now, that is correct: you have to handle it manually. I'm somewhat surprised it happens that often for you.

orioldelos commented 4 years ago

It does happen to me very often, every 6 to 7 cicles I get an exception. For now I am hyandling it by reissuing the exact same command when the exception is handled but I am very supprised that with somethingas simple I need to be worring about exceptions.

Just to give you an idea, here is my code that runs on a thread:

   def run(self):
        self.sonarmotor.position=0
        motorspeed = 10
        sweepangle = 360
        while not self.timeToStop:
            try:
                self.sonarmotor.on_to_position(motorspeed,sweepangle/2)
            except OSError as e:
                self.sonarmotor.on_to_position(motorspeed,sweepangle/2)
            self.sweependCallback()
            sweepangle = -sweepangle

With this code I get the exception but the motor still does what is supposed to.

WasabiFan commented 4 years ago

Sorry for the delay on this!

I don't have a good immediate answer here -- this definitely isn't a great experience, but I think the only real solution would be to add our own wrapper here and I'm somewhat concerned about perf. Is it possible this is specifically due to the threading? If you only use one thread does this still happen?

orioldelos commented 4 years ago

Hi. Well... I have made a fast test by taking the code that ran the motor back and forth with minor modifications to make it run stand-alone without threading and now I am not getting any exceptions. Therefore the problem seems to be related to the motor methods being called from a thread.

dlech commented 4 years ago

In MicroPython, threads use a signal to trigger the garbage collector, so this makes sense. Without threads, there are no signals to cause the EINTR.

orioldelos commented 4 years ago

So, do you see a solution for this. It is anoying not being able to use the motors from within threads without continuosly getting exceptions.

dwalton76 commented 4 years ago

@orioldelos see if the patch in https://github.com/ev3dev/ev3dev-lang-python/issues/704 helps?

WasabiFan commented 4 years ago

I wouldn't expect it to (?). I think David is exactly right — the reason there are so many EINTR errors is because Micropython multithreading relies on signals, and signals abort read/write syscalls (and probably others) and cause them to return early in an effort to let the signal be handled efficiently without deadlock. The "right" solution is probably to wrap I/O operations in a retry loop. We would need to profile this and see whether there is an impact on performance. I might be able to do this today/tomorrow. In general, it would be good to develop a set of reproducible timing benchmarks we can use as a reference.

orioldelos commented 4 years ago

@orioldelos see if the patch in #704 helps?

I am willing to try it but I have never made any modifications to micropython libraries. As I can see what I have installed are .mpy files wich i can't edit with a text editor.

dlech commented 4 years ago

FYI, I have been working on fixing this in upstream MicroPython. https://github.com/micropython/micropython/pull/5723. However, I don't expect this fix to be backported to ev3dev-stretch.

orioldelos commented 4 years ago

So the way it is looking, if I wish to use threads with ev3dev2 I better switch to Python instead of working with micropython.

WasabiFan commented 4 years ago

The simple answer here is "yes". The more nuanced answer:

I opened #732 last night, which will let us compare timing benchmarks before/after code changes. Pending some discussion over there, my next step will be to add a try/catch around the access code and see if it degrades performance. If not, I'll check that in, and your problem will be solved for micropython too in the next release. If it does affect performance, we'll have to discuss what might make the most sense. Below I included a way you can make this change specifically for your program and test it out.

For your case: if you switch to plain Python, this issue will indeed be avoided. Normal CPython will be slower than Micropython when starting up and somewhat slower at executing code, depending on what exactly you're doing.

Another option for you is to manually monkey-patch in the retry code from your own program; cursory testing shows this at least functions for reading properties (not including writing properties or commands):

import errno
from ev3dev2 import Device
def _get_attribute(self, attribute, name):
    """Device attribute getter"""
    for _ in range(5): #added
        try:
            if attribute is None:
                attribute = self._attribute_file_open(name)
            else:
                attribute.seek(0)
            return attribute, attribute.read().strip().decode()
        except OSError as ex: #added
            if ex.errno == errno.EINTR:
                continue
            else:
                self._raise_friendly_access_error(ex, name, None)
        except Exception as ex:
            self._raise_friendly_access_error(ex, name, None)
Device._get_attribute = _get_attribute

from ev3dev2.motor import Motor
m = Motor()
print(m.speed)

You would put this code (except for the last three lines) at the top of your own program. This is a modified version of this:

https://github.com/ev3dev/ev3dev-lang-python/blob/af12545d1622067ddf55f4bd70a4f24e58d0eabb/ev3dev2/__init__.py#L253-L262

You could do a similar thing with _set_attribute, which is right below it.