diff --git a/can/interfaces/slcan.py b/can/interfaces/slcan.py index 212c4c85c..bcb3121ca 100644 --- a/can/interfaces/slcan.py +++ b/can/interfaces/slcan.py @@ -64,6 +64,7 @@ def __init__( btr: Optional[str] = None, sleep_after_open: float = _SLEEP_AFTER_SERIAL_OPEN, rtscts: bool = False, + timeout: float = 0.001, **kwargs: Any, ) -> None: """ @@ -82,7 +83,8 @@ def __init__( Time to wait in seconds after opening serial connection :param rtscts: turn hardware handshake (RTS/CTS) on and off - + :param timeout: + Timeout for the serial or usb device in seconds (default 0.001) :raise ValueError: if both ``bitrate`` and ``btr`` are set or the channel is invalid :raise CanInterfaceNotImplementedError: if the serial module is missing :raise CanInitializationError: if the underlying serial connection could not be established @@ -98,7 +100,10 @@ def __init__( with error_check(exception_type=CanInitializationError): self.serialPortOrig = serial.serial_for_url( - channel, baudrate=ttyBaudrate, rtscts=rtscts + channel, + baudrate=ttyBaudrate, + rtscts=rtscts, + timeout=timeout, ) self._buffer = bytearray() @@ -150,46 +155,34 @@ def _write(self, string: str) -> None: self.serialPortOrig.flush() def _read(self, timeout: Optional[float]) -> Optional[str]: + _timeout = serial.Timeout(timeout) with error_check("Could not read from serial device"): - # first read what is already in receive buffer - while self.serialPortOrig.in_waiting: - self._buffer += self.serialPortOrig.read() - # if we still don't have a complete message, do a blocking read - start = time.time() - time_left = timeout - while not ( - ord(self._OK) in self._buffer or ord(self._ERROR) in self._buffer - ): - self.serialPortOrig.timeout = time_left - byte = self.serialPortOrig.read() - if byte: - self._buffer += byte - # if timeout is None, try indefinitely - if timeout is None: - continue - # try next one only if there still is time, and with - # reduced timeout - else: - time_left = timeout - (time.time() - start) - if time_left > 0: - continue + while True: + # Due to accessing `serialPortOrig.in_waiting` too often will reduce the performance. + # We read the `serialPortOrig.in_waiting` only once here. + in_waiting = self.serialPortOrig.in_waiting + for _ in range(max(1, in_waiting)): + new_byte = self.serialPortOrig.read(size=1) + if new_byte: + self._buffer.extend(new_byte) else: - return None + break + + if new_byte in (self._ERROR, self._OK): + string = self._buffer.decode() + self._buffer.clear() + return string + + if _timeout.expired(): + break - # return first message - for i in range(len(self._buffer)): - if self._buffer[i] == ord(self._OK) or self._buffer[i] == ord(self._ERROR): - string = self._buffer[: i + 1].decode() - del self._buffer[: i + 1] - break - return string + return None def flush(self) -> None: - del self._buffer[:] + self._buffer.clear() with error_check("Could not flush"): - while self.serialPortOrig.in_waiting: - self.serialPortOrig.read() + self.serialPortOrig.reset_input_buffer() def open(self) -> None: self._write("O") @@ -204,7 +197,7 @@ def _recv_internal( canId = None remote = False extended = False - frame = [] + data = None string = self._read(timeout) @@ -215,14 +208,12 @@ def _recv_internal( canId = int(string[1:9], 16) dlc = int(string[9]) extended = True - for i in range(0, dlc): - frame.append(int(string[10 + i * 2 : 12 + i * 2], 16)) + data = bytearray.fromhex(string[10 : 10 + dlc * 2]) elif string[0] == "t": # normal frame canId = int(string[1:4], 16) dlc = int(string[4]) - for i in range(0, dlc): - frame.append(int(string[5 + i * 2 : 7 + i * 2], 16)) + data = bytearray.fromhex(string[5 : 5 + dlc * 2]) elif string[0] == "r": # remote frame canId = int(string[1:4], 16) @@ -242,7 +233,7 @@ def _recv_internal( timestamp=time.time(), # Better than nothing... is_remote_frame=remote, dlc=dlc, - data=frame, + data=data, ) return msg, False return None, False @@ -252,15 +243,15 @@ def send(self, msg: Message, timeout: Optional[float] = None) -> None: self.serialPortOrig.write_timeout = timeout if msg.is_remote_frame: if msg.is_extended_id: - sendStr = "R%08X%d" % (msg.arbitration_id, msg.dlc) + sendStr = f"R{msg.arbitration_id:08X}{msg.dlc:d}" else: - sendStr = "r%03X%d" % (msg.arbitration_id, msg.dlc) + sendStr = f"r{msg.arbitration_id:03X}{msg.dlc:d}" else: if msg.is_extended_id: - sendStr = "T%08X%d" % (msg.arbitration_id, msg.dlc) + sendStr = f"T{msg.arbitration_id:08X}{msg.dlc:d}" else: - sendStr = "t%03X%d" % (msg.arbitration_id, msg.dlc) - sendStr += "".join(["%02X" % b for b in msg.data]) + sendStr = f"t{msg.arbitration_id:03X}{msg.dlc:d}" + sendStr += msg.data.hex().upper() self._write(sendStr) def shutdown(self) -> None: @@ -295,29 +286,17 @@ def get_version( cmd = "V" self._write(cmd) - start = time.time() - time_left = timeout - while True: - string = self._read(time_left) - - if not string: - pass - elif string[0] == cmd and len(string) == 6: - # convert ASCII coded version - hw_version = int(string[1:3]) - sw_version = int(string[3:5]) - return hw_version, sw_version - # if timeout is None, try indefinitely - if timeout is None: - continue - # try next one only if there still is time, and with - # reduced timeout - else: - time_left = timeout - (time.time() - start) - if time_left > 0: - continue - else: - return None, None + string = self._read(timeout) + + if not string: + pass + elif string[0] == cmd and len(string) == 6: + # convert ASCII coded version + hw_version = int(string[1:3]) + sw_version = int(string[3:5]) + return hw_version, sw_version + + return None, None def get_serial_number(self, timeout: Optional[float]) -> Optional[str]: """Get serial number of the slcan interface. @@ -331,24 +310,12 @@ def get_serial_number(self, timeout: Optional[float]) -> Optional[str]: cmd = "N" self._write(cmd) - start = time.time() - time_left = timeout - while True: - string = self._read(time_left) - - if not string: - pass - elif string[0] == cmd and len(string) == 6: - serial_number = string[1:-1] - return serial_number - # if timeout is None, try indefinitely - if timeout is None: - continue - # try next one only if there still is time, and with - # reduced timeout - else: - time_left = timeout - (time.time() - start) - if time_left > 0: - continue - else: - return None + string = self._read(timeout) + + if not string: + pass + elif string[0] == cmd and len(string) == 6: + serial_number = string[1:-1] + return serial_number + + return None diff --git a/test/test_slcan.py b/test/test_slcan.py index aa97e518b..8db2d402a 100644 --- a/test/test_slcan.py +++ b/test/test_slcan.py @@ -2,11 +2,26 @@ import unittest import can +from .config import IS_PYPY + + +""" +Mentioned in #1010 & #1490 + +> PyPy works best with pure Python applications. Whenever you use a C extension module, +> it runs much slower than in CPython. The reason is that PyPy can't optimize C extension modules since they're not fully supported. +> In addition, PyPy has to emulate reference counting for that part of the code, making it even slower. + +https://realpython.com/pypy-faster-python/#it-doesnt-work-well-with-c-extensions +""" +TIMEOUT = 0.5 if IS_PYPY else 0.001 # 0.001 is the default set in slcanBus class slcanTestCase(unittest.TestCase): def setUp(self): - self.bus = can.Bus("loop://", interface="slcan", sleep_after_open=0) + self.bus = can.Bus( + "loop://", interface="slcan", sleep_after_open=0, timeout=TIMEOUT + ) self.serial = self.bus.serialPortOrig self.serial.read(self.serial.in_waiting) @@ -15,7 +30,7 @@ def tearDown(self): def test_recv_extended(self): self.serial.write(b"T12ABCDEF2AA55\r") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) self.assertEqual(msg.arbitration_id, 0x12ABCDEF) self.assertEqual(msg.is_extended_id, True) @@ -33,7 +48,7 @@ def test_send_extended(self): def test_recv_standard(self): self.serial.write(b"t4563112233\r") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) self.assertEqual(msg.arbitration_id, 0x456) self.assertEqual(msg.is_extended_id, False) @@ -51,7 +66,7 @@ def test_send_standard(self): def test_recv_standard_remote(self): self.serial.write(b"r1238\r") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) self.assertEqual(msg.arbitration_id, 0x123) self.assertEqual(msg.is_extended_id, False) @@ -68,7 +83,7 @@ def test_send_standard_remote(self): def test_recv_extended_remote(self): self.serial.write(b"R12ABCDEF6\r") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) self.assertEqual(msg.arbitration_id, 0x12ABCDEF) self.assertEqual(msg.is_extended_id, True) @@ -85,11 +100,11 @@ def test_send_extended_remote(self): def test_partial_recv(self): self.serial.write(b"T12ABCDEF") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNone(msg) self.serial.write(b"2AA55\rT12") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) self.assertEqual(msg.arbitration_id, 0x12ABCDEF) self.assertEqual(msg.is_extended_id, True) @@ -97,11 +112,11 @@ def test_partial_recv(self): self.assertEqual(msg.dlc, 2) self.assertSequenceEqual(msg.data, [0xAA, 0x55]) - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNone(msg) self.serial.write(b"ABCDEF2AA55\r") - msg = self.bus.recv(0) + msg = self.bus.recv(TIMEOUT) self.assertIsNotNone(msg) def test_version(self):