From e750caa7a3a8ab2e1ce34a3a9f4f835621ebecc7 Mon Sep 17 00:00:00 2001 From: Gal Rahamim Date: Thu, 2 May 2024 12:14:34 +0000 Subject: [PATCH 1/6] CC-2174: modify CAN frame header structure to match updated struct can_frame from kernel --- can/interfaces/socketcan/constants.py | 14 +++- can/interfaces/socketcan/socketcan.py | 94 +++++++++++++++++++++++---- 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/can/interfaces/socketcan/constants.py b/can/interfaces/socketcan/constants.py index 3144a2cfa..941d52573 100644 --- a/can/interfaces/socketcan/constants.py +++ b/can/interfaces/socketcan/constants.py @@ -53,8 +53,18 @@ SIOCGSTAMP = 0x8906 EXTFLG = 0x0004 -CANFD_BRS = 0x01 -CANFD_ESI = 0x02 +CANFD_BRS = 0x01 # bit rate switch (second bitrate for payload data) +CANFD_ESI = 0x02 # error state indicator of the transmitting node +CANFD_FDF = 0x04 # mark CAN FD for dual use of struct canfd_frame + +# CAN payload length and DLC definitions according to ISO 11898-1 +CAN_MAX_DLC = 8 +CAN_MAX_RAW_DLC = 15 +CAN_MAX_DLEN = 8 + +# CAN FD payload length and DLC definitions according to ISO 11898-7 +CANFD_MAX_DLC = 15 +CANFD_MAX_DLEN = 64 CANFD_MTU = 72 diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 03bd8c3f8..6640ea62b 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -139,23 +139,68 @@ def bcm_header_factory( # The 32bit can id is directly followed by the 8bit data link count # The data field is aligned on an 8 byte boundary, hence we add padding # which aligns the data field to an 8 byte boundary. -CAN_FRAME_HEADER_STRUCT = struct.Struct("=IBB2x") +CAN_FRAME_HEADER_STRUCT = struct.Struct("=IBB1xB") def build_can_frame(msg: Message) -> bytes: """CAN frame packing/unpacking (see 'struct can_frame' in ) /** - * struct can_frame - basic CAN frame structure - * @can_id: the CAN ID of the frame and CAN_*_FLAG flags, see above. - * @can_dlc: the data length field of the CAN frame - * @data: the CAN frame payload. - */ + * struct can_frame - Classical CAN frame structure (aka CAN 2.0B) + * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition + * @len: CAN frame payload length in byte (0 .. 8) + * @can_dlc: deprecated name for CAN frame payload length in byte (0 .. 8) + * @__pad: padding + * @__res0: reserved / padding + * @len8_dlc: optional DLC value (9 .. 15) at 8 byte payload length + * len8_dlc contains values from 9 .. 15 when the payload length is + * 8 bytes but the DLC value (see ISO 11898-1) is greater then 8. + * CAN_CTRLMODE_CC_LEN8_DLC flag has to be enabled in CAN driver. + * @data: CAN frame payload (up to 8 byte) + */ struct can_frame { canid_t can_id; /* 32 bit CAN_ID + EFF/RTR/ERR flags */ - __u8 can_dlc; /* data length code: 0 .. 8 */ - __u8 data[8] __attribute__((aligned(8))); + union { + /* CAN frame payload length in byte (0 .. CAN_MAX_DLEN) + * was previously named can_dlc so we need to carry that + * name for legacy support + */ + __u8 len; + __u8 can_dlc; /* deprecated */ + } __attribute__((packed)); /* disable padding added in some ABIs */ + __u8 __pad; /* padding */ + __u8 __res0; /* reserved / padding */ + __u8 len8_dlc; /* optional DLC for 8 byte payload length (9 .. 15) */ + __u8 data[CAN_MAX_DLEN] __attribute__((aligned(8))); }; + /* + * defined bits for canfd_frame.flags + * + * The use of struct canfd_frame implies the FD Frame (FDF) bit to + * be set in the CAN frame bitstream on the wire. The FDF bit switch turns + * the CAN controllers bitstream processor into the CAN FD mode which creates + * two new options within the CAN FD frame specification: + * + * Bit Rate Switch - to indicate a second bitrate is/was used for the payload + * Error State Indicator - represents the error state of the transmitting node + * + * As the CANFD_ESI bit is internally generated by the transmitting CAN + * controller only the CANFD_BRS bit is relevant for real CAN controllers when + * building a CAN FD frame for transmission. Setting the CANFD_ESI bit can make + * sense for virtual CAN interfaces to test applications with echoed frames. + * + * The struct can_frame and struct canfd_frame intentionally share the same + * layout to be able to write CAN frame content into a CAN FD frame structure. + * When this is done the former differentiation via CAN_MTU / CANFD_MTU gets + * lost. CANFD_FDF allows programmers to mark CAN FD frames in the case of + * using struct canfd_frame for mixed CAN / CAN FD content (dual use). + * Since the introduction of CAN XL the CANFD_FDF flag is set in all CAN FD + * frame structures provided by the CAN subsystem of the Linux kernel. + */ + #define CANFD_BRS 0x01 /* bit rate switch (second bitrate for payload data) */ + #define CANFD_ESI 0x02 /* error state indicator of the transmitting node */ + #define CANFD_FDF 0x04 /* mark CAN FD for dual use of struct canfd_frame */ + /** * struct canfd_frame - CAN flexible data rate frame structure * @can_id: CAN ID of the frame and CAN_*_FLAG flags, see canid_t definition @@ -175,14 +220,25 @@ def build_can_frame(msg: Message) -> bytes: }; """ can_id = _compose_arbitration_id(msg) + flags = 0 + + # The socketcan code identify the received FD frame by the packet length. + # Therefore, padding to the data length is performed according to the message type (Classic / FD) + if msg.is_fd: + flags |= constants.CANFD_FDF + max_len = constants.CANFD_MAX_DLEN + else: + max_len = constants.CAN_MAX_DLEN + if msg.bitrate_switch: flags |= constants.CANFD_BRS if msg.error_state_indicator: flags |= constants.CANFD_ESI - max_len = 64 if msg.is_fd else 8 + data = bytes(msg.data).ljust(max_len, b"\x00") - return CAN_FRAME_HEADER_STRUCT.pack(can_id, msg.dlc, flags) + data + + return CAN_FRAME_HEADER_STRUCT.pack(can_id, len(msg.data), flags, msg.dlc) + data def build_bcm_header( @@ -260,11 +316,22 @@ def build_bcm_update_header(can_id: int, msg_flags: int, nframes: int = 1) -> by def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: - can_id, can_dlc, flags = CAN_FRAME_HEADER_STRUCT.unpack_from(frame) + can_id, data_len, flags, len8_dlc = CAN_FRAME_HEADER_STRUCT.unpack_from(frame) if len(frame) != constants.CANFD_MTU: # Flags not valid in non-FD frames flags = 0 - return can_id, can_dlc, flags, frame[8 : 8 + can_dlc] + + # Allow deprecated can frames with old struct + if ( + data_len == constants.CAN_MAX_DLEN and + len8_dlc > constants.CAN_MAX_DLEN and + len8_dlc <= constants.CAN_MAX_RAW_DLC + ): + can_dlc = len8_dlc + else: + can_dlc = data_len + + return can_id, can_dlc, flags, frame[8: 8 + data_len] def create_bcm_socket(channel: str) -> socket.socket: @@ -292,7 +359,8 @@ def send_bcm(bcm_socket: socket.socket, data: bytes) -> int: else: specific_message = "" - raise can.CanOperationError(base + specific_message, error.errno) from error + raise can.CanOperationError( + base + specific_message, error.errno) from error def _compose_arbitration_id(message: Message) -> int: From 6bdf926f8b8d0fe47ff7f3eaa2e7eb40a0fa48dd Mon Sep 17 00:00:00 2001 From: Eldad Sitbon Date: Thu, 5 Sep 2024 09:29:27 +0000 Subject: [PATCH 2/6] allow only valid fd message lengths --- can/interfaces/socketcan/socketcan.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 6640ea62b..06b641e64 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -238,7 +238,8 @@ def build_can_frame(msg: Message) -> bytes: data = bytes(msg.data).ljust(max_len, b"\x00") - return CAN_FRAME_HEADER_STRUCT.pack(can_id, len(msg.data), flags, msg.dlc) + data + data_len = min(i for i in can.util.CAN_FD_DLC if i >= len(msg.data)) + return CAN_FRAME_HEADER_STRUCT.pack(can_id, data_len, flags, msg.dlc) + data def build_bcm_header( @@ -321,17 +322,20 @@ def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: # Flags not valid in non-FD frames flags = 0 + if data_len not in can.util.CAN_FD_DLC: + data_len = min(i for i in can.util.CAN_FD_DLC if i >= data_len) + # Allow deprecated can frames with old struct if ( - data_len == constants.CAN_MAX_DLEN and - len8_dlc > constants.CAN_MAX_DLEN and - len8_dlc <= constants.CAN_MAX_RAW_DLC + data_len == constants.CAN_MAX_DLEN + and len8_dlc > constants.CAN_MAX_DLEN + and len8_dlc <= constants.CAN_MAX_RAW_DLC ): can_dlc = len8_dlc else: can_dlc = data_len - return can_id, can_dlc, flags, frame[8: 8 + data_len] + return can_id, can_dlc, flags, frame[8 : 8 + data_len] def create_bcm_socket(channel: str) -> socket.socket: @@ -359,8 +363,7 @@ def send_bcm(bcm_socket: socket.socket, data: bytes) -> int: else: specific_message = "" - raise can.CanOperationError( - base + specific_message, error.errno) from error + raise can.CanOperationError(base + specific_message, error.errno) from error def _compose_arbitration_id(message: Message) -> int: From 3013dd53f923322beaffe051a5b7b8ac46dca40b Mon Sep 17 00:00:00 2001 From: Eldad Sitbon Date: Sun, 8 Sep 2024 07:05:05 +0000 Subject: [PATCH 3/6] add remote message special case for dlc handling --- can/interfaces/socketcan/socketcan.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 06b641e64..b9f58f089 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -238,8 +238,10 @@ def build_can_frame(msg: Message) -> bytes: data = bytes(msg.data).ljust(max_len, b"\x00") - data_len = min(i for i in can.util.CAN_FD_DLC if i >= len(msg.data)) - return CAN_FRAME_HEADER_STRUCT.pack(can_id, data_len, flags, msg.dlc) + data + if msg.is_remote_frame: + data_len = msg.dlc + else: + data_len = min(i for i in can.util.CAN_FD_DLC if i >= len(msg.data)) def build_bcm_header( From 3d602a36e2abc66cd510e4ab82d65baaa37ee192 Mon Sep 17 00:00:00 2001 From: Eldad Sitbon Date: Sun, 8 Sep 2024 07:05:26 +0000 Subject: [PATCH 4/6] fix pylint issues --- can/interfaces/socketcan/socketcan.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index b9f58f089..3bcbc2142 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -224,7 +224,7 @@ def build_can_frame(msg: Message) -> bytes: flags = 0 # The socketcan code identify the received FD frame by the packet length. - # Therefore, padding to the data length is performed according to the message type (Classic / FD) + # So, padding to the data length is performed according to the message type (Classic / FD) if msg.is_fd: flags |= constants.CANFD_FDF max_len = constants.CANFD_MAX_DLEN @@ -242,6 +242,8 @@ def build_can_frame(msg: Message) -> bytes: data_len = msg.dlc else: data_len = min(i for i in can.util.CAN_FD_DLC if i >= len(msg.data)) + header = CAN_FRAME_HEADER_STRUCT.pack(can_id, data_len, flags, msg.dlc) + return header + data def build_bcm_header( @@ -330,8 +332,7 @@ def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: # Allow deprecated can frames with old struct if ( data_len == constants.CAN_MAX_DLEN - and len8_dlc > constants.CAN_MAX_DLEN - and len8_dlc <= constants.CAN_MAX_RAW_DLC + and constants.CAN_MAX_DLEN < len8_dlc <= constants.CAN_MAX_RAW_DLC ): can_dlc = len8_dlc else: From 6960e51c0cb97b8ad05a655d4cef3e9de63e4c46 Mon Sep 17 00:00:00 2001 From: Gal Rahamim Date: Tue, 11 Feb 2025 10:04:30 +0200 Subject: [PATCH 5/6] verify non FD frame before assigning the len8 DLC --- can/interfaces/socketcan/socketcan.py | 28 ++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 3bcbc2142..1d0646678 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -320,23 +320,29 @@ def build_bcm_update_header(can_id: int, msg_flags: int, nframes: int = 1) -> by ) +def is_frame_fd(frame: bytes): + # According to the SocketCAN implementation the frame length + # should indicate if the message is FD or not (not the flag value) + return len(frame) == constants.CANFD_MTU + + def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: can_id, data_len, flags, len8_dlc = CAN_FRAME_HEADER_STRUCT.unpack_from(frame) - if len(frame) != constants.CANFD_MTU: - # Flags not valid in non-FD frames - flags = 0 if data_len not in can.util.CAN_FD_DLC: data_len = min(i for i in can.util.CAN_FD_DLC if i >= data_len) + + can_dlc = data_len - # Allow deprecated can frames with old struct - if ( - data_len == constants.CAN_MAX_DLEN - and constants.CAN_MAX_DLEN < len8_dlc <= constants.CAN_MAX_RAW_DLC - ): - can_dlc = len8_dlc - else: - can_dlc = data_len + if not is_frame_fd(frame): + # Flags not valid in non-FD frames + flags = 0 + + if ( + data_len == constants.CAN_MAX_DLEN + and constants.CAN_MAX_DLEN < len8_dlc <= constants.CAN_MAX_RAW_DLC + ): + can_dlc = len8_dlc return can_id, can_dlc, flags, frame[8 : 8 + data_len] From 6dcfcc5a8234652872cfeadb4e2ce8886c4cff23 Mon Sep 17 00:00:00 2001 From: Gal Rahamim Date: Tue, 11 Feb 2025 10:42:21 +0200 Subject: [PATCH 6/6] Fix formatting --- can/interfaces/socketcan/socketcan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 1fcbf9c1e..a5819dcb5 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -321,7 +321,7 @@ def build_bcm_update_header(can_id: int, msg_flags: int, nframes: int = 1) -> by def is_frame_fd(frame: bytes): - # According to the SocketCAN implementation the frame length + # According to the SocketCAN implementation the frame length # should indicate if the message is FD or not (not the flag value) return len(frame) == constants.CANFD_MTU @@ -331,7 +331,7 @@ def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: if data_len not in can.util.CAN_FD_DLC: data_len = min(i for i in can.util.CAN_FD_DLC if i >= data_len) - + can_dlc = data_len if not is_frame_fd(frame):