Add Support for Remote and Error Frames to SerialBus

.github/workflows/ci.yml

@@ -41,6 +41,7 @@ jobs:
     - name: Setup SocketCAN
       if: ${{ matrix.os == 'ubuntu-latest' }}
       run: |
+        sudo apt-get update
         sudo apt-get -y install linux-modules-extra-$(uname -r)
         sudo ./test/open_vcan.sh
     - name: Test with pytest via tox

can/interfaces/serial/serial_can.py

@@ -42,6 +42,13 @@ def list_comports() -> list[Any]:
         return []
 
 
+CAN_ERR_FLAG = 0x20000000
+CAN_RTR_FLAG = 0x40000000
+CAN_EFF_FLAG = 0x80000000
+CAN_ID_MASK_EXT = 0x1FFFFFFF
+CAN_ID_MASK_STD = 0x7FF
+
+
 class SerialBus(BusABC):
     """
     Enable basic can communication over a serial device.
@@ -116,9 +123,6 @@ def send(self, msg: Message, timeout: Optional[float] = None) -> None:
         :param msg:
             Message to send.
 
-            .. note:: Flags like ``extended_id``, ``is_remote_frame`` and
-                      ``is_error_frame`` will be ignored.
-
             .. note:: If the timestamp is a float value it will be converted
                       to an integer.
 
@@ -134,20 +138,25 @@ def send(self, msg: Message, timeout: Optional[float] = None) -> None:
             raise ValueError(f"Timestamp is out of range: {msg.timestamp}") from None
 
         # Pack arbitration ID
-        try:
-            arbitration_id = msg.arbitration_id + (0 if msg.is_extended_id else 0x20000000)
-            arbitration_id = struct.pack("<I", arbitration_id)
-        except struct.error:
-            raise ValueError(
-                f"Arbitration ID is out of range: {msg.arbitration_id}"
-            ) from None
+        if msg.is_extended_id:
+            arbitration_id = msg.arbitration_id & CAN_ID_MASK_EXT
+            arbitration_id |= CAN_EFF_FLAG
+        else:
+            arbitration_id = msg.arbitration_id & CAN_ID_MASK_STD
+
+        if msg.is_error_frame:
+            arbitration_id |= CAN_ERR_FLAG
+        if msg.is_remote_frame:
+            arbitration_id |= CAN_RTR_FLAG
+
+        arbitration_id_bytes = struct.pack("<I", arbitration_id)
 
         # Assemble message
         byte_msg = bytearray()
         byte_msg.append(0xAA)
         byte_msg += timestamp
         byte_msg.append(msg.dlc)
-        byte_msg += arbitration_id
+        byte_msg += arbitration_id_bytes
         byte_msg += msg.data
         byte_msg.append(0xBB)
 
@@ -172,11 +181,6 @@ def _recv_internal(
 
         :returns:
             Received message and :obj:`False` (because no filtering as taken place).
-
-            .. warning::
-                Flags like ``is_extended_id``, ``is_remote_frame`` and ``is_error_frame``
-                will not be set over this function, the flags in the return
-                message are the default values.
         """
         try:
             rx_byte = self._ser.read()
