diff --git a/plin/device.py b/plin/device.py index 774ff0f..f2f0a73 100644 --- a/plin/device.py +++ b/plin/device.py @@ -514,25 +514,29 @@ def set_led_state(self, enable: bool): def read(self, block=True) -> Union[PLINMessage, None]: ''' - Reads a PLINMessage from the LIN bus with an optional timeout in milliseconds. + Reads the next PLINMessage the driver has queued. - A timeout value of 0 blocks until data is read. If the timeout is reached before data is read, None is returned. + Every record is returned as the device reported it, including frames whose data is all 0xFF: + that is what an unanswered header looks like (flags carry SLV_NOT_RSP / TIMEOUT), and it is + indistinguishable from a slave that answered with recessive bytes, so the caller must decide + from the flags, not the data. Non-FRAME records (SLEEP, WAKEUP, OVERRUN, ...) are returned + too; check message.type. + + Returns None when a non-blocking read finds nothing queued. ''' if self.fd: blocking = os.get_blocking(self.fd) os.set_blocking(self.fd, block) try: result = os.read(self.fd, PLINMessage.buffer_length) - message = PLINMessage.from_buffer_copy(result) - # If bytes read was invalid. - if bytes(message.data) == PLIN_EMPTY_DATA: - message = None - except KeyboardInterrupt: - raise KeyboardInterrupt - except: - message = None - os.set_blocking(self.fd, blocking) - return message + except BlockingIOError: + return None + finally: + os.set_blocking(self.fd, blocking) + + if len(result) < PLINMessage.buffer_length: + return None + return PLINMessage.from_buffer_copy(result) else: raise Exception("PLIN not connected!") diff --git a/pyproject.toml b/pyproject.toml index 1b681af..54a9f61 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "plin-linux" -version = "0.0.3" +version = "0.0.4" authors = [ { name="William Zhang", email="williamzhang@rivian.com" }, ] diff --git a/tests/unit/test_device.py b/tests/unit/test_device.py index b6a0588..a768de9 100644 --- a/tests/unit/test_device.py +++ b/tests/unit/test_device.py @@ -135,3 +135,59 @@ def test_get_status(plin_master, mock_ioctl): mock_ioctl.assert_called_once() _, ioctl_num, arg = mock_ioctl.mock_calls[0].args assert ioctl_num == PLIOGETSTATUS + + +def _record(id: int, flags: int, data: bytes, msg_type: int = 0, length: int = 8) -> bytes: + msg = PLINMessage(type=msg_type, flags=flags, id=id, len=length, dir=PLINFrameDirection.SUBSCRIBER, + cs_type=PLINFrameChecksumType.CLASSIC, ts_us=1234, data=bytearray(data)) + return bytes(msg) + + +def test_read_keeps_unanswered_header(plin_master): + # A header no slave answers arrives with recessive 0xFF data and SLV_NOT_RSP | TIMEOUT set. It + # used to be discarded as "invalid", which hid every empty poll from a passive logger. + plin, _ = plin_master + record = _record(0x3D, PLINFrameErrorFlag.SLV_NOT_RSP | PLINFrameErrorFlag.TIMEOUT, b"\xff" * 8) + + with patch("os.get_blocking", return_value=True), patch("os.set_blocking"), \ + patch("os.read", return_value=record): + message = plin.read() + + assert message is not None + assert message.id == 0x3D + assert message.flags == (PLINFrameErrorFlag.SLV_NOT_RSP | PLINFrameErrorFlag.TIMEOUT) + assert bytes(message.data) == b"\xff" * 8 + + +def test_read_returns_non_frame_records(plin_master): + plin, _ = plin_master + record = _record(0x00, 0, b"\x00" * 8, msg_type=1, length=0) # SLEEP + + with patch("os.get_blocking", return_value=True), patch("os.set_blocking"), \ + patch("os.read", return_value=record): + message = plin.read() + + assert message is not None + assert message.type == 1 + + +def test_read_nonblocking_empty_returns_none(plin_master): + plin, _ = plin_master + + with patch("os.get_blocking", return_value=True), patch("os.set_blocking") as set_blocking, \ + patch("os.read", side_effect=BlockingIOError): + message = plin.read(block=False) + + assert message is None + # Blocking mode is restored even when nothing was queued. + set_blocking.assert_called_with(plin.fd, True) + + +def test_read_short_record_returns_none(plin_master): + plin, _ = plin_master + + with patch("os.get_blocking", return_value=True), patch("os.set_blocking"), \ + patch("os.read", return_value=b"\x00" * 4): + message = plin.read() + + assert message is None