@@ -189,16 +193,14 @@ def _recv_internal(
 
                 s = self._ser.read(4)
                 arbitration_id = struct.unpack("<I", s)[0]
-                is_extended_id = False if arbitration_id & 0x20000000 else True
-                arbitration_id -= 0 if is_extended_id else 0x20000000
-                if is_extended_id and arbitration_id >= 0x20000000:
-                    raise ValueError(
-                        "received arbitration id may not exceed or equal 2^29 (0x20000000) if extended"
-                    )
-                if not is_extended_id and arbitration_id >= 0x800:
-                    raise ValueError(
-                        "received arbitration id may not exceed or equal 2^11 (0x800) if not extended"
-                    )
+                is_extended_id = bool(arbitration_id & CAN_EFF_FLAG)
+                is_error_frame = bool(arbitration_id & CAN_ERR_FLAG)
+                is_remote_frame = bool(arbitration_id & CAN_RTR_FLAG)
+
+                if is_extended_id:
+                    arbitration_id = arbitration_id & CAN_ID_MASK_EXT
+                else:
+                    arbitration_id = arbitration_id & CAN_ID_MASK_STD
 
                 data = self._ser.read(dlc)
 
@@ -212,6 +214,8 @@ def _recv_internal(
                         dlc=dlc,
                         data=data,
                         is_extended_id=is_extended_id,
+                        is_error_frame=is_error_frame,
+                        is_remote_frame=is_remote_frame,
                     )
                     return msg, False
 

doc/interfaces/serial.rst

@@ -30,9 +30,9 @@ six parts. The start and the stop byte for the frame, the timestamp, DLC,
 arbitration ID and the payload. The payload has a variable length of between
 0 and 8 bytes, the other parts are fixed. Both, the timestamp and the
 arbitration ID will be interpreted as 4 byte unsigned integers. The DLC is
-also an unsigned integer with a length of 1 byte. Non-extended (11-bit)
-identifiers are encoded by adding 0x20000000 to the 11-bit ID. For example, an
-11-bit CAN ID of 0x123 is encoded with an arbitration ID of 0x20000123.
+also an unsigned integer with a length of 1 byte. Extended (29-bit)
+identifiers are encoded by adding 0x80000000 to the ID. For example, a
+29-bit CAN ID of 0x123 is encoded with an arbitration ID of 0x80000123.
 
 Serial frame format
 ^^^^^^^^^^^^^^^^^^^
@@ -105,20 +105,20 @@ Examples of serial frames
 | 0xAA           | 0x66 0x73 0x00 0x00 | 0x00 | 0x01 0x00 0x00 0x00 | 0xBB         |
 +----------------+---------------------+------+---------------------+--------------+
 
-.. rubric:: CAN message with 0 byte payload with an 11-bit CAN ID
+.. rubric:: Extended Frame CAN message with 0 byte payload with an 29-bit CAN ID
 
 +----------------+---------+
 | CAN message              |
 +----------------+---------+
 | Arbitration ID | Payload |
 +================+=========+
-| 0x20000001 (1) | None    |
+| 0x80000001 (1) | None    |
 +----------------+---------+
 
 +----------------+---------------------+------+---------------------+--------------+
 | Serial frame                                                                     |
 +----------------+---------------------+------+---------------------+--------------+
 | Start of frame | Timestamp           | DLC  | Arbitration ID      | End of frame |
 +================+=====================+======+=====================+==============+
-| 0xAA           | 0x66 0x73 0x00 0x00 | 0x00 | 0x01 0x00 0x00 0x20 | 0xBB         |
+| 0xAA           | 0x66 0x73 0x00 0x00 | 0x00 | 0x01 0x00 0x00 0x80 | 0xBB         |
 +----------------+---------------------+------+---------------------+--------------+

test/serial_test.py

@@ -84,38 +84,38 @@ def test_rx_tx_data_none(self):
         msg_receive = self.bus.recv()
         self.assertMessageEqual(msg, msg_receive)
 
-    def test_rx_tx_min_id(self):
+    def test_rx_tx_min_std_id(self):
         """
-        Tests the transfer with the lowest extended arbitration id
+        Tests the transfer with the lowest standard arbitration id
         """
-        msg = can.Message(arbitration_id=0)
+        msg = can.Message(arbitration_id=0, is_extended_id=False)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
         self.assertMessageEqual(msg, msg_receive)
 
-    def test_rx_tx_max_id(self):
+    def test_rx_tx_max_std_id(self):
         """
-        Tests the transfer with the highest extended arbitration id
+        Tests the transfer with the highest standard arbitration id
         """
-        msg = can.Message(arbitration_id=536870911)
+        msg = can.Message(arbitration_id=0x7FF, is_extended_id=False)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
         self.assertMessageEqual(msg, msg_receive)
 
-    def test_rx_tx_min_nonext_id(self):
+    def test_rx_tx_min_ext_id(self):
         """
-        Tests the transfer with the lowest non-extended arbitration id
+        Tests the transfer with the lowest extended arbitration id
         """
-        msg = can.Message(arbitration_id=0x000, is_extended_id=False)
+        msg = can.Message(arbitration_id=0x000, is_extended_id=True)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
         self.assertMessageEqual(msg, msg_receive)
 
-    def test_rx_tx_max_nonext_id(self):
+    def test_rx_tx_max_ext_id(self):
         """
-        Tests the transfer with the highest non-extended arbitration id
+        Tests the transfer with the highest extended arbitration id
         """
-        msg = can.Message(arbitration_id=0x7FF, is_extended_id=False)
+        msg = can.Message(arbitration_id=0x1FFFFFFF, is_extended_id=True)
         self.bus.send(msg)
         msg_receive = self.bus.recv()
         self.assertMessageEqual(msg, msg_receive)
@@ -155,6 +155,28 @@ def test_rx_tx_min_timestamp_error(self):
         msg = can.Message(timestamp=-1)
         self.assertRaises(ValueError, self.bus.send, msg)
 
+    def test_rx_tx_err_frame(self):
+        """
+        Test the transfer of error frames.
+        """
+        msg = can.Message(
+            is_extended_id=False, is_error_frame=True, is_remote_frame=False
+        )
+        self.bus.send(msg)
+        msg_receive = self.bus.recv()
+        self.assertMessageEqual(msg, msg_receive)
+
+    def test_rx_tx_rtr_frame(self):
+        """
+        Test the transfer of remote frames.
+        """
+        msg = can.Message(
+            is_extended_id=False, is_error_frame=False, is_remote_frame=True
+        )
+        self.bus.send(msg)
+        msg_receive = self.bus.recv()
+        self.assertMessageEqual(msg, msg_receive)
+
     def test_when_no_fileno(self):
         """
         Tests for the fileno method catching the missing pyserial implementeation on the Windows platform