From eb0331d140afd9ee940d57a5a5f324e6da9fbb15 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Sat, 31 Dec 2022 19:42:57 +0100 Subject: [PATCH 1/7] Update pylint (#1471) * update pylint version * add coverage.lcov to .gitignore * check can/io with pylint * check can/interfaces/socketcan with pylint * address review comments * replace wildcard import * fix TRCWriter line endings Co-authored-by: zariiii9003 --- .github/workflows/ci.yml | 6 +- .gitignore | 1 + .pylintrc | 3 +- can/bus.py | 4 +- can/interfaces/socketcan/socketcan.py | 94 +++++++++-------- can/interfaces/socketcan/utils.py | 3 +- can/io/asc.py | 13 +-- can/io/blf.py | 2 - can/io/canutils.py | 10 +- can/io/csv.py | 2 - can/io/generic.py | 25 +++-- can/io/logger.py | 28 +++-- can/io/player.py | 5 +- can/io/printer.py | 1 - can/io/sqlite.py | 2 - can/io/trc.py | 142 ++++++++++++-------------- can/listener.py | 10 +- can/logconvert.py | 2 +- can/logger.py | 2 +- can/message.py | 4 +- requirements-lint.txt | 2 +- 21 files changed, 170 insertions(+), 191 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8bcc273ab..c6a5c4253 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,10 +91,12 @@ jobs: run: | pylint --rcfile=.pylintrc \ can/**.py \ + can/io \ setup.py \ - doc.conf \ + doc/conf.py \ scripts/**.py \ - examples/**.py + examples/**.py \ + can/interfaces/socketcan format: runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 258ca73ea..03775bd7c 100644 --- a/.gitignore +++ b/.gitignore @@ -52,6 +52,7 @@ htmlcov/ .cache nosetests.xml coverage.xml +coverage.lcov *,cover .hypothesis/ test.* diff --git a/.pylintrc b/.pylintrc index a42935e6a..cc4c50d88 100644 --- a/.pylintrc +++ b/.pylintrc @@ -81,7 +81,8 @@ disable=invalid-name, # either give multiple identifier separated by comma (,) or put this option # multiple time (only on the command line, not in the configuration file where # it should appear only once). See also the "--disable" option for examples. -enable=c-extension-no-member +enable=c-extension-no-member, + useless-suppression, [REPORTS] diff --git a/can/bus.py b/can/bus.py index c0793c5f7..d0192fc2d 100644 --- a/can/bus.py +++ b/can/bus.py @@ -464,7 +464,5 @@ class _SelfRemovingCyclicTask(CyclicSendTaskABC, ABC): Only needed for typing :meth:`Bus._periodic_tasks`. Do not instantiate. """ - def stop( # pylint: disable=arguments-differ - self, remove_task: bool = True - ) -> None: + def stop(self, remove_task: bool = True) -> None: raise NotImplementedError() diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index f0545f7df..74fbe8197 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -21,12 +21,6 @@ log_tx = log.getChild("tx") log_rx = log.getChild("rx") -try: - import fcntl -except ImportError: - log.error("fcntl not available on this platform") - - try: from socket import CMSG_SPACE @@ -44,7 +38,7 @@ LimitedDurationCyclicSendTaskABC, ) from can.typechecking import CanFilters -from can.interfaces.socketcan.constants import * # CAN_RAW, CAN_*_FLAG +from can.interfaces.socketcan import constants from can.interfaces.socketcan.utils import pack_filters, find_available_interfaces @@ -177,9 +171,9 @@ def build_can_frame(msg: Message) -> bytes: can_id = _compose_arbitration_id(msg) flags = 0 if msg.bitrate_switch: - flags |= CANFD_BRS + flags |= constants.CANFD_BRS if msg.error_state_indicator: - flags |= CANFD_ESI + 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 @@ -211,7 +205,7 @@ def build_bcm_header( def build_bcm_tx_delete_header(can_id: int, flags: int) -> bytes: - opcode = CAN_BCM_TX_DELETE + opcode = constants.CAN_BCM_TX_DELETE return build_bcm_header(opcode, flags, 0, 0, 0, 0, 0, can_id, 1) @@ -223,13 +217,13 @@ def build_bcm_transmit_header( msg_flags: int, nframes: int = 1, ) -> bytes: - opcode = CAN_BCM_TX_SETUP + opcode = constants.CAN_BCM_TX_SETUP - flags = msg_flags | SETTIMER | STARTTIMER + flags = msg_flags | constants.SETTIMER | constants.STARTTIMER if initial_period > 0: # Note `TX_COUNTEVT` creates the message TX_EXPIRED when count expires - flags |= TX_COUNTEVT + flags |= constants.TX_COUNTEVT def split_time(value: float) -> Tuple[int, int]: """Given seconds as a float, return whole seconds and microseconds""" @@ -254,12 +248,14 @@ def split_time(value: float) -> Tuple[int, int]: def build_bcm_update_header(can_id: int, msg_flags: int, nframes: int = 1) -> bytes: - return build_bcm_header(CAN_BCM_TX_SETUP, msg_flags, 0, 0, 0, 0, 0, can_id, nframes) + return build_bcm_header( + constants.CAN_BCM_TX_SETUP, msg_flags, 0, 0, 0, 0, 0, can_id, nframes + ) def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: can_id, can_dlc, flags = CAN_FRAME_HEADER_STRUCT.unpack_from(frame) - if len(frame) != CANFD_MTU: + 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] @@ -267,7 +263,7 @@ def dissect_can_frame(frame: bytes) -> Tuple[int, int, int, bytes]: def create_bcm_socket(channel: str) -> socket.socket: """create a broadcast manager socket and connect to the given interface""" - s = socket.socket(PF_CAN, socket.SOCK_DGRAM, CAN_BCM) + s = socket.socket(constants.PF_CAN, socket.SOCK_DGRAM, constants.CAN_BCM) s.connect((channel,)) return s @@ -297,13 +293,13 @@ def _compose_arbitration_id(message: Message) -> int: can_id = message.arbitration_id if message.is_extended_id: log.debug("sending an extended id type message") - can_id |= CAN_EFF_FLAG + can_id |= constants.CAN_EFF_FLAG if message.is_remote_frame: log.debug("requesting a remote frame") - can_id |= CAN_RTR_FLAG + can_id |= constants.CAN_RTR_FLAG if message.is_error_frame: log.debug("sending error frame") - can_id |= CAN_ERR_FLAG + can_id |= constants.CAN_ERR_FLAG return can_id @@ -354,7 +350,7 @@ def _tx_setup( ) -> None: # Create a low level packed frame to pass to the kernel body = bytearray() - self.flags = CAN_FD_FRAME if messages[0].is_fd else 0 + self.flags = constants.CAN_FD_FRAME if messages[0].is_fd else 0 if self.duration: count = int(self.duration / self.period) @@ -380,7 +376,7 @@ def _check_bcm_task(self) -> None: # Do a TX_READ on a task ID, and check if we get EINVAL. If so, # then we are referring to a CAN message with an existing ID check_header = build_bcm_header( - opcode=CAN_BCM_TX_READ, + opcode=constants.CAN_BCM_TX_READ, flags=0, count=0, ival1_seconds=0, @@ -391,7 +387,7 @@ def _check_bcm_task(self) -> None: nframes=0, ) log.debug( - f"Reading properties of (cyclic) transmission task id={self.task_id}", + "Reading properties of (cyclic) transmission task id=%d", self.task_id ) try: self.bcm_socket.send(check_header) @@ -495,7 +491,7 @@ def create_socket() -> socket.socket: """Creates a raw CAN socket. The socket will be returned unbound to any interface. """ - sock = socket.socket(PF_CAN, socket.SOCK_RAW, CAN_RAW) + sock = socket.socket(constants.PF_CAN, socket.SOCK_RAW, constants.CAN_RAW) log.info("Created a socket") @@ -534,7 +530,7 @@ def capture_message( # Fetching the Arb ID, DLC and Data try: cf, ancillary_data, msg_flags, addr = sock.recvmsg( - CANFD_MTU, RECEIVED_ANCILLARY_BUFFER_SIZE + constants.CANFD_MTU, RECEIVED_ANCILLARY_BUFFER_SIZE ) if get_channel: channel = addr[0] if isinstance(addr, tuple) else addr @@ -549,7 +545,7 @@ def capture_message( assert len(ancillary_data) == 1, "only requested a single extra field" cmsg_level, cmsg_type, cmsg_data = ancillary_data[0] assert ( - cmsg_level == socket.SOL_SOCKET and cmsg_type == SO_TIMESTAMPNS + cmsg_level == socket.SOL_SOCKET and cmsg_type == constants.SO_TIMESTAMPNS ), "received control message type that was not requested" # see https://man7.org/linux/man-pages/man3/timespec.3.html -> struct timespec for details seconds, nanoseconds = RECEIVED_TIMESTAMP_STRUCT.unpack_from(cmsg_data) @@ -564,12 +560,12 @@ def capture_message( # #define CAN_EFF_FLAG 0x80000000U /* EFF/SFF is set in the MSB */ # #define CAN_RTR_FLAG 0x40000000U /* remote transmission request */ # #define CAN_ERR_FLAG 0x20000000U /* error frame */ - is_extended_frame_format = bool(can_id & CAN_EFF_FLAG) - is_remote_transmission_request = bool(can_id & CAN_RTR_FLAG) - is_error_frame = bool(can_id & CAN_ERR_FLAG) - is_fd = len(cf) == CANFD_MTU - bitrate_switch = bool(flags & CANFD_BRS) - error_state_indicator = bool(flags & CANFD_ESI) + is_extended_frame_format = bool(can_id & constants.CAN_EFF_FLAG) + is_remote_transmission_request = bool(can_id & constants.CAN_RTR_FLAG) + is_error_frame = bool(can_id & constants.CAN_ERR_FLAG) + is_fd = len(cf) == constants.CANFD_MTU + bitrate_switch = bool(flags & constants.CANFD_BRS) + error_state_indicator = bool(flags & constants.CANFD_ESI) # Section 4.7.1: MSG_DONTROUTE: set when the received frame was created on the local host. is_rx = not bool(msg_flags & socket.MSG_DONTROUTE) @@ -625,8 +621,8 @@ def __init__( ) -> None: """Creates a new socketcan bus. - If setting some socket options fails, an error will be printed but no exception will be thrown. - This includes enabling: + If setting some socket options fails, an error will be printed + but no exception will be thrown. This includes enabling: - that own messages should be received, - CAN-FD frames and @@ -656,7 +652,7 @@ def __init__( """ self.socket = create_socket() self.channel = channel - self.channel_info = "socketcan channel '%s'" % channel + self.channel_info = f"socketcan channel '{channel}'" self._bcm_sockets: Dict[str, socket.socket] = {} self._is_filtered = False self._task_id = 0 @@ -665,7 +661,9 @@ def __init__( # set the local_loopback parameter try: self.socket.setsockopt( - SOL_CAN_RAW, CAN_RAW_LOOPBACK, 1 if local_loopback else 0 + constants.SOL_CAN_RAW, + constants.CAN_RAW_LOOPBACK, + 1 if local_loopback else 0, ) except OSError as error: log.error("Could not set local loopback flag(%s)", error) @@ -673,7 +671,9 @@ def __init__( # set the receive_own_messages parameter try: self.socket.setsockopt( - SOL_CAN_RAW, CAN_RAW_RECV_OWN_MSGS, 1 if receive_own_messages else 0 + constants.SOL_CAN_RAW, + constants.CAN_RAW_RECV_OWN_MSGS, + 1 if receive_own_messages else 0, ) except OSError as error: log.error("Could not receive own messages (%s)", error) @@ -681,23 +681,27 @@ def __init__( # enable CAN-FD frames if desired if fd: try: - self.socket.setsockopt(SOL_CAN_RAW, CAN_RAW_FD_FRAMES, 1) + self.socket.setsockopt( + constants.SOL_CAN_RAW, constants.CAN_RAW_FD_FRAMES, 1 + ) except OSError as error: log.error("Could not enable CAN-FD frames (%s)", error) if not ignore_rx_error_frames: # enable error frames try: - self.socket.setsockopt(SOL_CAN_RAW, CAN_RAW_ERR_FILTER, 0x1FFFFFFF) + self.socket.setsockopt( + constants.SOL_CAN_RAW, constants.CAN_RAW_ERR_FILTER, 0x1FFFFFFF + ) except OSError as error: log.error("Could not enable error frames (%s)", error) # enable nanosecond resolution timestamping # we can always do this since - # 1) is is guaranteed to be at least as precise as without + # 1) it is guaranteed to be at least as precise as without # 2) it is available since Linux 2.6.22, and CAN support was only added afterward # so this is always supported by the kernel - self.socket.setsockopt(socket.SOL_SOCKET, SO_TIMESTAMPNS, 1) + self.socket.setsockopt(socket.SOL_SOCKET, constants.SO_TIMESTAMPNS, 1) bind_socket(self.socket, channel) kwargs.update( @@ -830,7 +834,9 @@ def _send_periodic_internal( general the message will be sent at the given rate until at least *duration* seconds. """ - msgs = LimitedDurationCyclicSendTaskABC._check_and_convert_messages(msgs) + msgs = LimitedDurationCyclicSendTaskABC._check_and_convert_messages( # pylint: disable=protected-access + msgs + ) msgs_channel = str(msgs[0].channel) if msgs[0].channel else None bcm_socket = self._get_bcm_socket(msgs_channel or self.channel) @@ -850,7 +856,9 @@ def _get_bcm_socket(self, channel: str) -> socket.socket: def _apply_filters(self, filters: Optional[can.typechecking.CanFilters]) -> None: try: - self.socket.setsockopt(SOL_CAN_RAW, CAN_RAW_FILTER, pack_filters(filters)) + self.socket.setsockopt( + constants.SOL_CAN_RAW, constants.CAN_RAW_FILTER, pack_filters(filters) + ) except OSError as error: # fall back to "software filtering" (= not in kernel) self._is_filtered = False @@ -899,8 +907,6 @@ def sender(event: threading.Event) -> None: sender_socket.send(build_can_frame(msg)) print("Sender sent a message.") - import threading - e = threading.Event() threading.Thread(target=receiver, args=(e,)).start() threading.Thread(target=sender, args=(e,)).start() diff --git a/can/interfaces/socketcan/utils.py b/can/interfaces/socketcan/utils.py index ecc870ca4..25f04617f 100644 --- a/can/interfaces/socketcan/utils.py +++ b/can/interfaces/socketcan/utils.py @@ -52,7 +52,8 @@ def find_available_interfaces() -> Iterable[str]: command = ["ip", "-o", "link", "list", "up"] output = subprocess.check_output(command, text=True) - except Exception as e: # subprocess.CalledProcessError is too specific + except Exception as e: # pylint: disable=broad-except + # subprocess.CalledProcessError is too specific log.error("failed to fetch opened can devices: %s", e) return [] diff --git a/can/io/asc.py b/can/io/asc.py index 7cefa5b76..eb59c0471 100644 --- a/can/io/asc.py +++ b/can/io/asc.py @@ -2,7 +2,7 @@ Contains handling of ASC logging files. Example .asc files: - - https://bitbucket.org/tobylorenz/vector_asc/src/47556e1a6d32c859224ca62d075e1efcc67fa690/src/Vector/ASC/tests/unittests/data/CAN_Log_Trigger_3_2.asc?at=master&fileviewer=file-view-default + - https://bitbucket.org/tobylorenz/vector_asc/src/master/src/Vector/ASC/tests/unittests/data/ - under `test/data/logfile.asc` """ import re @@ -39,7 +39,6 @@ def __init__( file: Union[StringPathLike, TextIO], base: str = "hex", relative_timestamp: bool = True, - *args: Any, **kwargs: Any, ) -> None: """ @@ -93,7 +92,7 @@ def _extract_header(self) -> None: ) continue - elif base_match: + if base_match: base = base_match.group("base") timestamp_format = base_match.group("timestamp_format") self.base = base @@ -101,15 +100,14 @@ def _extract_header(self) -> None: self.timestamps_format = timestamp_format or "absolute" continue - elif comment_match: + if comment_match: continue - elif events_match: + if events_match: self.internal_events_logged = events_match.group("no_events") is None break - else: - break + break @staticmethod def _datetime_to_timestamp(datetime_string: str) -> float: @@ -354,7 +352,6 @@ def __init__( self, file: Union[StringPathLike, TextIO], channel: int = 1, - *args: Any, **kwargs: Any, ) -> None: """ diff --git a/can/io/blf.py b/can/io/blf.py index 93fa54ca2..8d5ade8c8 100644 --- a/can/io/blf.py +++ b/can/io/blf.py @@ -146,7 +146,6 @@ class BLFReader(MessageReader): def __init__( self, file: Union[StringPathLike, BinaryIO], - *args: Any, **kwargs: Any, ) -> None: """ @@ -375,7 +374,6 @@ def __init__( append: bool = False, channel: int = 1, compression_level: int = -1, - *args: Any, **kwargs: Any, ) -> None: """ diff --git a/can/io/canutils.py b/can/io/canutils.py index e159ecdf4..c57a6ca97 100644 --- a/can/io/canutils.py +++ b/can/io/canutils.py @@ -37,7 +37,6 @@ class CanutilsLogReader(MessageReader): def __init__( self, file: Union[StringPathLike, TextIO], - *args: Any, **kwargs: Any, ) -> None: """ @@ -137,7 +136,6 @@ def __init__( file: Union[StringPathLike, TextIO], channel: str = "vcan0", append: bool = False, - *args: Any, **kwargs: Any, ): """ @@ -173,11 +171,11 @@ def on_message_received(self, msg): framestr = f"({timestamp:f}) {channel}" if msg.is_error_frame: - framestr += " %08X#" % (CAN_ERR_FLAG | CAN_ERR_BUSERROR) + framestr += f" {CAN_ERR_FLAG | CAN_ERR_BUSERROR:08X}#" elif msg.is_extended_id: - framestr += " %08X#" % (msg.arbitration_id) + framestr += f" {msg.arbitration_id:08X}#" else: - framestr += " %03X#" % (msg.arbitration_id) + framestr += f" {msg.arbitration_id:03X}#" if msg.is_error_frame: eol = "\n" @@ -193,7 +191,7 @@ def on_message_received(self, msg): fd_flags |= CANFD_BRS if msg.error_state_indicator: fd_flags |= CANFD_ESI - framestr += "#%X" % fd_flags + framestr += f"#{fd_flags:X}" framestr += f"{msg.data.hex().upper()}{eol}" self.file.write(framestr) diff --git a/can/io/csv.py b/can/io/csv.py index 2e2f46699..ecfc5de35 100644 --- a/can/io/csv.py +++ b/can/io/csv.py @@ -31,7 +31,6 @@ class CSVReader(MessageReader): def __init__( self, file: Union[StringPathLike, TextIO], - *args: Any, **kwargs: Any, ) -> None: """ @@ -95,7 +94,6 @@ def __init__( self, file: Union[StringPathLike, TextIO], append: bool = False, - *args: Any, **kwargs: Any, ) -> None: """ diff --git a/can/io/generic.py b/can/io/generic.py index d5c7a2057..77bba4501 100644 --- a/can/io/generic.py +++ b/can/io/generic.py @@ -1,5 +1,5 @@ """Contains generic base classes for file IO.""" - +import locale from abc import ABCMeta from typing import ( Optional, @@ -32,8 +32,7 @@ def __init__( self, file: Optional[can.typechecking.AcceptedIOType], mode: str = "rt", - *args: Any, - **kwargs: Any + **kwargs: Any, ) -> None: """ :param file: a path-like object to open a file, a file-like object @@ -45,11 +44,18 @@ def __init__( # file is None or some file-like object self.file = cast(Optional[can.typechecking.FileLike], file) else: + encoding: Optional[str] = ( + None + if "b" in mode + else kwargs.get("encoding", locale.getpreferredencoding(False)) + ) # pylint: disable=consider-using-with # file is some path-like object self.file = cast( can.typechecking.FileLike, - open(cast(can.typechecking.StringPathLike, file), mode), + open( + cast(can.typechecking.StringPathLike, file), mode, encoding=encoding + ), ) # for multiple inheritance @@ -74,37 +80,30 @@ def stop(self) -> None: self.file.close() -# pylint: disable=abstract-method,too-few-public-methods class MessageWriter(BaseIOHandler, can.Listener, metaclass=ABCMeta): """The base class for all writers.""" file: Optional[can.typechecking.FileLike] -# pylint: disable=abstract-method,too-few-public-methods class FileIOMessageWriter(MessageWriter, metaclass=ABCMeta): """A specialized base class for all writers with file descriptors.""" file: can.typechecking.FileLike def __init__( - self, - file: can.typechecking.AcceptedIOType, - mode: str = "wt", - *args: Any, - **kwargs: Any + self, file: can.typechecking.AcceptedIOType, mode: str = "wt", **kwargs: Any ) -> None: # Not possible with the type signature, but be verbose for user-friendliness if file is None: raise ValueError("The given file cannot be None") - super().__init__(file, mode) + super().__init__(file, mode, **kwargs) def file_size(self) -> int: """Return an estimate of the current file size in bytes.""" return self.file.tell() -# pylint: disable=too-few-public-methods class MessageReader(BaseIOHandler, Iterable[can.Message], metaclass=ABCMeta): """The base class for all readers.""" diff --git a/can/io/logger.py b/can/io/logger.py index a08cf9869..b6ea23380 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -27,7 +27,7 @@ from ..typechecking import StringPathLike, FileLike, AcceptedIOType -class Logger(MessageWriter): # pylint: disable=abstract-method +class Logger(MessageWriter): """ Logs CAN messages to a file. @@ -66,7 +66,7 @@ class Logger(MessageWriter): # pylint: disable=abstract-method @staticmethod def __new__( # type: ignore - cls: Any, filename: Optional[StringPathLike], *args: Any, **kwargs: Any + cls: Any, filename: Optional[StringPathLike], **kwargs: Any ) -> MessageWriter: """ :param filename: the filename/path of the file to write to, @@ -75,7 +75,7 @@ def __new__( # type: ignore :raises ValueError: if the filename's suffix is of an unknown file type """ if filename is None: - return Printer(*args, **kwargs) + return Printer(**kwargs) if not Logger.fetched_plugins: Logger.message_writers.update( @@ -90,19 +90,17 @@ def __new__( # type: ignore file_or_filename: AcceptedIOType = filename if suffix == ".gz": - suffix, file_or_filename = Logger.compress(filename, *args, **kwargs) + suffix, file_or_filename = Logger.compress(filename, **kwargs) try: - return Logger.message_writers[suffix](file_or_filename, *args, **kwargs) + return Logger.message_writers[suffix](file=file_or_filename, **kwargs) except KeyError: raise ValueError( f'No write support for this unknown log format "{suffix}"' ) from None @staticmethod - def compress( - filename: StringPathLike, *args: Any, **kwargs: Any - ) -> Tuple[str, FileLike]: + def compress(filename: StringPathLike, **kwargs: Any) -> Tuple[str, FileLike]: """ Return the suffix and io object of the decompressed file. File will automatically recompress upon close. @@ -154,11 +152,10 @@ class BaseRotatingLogger(Listener, BaseIOHandler, ABC): #: An integer counter to track the number of rollovers. rollover_count: int = 0 - def __init__(self, *args: Any, **kwargs: Any) -> None: + def __init__(self, **kwargs: Any) -> None: Listener.__init__(self) - BaseIOHandler.__init__(self, None) + BaseIOHandler.__init__(self, file=None) - self.writer_args = args self.writer_kwargs = kwargs # Expected to be set by the subclass @@ -184,7 +181,7 @@ def rotation_filename(self, default_name: StringPathLike) -> StringPathLike: if not callable(self.namer): return default_name - return self.namer(default_name) + return self.namer(default_name) # pylint: disable=not-callable def rotate(self, source: StringPathLike, dest: StringPathLike) -> None: """When rotating, rotate the current log. @@ -205,7 +202,7 @@ def rotate(self, source: StringPathLike, dest: StringPathLike) -> None: if os.path.exists(source): os.rename(source, dest) else: - self.rotator(source, dest) + self.rotator(source, dest) # pylint: disable=not-callable def on_message_received(self, msg: Message) -> None: """This method is called to handle the given message. @@ -234,7 +231,7 @@ def _get_new_writer(self, filename: StringPathLike) -> FileIOMessageWriter: suffix = "".join(pathlib.Path(filename).suffixes[-2:]).lower() if suffix in self._supported_formats: - logger = Logger(filename, *self.writer_args, **self.writer_kwargs) + logger = Logger(filename=filename, **self.writer_kwargs) if isinstance(logger, FileIOMessageWriter): return logger elif isinstance(logger, Printer) and logger.file is not None: @@ -323,7 +320,6 @@ def __init__( self, base_filename: StringPathLike, max_bytes: int = 0, - *args: Any, **kwargs: Any, ) -> None: """ @@ -334,7 +330,7 @@ def __init__( The size threshold at which a new log file shall be created. If set to 0, no rollover will be performed. """ - super().__init__(*args, **kwargs) + super().__init__(**kwargs) self.base_filename = os.path.abspath(base_filename) self.max_bytes = max_bytes diff --git a/can/io/player.py b/can/io/player.py index 21d1964bb..0e062ecb7 100644 --- a/can/io/player.py +++ b/can/io/player.py @@ -65,7 +65,6 @@ class LogReader(MessageReader): def __new__( # type: ignore cls: typing.Any, filename: StringPathLike, - *args: typing.Any, **kwargs: typing.Any, ) -> MessageReader: """ @@ -87,7 +86,7 @@ def __new__( # type: ignore if suffix == ".gz": suffix, file_or_filename = LogReader.decompress(filename) try: - return LogReader.message_readers[suffix](file_or_filename, *args, **kwargs) + return LogReader.message_readers[suffix](file=file_or_filename, **kwargs) except KeyError: raise ValueError( f'No read support for this unknown log format "{suffix}"' @@ -109,7 +108,7 @@ def __iter__(self) -> typing.Generator[Message, None, None]: raise NotImplementedError() -class MessageSync: # pylint: disable=too-few-public-methods +class MessageSync: """ Used to iterate over some given messages in the recorded time. """ diff --git a/can/io/printer.py b/can/io/printer.py index 6a43c63b9..01da12e84 100644 --- a/can/io/printer.py +++ b/can/io/printer.py @@ -29,7 +29,6 @@ def __init__( self, file: Optional[Union[StringPathLike, TextIO]] = None, append: bool = False, - *args: Any, **kwargs: Any ) -> None: """ diff --git a/can/io/sqlite.py b/can/io/sqlite.py index 0a4de85f2..33f5d293f 100644 --- a/can/io/sqlite.py +++ b/can/io/sqlite.py @@ -36,7 +36,6 @@ def __init__( self, file: StringPathLike, table_name: str = "messages", - *args: Any, **kwargs: Any, ) -> None: """ @@ -138,7 +137,6 @@ def __init__( self, file: StringPathLike, table_name: str = "messages", - *args: Any, **kwargs: Any, ) -> None: """ diff --git a/can/io/trc.py b/can/io/trc.py index 0a07f01c9..ec08d1af1 100644 --- a/can/io/trc.py +++ b/can/io/trc.py @@ -7,12 +7,12 @@ Version 1.1 will be implemented as it is most commonly used """ # noqa -from typing import Generator, Optional, Union, TextIO from datetime import datetime, timedelta from enum import Enum -from io import TextIOWrapper +import io import os import logging +from typing import Generator, Optional, Union, TextIO, Callable, List from ..message import Message from ..util import channel2int @@ -55,11 +55,14 @@ def __init__( if not self.file: raise ValueError("The given file cannot be None") + self._parse_cols: Callable[[List[str]], Optional[Message]] = lambda x: None + def _extract_header(self): + line = "" for line in self.file: line = line.strip() if line.startswith(";$FILEVERSION"): - logger.debug(f"TRCReader: Found file version '{line}'") + logger.debug("TRCReader: Found file version '%s'", line) try: file_version = line.split("=")[1] if file_version == "1.1": @@ -91,7 +94,7 @@ def _extract_header(self): return line - def _parse_msg_V1_0(self, cols): + def _parse_msg_V1_0(self, cols: List[str]) -> Optional[Message]: arbit_id = cols[2] if arbit_id == "FFFFFFFF": logger.info("TRCReader: Dropping bus info line") @@ -106,7 +109,7 @@ def _parse_msg_V1_0(self, cols): msg.data = bytearray([int(cols[i + 4], 16) for i in range(msg.dlc)]) return msg - def _parse_msg_V1_1(self, cols): + def _parse_msg_V1_1(self, cols: List[str]) -> Optional[Message]: arbit_id = cols[3] msg = Message() @@ -119,7 +122,7 @@ def _parse_msg_V1_1(self, cols): msg.is_rx = cols[2] == "Rx" return msg - def _parse_msg_V2_1(self, cols): + def _parse_msg_V2_1(self, cols: List[str]) -> Optional[Message]: msg = Message() msg.timestamp = float(cols[1]) / 1000 msg.arbitration_id = int(cols[4], 16) @@ -130,29 +133,29 @@ def _parse_msg_V2_1(self, cols): msg.is_rx = cols[5] == "Rx" return msg - def _parse_cols_V1_1(self, cols): + def _parse_cols_V1_1(self, cols: List[str]) -> Optional[Message]: dtype = cols[2] - if dtype == "Tx" or dtype == "Rx": + if dtype in ("Tx", "Rx"): return self._parse_msg_V1_1(cols) else: - logger.info(f"TRCReader: Unsupported type '{dtype}'") + logger.info("TRCReader: Unsupported type '%s'", dtype) return None - def _parse_cols_V2_1(self, cols): + def _parse_cols_V2_1(self, cols: List[str]) -> Optional[Message]: dtype = cols[2] if dtype == "DT": return self._parse_msg_V2_1(cols) else: - logger.info(f"TRCReader: Unsupported type '{dtype}'") + logger.info("TRCReader: Unsupported type '%s'", dtype) return None - def _parse_line(self, line): - logger.debug(f"TRCReader: Parse '{line}'") + def _parse_line(self, line: str) -> Optional[Message]: + logger.debug("TRCReader: Parse '%s'", line) try: cols = line.split() return self._parse_cols(cols) except IndexError: - logger.warning(f"TRCReader: Failed to parse message '{line}'") + logger.warning("TRCReader: Failed to parse message '%s'", line) return None def __iter__(self) -> Generator[Message, None, None]: @@ -211,81 +214,66 @@ def __init__( """ super().__init__(file, mode="w") self.channel = channel - if type(file) is str: - self.filepath = os.path.abspath(file) - elif type(file) is TextIOWrapper: - self.filepath = "Unknown" - logger.warning("TRCWriter: Text mode io can result in wrong line endings") - logger.debug( - f"TRCWriter: Text mode io line ending setting: {file.newlines}" - ) + + if isinstance(self.file, io.TextIOWrapper): + self.file.reconfigure(newline="\r\n") else: - self.filepath = "Unknown" + raise TypeError("File must be opened in text mode.") + self.filepath = os.path.abspath(self.file.name) self.header_written = False self.msgnr = 0 self.first_timestamp = None self.file_version = TRCFileVersion.V2_1 + self._msg_fmt_string = self.FORMAT_MESSAGE_V1_0 self._format_message = self._format_message_init - def _write_line(self, line: str) -> None: - self.file.write(line + "\r\n") - - def _write_lines(self, lines: list) -> None: - for line in lines: - self._write_line(line) - def _write_header_V1_0(self, start_time: timedelta) -> None: - self._write_line( - ";##########################################################################" - ) - self._write_line(f"; {self.filepath}") - self._write_line(";") - self._write_line("; Generated by python-can TRCWriter") - self._write_line(f"; Start time: {start_time}") - self._write_line("; PCAN-Net: N/A") - self._write_line(";") - self._write_line("; Columns description:") - self._write_line("; ~~~~~~~~~~~~~~~~~~~~~") - self._write_line("; +-current number in actual sample") - self._write_line("; | +time offset of message (ms)") - self._write_line("; | | +ID of message (hex)") - self._write_line("; | | | +data length code") - self._write_line("; | | | | +data bytes (hex) ...") - self._write_line("; | | | | |") - self._write_line(";----+- ---+--- ----+--- + -+ -- -- ...") + lines = [ + ";##########################################################################", + f"; {self.filepath}", + ";", + "; Generated by python-can TRCWriter", + f"; Start time: {start_time}", + "; PCAN-Net: N/A", + ";", + "; Columns description:", + "; ~~~~~~~~~~~~~~~~~~~~~", + "; +-current number in actual sample", + "; | +time offset of message (ms", + "; | | +ID of message (hex", + "; | | | +data length code", + "; | | | | +data bytes (hex ...", + "; | | | | |", + ";----+- ---+--- ----+--- + -+ -- -- ...", + ] + self.file.writelines(line + "\n" for line in lines) def _write_header_V2_1(self, header_time: timedelta, start_time: datetime) -> None: milliseconds = int( (header_time.seconds * 1000) + (header_time.microseconds / 1000) ) - - self._write_line(";$FILEVERSION=2.1") - self._write_line(f";$STARTTIME={header_time.days}.{milliseconds}") - self._write_line(";$COLUMNS=N,O,T,B,I,d,R,L,D") - self._write_line(";") - self._write_line(f"; {self.filepath}") - self._write_line(";") - self._write_line(f"; Start time: {start_time}") - self._write_line("; Generated by python-can TRCWriter") - self._write_line( - ";-------------------------------------------------------------------------------" - ) - self._write_line("; Bus Name Connection Protocol") - self._write_line("; N/A N/A N/A N/A") - self._write_line( - ";-------------------------------------------------------------------------------" - ) - self._write_lines( - [ - "; Message Time Type ID Rx/Tx", - "; Number Offset | Bus [hex] | Reserved", - "; | [ms] | | | | | Data Length Code", - "; | | | | | | | | Data [hex] ...", - "; | | | | | | | | |", - ";---+-- ------+------ +- +- --+----- +- +- +--- +- -- -- -- -- -- -- --", - ] - ) + lines = [ + ";$FILEVERSION=2.1", + f";$STARTTIME={header_time.days}.{milliseconds}", + ";$COLUMNS=N,O,T,B,I,d,R,L,D", + ";", + f"; {self.filepath}", + ";", + f"; Start time: {start_time}", + "; Generated by python-can TRCWriter", + ";-------------------------------------------------------------------------------", + "; Bus Name Connection Protocol", + "; N/A N/A N/A N/A", + ";-------------------------------------------------------------------------------", + "; Message Time Type ID Rx/Tx", + "; Number Offset | Bus [hex] | Reserved", + "; | [ms] | | | | | Data Length Code", + "; | | | | | | | | Data [hex] ...", + "; | | | | | | | | |", + ";---+-- ------+------ +- +- --+----- +- +- +--- +- -- -- -- -- -- -- --", + ] + self.file.writelines(line + "\n" for line in lines) def _format_message_by_format(self, msg, channel): if msg.is_extended_id: @@ -316,7 +304,7 @@ def _format_message_init(self, msg, channel): else: raise NotImplementedError("File format is not supported") - return self._format_message(msg, channel) + return self._format_message_by_format(msg, channel) def write_header(self, timestamp: float) -> None: # write start of file header @@ -336,7 +324,7 @@ def log_event(self, message: str, timestamp: float) -> None: if not self.header_written: self.write_header(timestamp) - self._write_line(message) + self.file.write(message + "\n") def on_message_received(self, msg: Message) -> None: if self.first_timestamp is None: diff --git a/can/listener.py b/can/listener.py index 6c9cdf0be..e68d813d1 100644 --- a/can/listener.py +++ b/can/listener.py @@ -7,7 +7,7 @@ import asyncio from abc import ABCMeta, abstractmethod from queue import SimpleQueue, Empty -from typing import Any, AsyncIterator, Awaitable, Optional +from typing import Any, AsyncIterator, Optional from can.message import Message from can.bus import BusABC @@ -126,7 +126,9 @@ def stop(self) -> None: self.is_stopped = True -class AsyncBufferedReader(Listener): # pylint: disable=abstract-method +class AsyncBufferedReader( + Listener, AsyncIterator[Message] +): # pylint: disable=abstract-method """A message buffer for use with :mod:`asyncio`. See :ref:`asyncio` for how to use with :class:`can.Notifier`. @@ -174,5 +176,5 @@ async def get_message(self) -> Message: def __aiter__(self) -> AsyncIterator[Message]: return self - def __anext__(self) -> Awaitable[Message]: - return self.buffer.get() + async def __anext__(self) -> Message: + return await self.buffer.get() diff --git a/can/logconvert.py b/can/logconvert.py index 730e82304..d89155758 100644 --- a/can/logconvert.py +++ b/can/logconvert.py @@ -56,7 +56,7 @@ def main(): with logger: try: - for m in reader: # pylint: disable=not-an-iterable + for m in reader: logger(m) except KeyboardInterrupt: sys.exit(1) diff --git a/can/logger.py b/can/logger.py index f13b78bfc..55e67b27e 100644 --- a/can/logger.py +++ b/can/logger.py @@ -58,7 +58,7 @@ def _create_base_argument_parser(parser: argparse.ArgumentParser) -> None: def _append_filter_argument( parser: Union[ argparse.ArgumentParser, - argparse._ArgumentGroup, # pylint: disable=protected-access + argparse._ArgumentGroup, ], *args: str, **kwargs: Any, diff --git a/can/message.py b/can/message.py index 8e0c4deee..48933b2da 100644 --- a/can/message.py +++ b/can/message.py @@ -228,9 +228,7 @@ def __deepcopy__(self, memo: dict) -> "Message": error_state_indicator=self.error_state_indicator, ) - def _check( - self, - ) -> None: # pylint: disable=too-many-branches; it's still simple code + def _check(self) -> None: """Checks if the message parameters are valid. Assumes that the attribute types are already correct. diff --git a/requirements-lint.txt b/requirements-lint.txt index 2952103c3..28bcf2aa2 100644 --- a/requirements-lint.txt +++ b/requirements-lint.txt @@ -1,4 +1,4 @@ -pylint==2.12.2 +pylint==2.15.9 black~=22.10.0 mypy==0.991 mypy-extensions==0.4.3 From 7013796ad8341e627ea0f46cada2a82506be4562 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Mon, 2 Jan 2023 00:40:55 +0100 Subject: [PATCH 2/7] add `ignore_config` parameter to can.Bus --- can/interface.py | 45 +++++++++++++++++++++++++-------------------- test/test_bus.py | 14 ++++++++++++++ 2 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 test/test_bus.py diff --git a/can/interface.py b/can/interface.py index 76d0dd1a5..83ead4021 100644 --- a/can/interface.py +++ b/can/interface.py @@ -8,8 +8,8 @@ import logging from typing import Any, cast, Iterable, Type, Optional, Union, List +from . import util from .bus import BusABC -from .util import load_config, deprecated_args_alias from .interfaces import BACKENDS from .exceptions import CanInterfaceNotImplementedError from .typechecking import AutoDetectedConfig, Channel @@ -61,6 +61,13 @@ class Bus(BusABC): # pylint: disable=abstract-method Instantiates a CAN Bus of the given ``interface``, falls back to reading a configuration file from default locations. + .. note:: + Please note that while the arguments provided to this class take precedence + over any existing values from configuration, it is possible that other parameters + from the configuration may be added to the bus instantiation. + This could potentially have unintended consequences. To prevent this, + you may use the *ignore_config* parameter to ignore any existing configurations. + :param channel: Channel identification. Expected type is backend dependent. Set to ``None`` to let it be resolved automatically from the default @@ -71,8 +78,13 @@ class Bus(BusABC): # pylint: disable=abstract-method Set to ``None`` to let it be resolved automatically from the default :ref:`configuration`. - :param args: - ``interface`` specific positional arguments. + :param context: + Extra 'context', that is passed to config sources. + This can be used to select a section other than 'default' in the configuration file. + + :param ignore_config: + If ``True``, only the given arguments will be used for the bus instantiation. Existing + configuration sources will be ignored. :param kwargs: ``interface`` specific keyword arguments. @@ -88,12 +100,13 @@ class Bus(BusABC): # pylint: disable=abstract-method """ @staticmethod - @deprecated_args_alias(bustype="interface") # Deprecated since python-can 4.2 - def __new__( # type: ignore # pylint: disable=keyword-arg-before-vararg + @util.deprecated_args_alias(bustype="interface") # Deprecated since python-can 4.2 + def __new__( # type: ignore cls: Any, channel: Optional[Channel] = None, interface: Optional[str] = None, - *args: Any, + context: Optional[str] = None, + ignore_config: bool = False, **kwargs: Any, ) -> BusABC: # figure out the rest of the configuration; this might raise an error @@ -101,12 +114,9 @@ def __new__( # type: ignore # pylint: disable=keyword-arg-before-vararg kwargs["interface"] = interface if channel is not None: kwargs["channel"] = channel - if "context" in kwargs: - context = kwargs["context"] - del kwargs["context"] - else: - context = None - kwargs = load_config(config=kwargs, context=context) + + if not ignore_config: + kwargs = util.load_config(config=kwargs, context=context) # resolve the bus class to use for that interface cls = _get_class_for_interface(kwargs["interface"]) @@ -115,17 +125,12 @@ def __new__( # type: ignore # pylint: disable=keyword-arg-before-vararg del kwargs["interface"] # make sure the bus can handle this config format - if "channel" not in kwargs: - raise ValueError("'channel' argument missing") - else: - channel = kwargs["channel"] - del kwargs["channel"] - + channel = kwargs.pop("channel", channel) if channel is None: # Use the default channel for the backend - bus = cls(*args, **kwargs) + bus = cls(**kwargs) else: - bus = cls(channel, *args, **kwargs) + bus = cls(channel, **kwargs) return cast(BusABC, bus) diff --git a/test/test_bus.py b/test/test_bus.py new file mode 100644 index 000000000..e11d829d3 --- /dev/null +++ b/test/test_bus.py @@ -0,0 +1,14 @@ +from unittest.mock import patch + +import can + + +def test_bus_ignore_config(): + with patch.object( + target=can.util, attribute="load_config", side_effect=can.util.load_config + ): + _ = can.Bus(interface="virtual", ignore_config=True) + assert not can.util.load_config.called + + _ = can.Bus(interface="virtual") + assert can.util.load_config.called From 2f50efdc03960d5247dff5c4be957d26458912bb Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Mon, 2 Jan 2023 12:38:17 +0100 Subject: [PATCH 3/7] rename context -> config_context --- can/interface.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/can/interface.py b/can/interface.py index 83ead4021..2bc8820f5 100644 --- a/can/interface.py +++ b/can/interface.py @@ -78,7 +78,7 @@ class Bus(BusABC): # pylint: disable=abstract-method Set to ``None`` to let it be resolved automatically from the default :ref:`configuration`. - :param context: + :param config_context: Extra 'context', that is passed to config sources. This can be used to select a section other than 'default' in the configuration file. @@ -100,12 +100,14 @@ class Bus(BusABC): # pylint: disable=abstract-method """ @staticmethod - @util.deprecated_args_alias(bustype="interface") # Deprecated since python-can 4.2 + @util.deprecated_args_alias( # Deprecated since python-can 4.2 + bustype="interface", context="config_context" + ) def __new__( # type: ignore cls: Any, channel: Optional[Channel] = None, interface: Optional[str] = None, - context: Optional[str] = None, + config_context: Optional[str] = None, ignore_config: bool = False, **kwargs: Any, ) -> BusABC: @@ -116,7 +118,7 @@ def __new__( # type: ignore kwargs["channel"] = channel if not ignore_config: - kwargs = util.load_config(config=kwargs, context=context) + kwargs = util.load_config(config=kwargs, context=config_context) # resolve the bus class to use for that interface cls = _get_class_for_interface(kwargs["interface"]) From 1278a0f1accde270b7fdd5d1ff9127456820a794 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Wed, 4 Jan 2023 20:27:30 +0100 Subject: [PATCH 4/7] Run doctest in CI (#1476) --- .github/workflows/ci.yml | 5 ++++- can/bus.py | 10 ++++++---- can/exceptions.py | 24 +++++++++++++++--------- can/interfaces/kvaser/canlib.py | 15 ++++++++++++--- doc/interfaces/ixxat.rst | 19 ++++++++++++++----- doc/message.rst | 20 ++++++++++---------- 6 files changed, 61 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6a5c4253..c610c6a76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,7 +129,10 @@ jobs: pip install -r doc/doc-requirements.txt - name: Build documentation run: | - python -m sphinx -Wan doc build + python -m sphinx -Wan --keep-going doc build + - name: Run doctest + run: | + python -m sphinx -b doctest -W --keep-going doc build - uses: actions/upload-artifact@v3 with: name: sphinx-out diff --git a/can/bus.py b/can/bus.py index d0192fc2d..f29b8ea6e 100644 --- a/can/bus.py +++ b/can/bus.py @@ -314,8 +314,10 @@ def stop_all_periodic_tasks(self, remove_tasks: bool = True) -> None: def __iter__(self) -> Iterator[Message]: """Allow iteration on messages as they are received. - >>> for msg in bus: - ... print(msg) + .. code-block:: python + + for msg in bus: + print(msg) :yields: @@ -352,9 +354,9 @@ def set_filters( :param filters: A iterable of dictionaries each containing a "can_id", - a "can_mask", and an optional "extended" key. + a "can_mask", and an optional "extended" key:: - >>> [{"can_id": 0x11, "can_mask": 0x21, "extended": False}] + [{"can_id": 0x11, "can_mask": 0x21, "extended": False}] A filter matches, when `` & can_mask == can_id & can_mask``. diff --git a/can/exceptions.py b/can/exceptions.py index 7496c6c0e..57130082a 100644 --- a/can/exceptions.py +++ b/can/exceptions.py @@ -32,15 +32,21 @@ class CanError(Exception): If specified, the error code is automatically appended to the message: - >>> # With an error code (it also works with a specific error): - >>> error = CanOperationError(message="Failed to do the thing", error_code=42) - >>> str(error) - 'Failed to do the thing [Error Code 42]' - >>> - >>> # Missing the error code: - >>> plain_error = CanError(message="Something went wrong ...") - >>> str(plain_error) - 'Something went wrong ...' + .. testsetup:: canerror + + from can import CanError, CanOperationError + + .. doctest:: canerror + + >>> # With an error code (it also works with a specific error): + >>> error = CanOperationError(message="Failed to do the thing", error_code=42) + >>> str(error) + 'Failed to do the thing [Error Code 42]' + >>> + >>> # Missing the error code: + >>> plain_error = CanError(message="Something went wrong ...") + >>> str(plain_error) + 'Something went wrong ...' :param error_code: An optional error code to narrow down the cause of the fault diff --git a/can/interfaces/kvaser/canlib.py b/can/interfaces/kvaser/canlib.py index a8bb7bac7..2bbf8f0bf 100644 --- a/can/interfaces/kvaser/canlib.py +++ b/can/interfaces/kvaser/canlib.py @@ -662,9 +662,18 @@ def get_stats(self) -> structures.BusStatistics: Use like so: - >>> stats = bus.get_stats() - >>> print(stats) - std_data: 0, std_remote: 0, ext_data: 0, ext_remote: 0, err_frame: 0, bus_load: 0.0%, overruns: 0 + .. testsetup:: kvaser + + from unittest.mock import Mock + from can.interfaces.kvaser.structures import BusStatistics + bus = Mock() + bus.get_stats = Mock(side_effect=lambda: BusStatistics()) + + .. doctest:: kvaser + + >>> stats = bus.get_stats() + >>> print(stats) + std_data: 0, std_remote: 0, ext_data: 0, ext_remote: 0, err_frame: 0, bus_load: 0.0%, overruns: 0 :returns: bus statistics. """ diff --git a/doc/interfaces/ixxat.rst b/doc/interfaces/ixxat.rst index 61df70638..f73a01036 100644 --- a/doc/interfaces/ixxat.rst +++ b/doc/interfaces/ixxat.rst @@ -59,11 +59,20 @@ List available devices In case you have connected multiple IXXAT devices, you have to select them by using their unique hardware id. To get a list of all connected IXXAT you can use the function ``get_ixxat_hwids()`` as demonstrated below: - >>> from can.interfaces.ixxat import get_ixxat_hwids - >>> for hwid in get_ixxat_hwids(): - ... print("Found IXXAT with hardware id '%s'." % hwid) - Found IXXAT with hardware id 'HW441489'. - Found IXXAT with hardware id 'HW107422'. + .. testsetup:: ixxat + + from unittest.mock import Mock + import can.interfaces.ixxat + assert hasattr(can.interfaces.ixxat, "get_ixxat_hwids") + can.interfaces.ixxat.get_ixxat_hwids = Mock(side_effect=lambda: ['HW441489', 'HW107422']) + + .. doctest:: ixxat + + >>> from can.interfaces.ixxat import get_ixxat_hwids + >>> for hwid in get_ixxat_hwids(): + ... print("Found IXXAT with hardware id '%s'." % hwid) + Found IXXAT with hardware id 'HW441489'. + Found IXXAT with hardware id 'HW107422'. Bus diff --git a/doc/message.rst b/doc/message.rst index 78ccc0b50..d47473e17 100644 --- a/doc/message.rst +++ b/doc/message.rst @@ -15,7 +15,7 @@ Message >>> test.dlc 5 >>> print(test) - Timestamp: 0.000000 ID: 00000000 010 DLC: 5 01 02 03 04 05 + Timestamp: 0.000000 ID: 00000000 X Rx DL: 5 01 02 03 04 05 The :attr:`~can.Message.arbitration_id` field in a CAN message may be either @@ -44,7 +44,7 @@ Message 2\ :sup:`29` - 1 for 29-bit identifiers). >>> print(Message(is_extended_id=False, arbitration_id=100)) - Timestamp: 0.000000 ID: 0064 S DLC: 0 + Timestamp: 0.000000 ID: 0064 S Rx DL: 0 .. attribute:: data @@ -56,7 +56,7 @@ Message >>> example_data = bytearray([1, 2, 3]) >>> print(Message(data=example_data)) - Timestamp: 0.000000 ID: 00000000 X DLC: 3 01 02 03 + Timestamp: 0.000000 ID: 00000000 X Rx DL: 3 01 02 03 A :class:`~can.Message` can also be created with bytes, or lists of ints: @@ -106,9 +106,9 @@ Message Previously this was exposed as `id_type`. >>> print(Message(is_extended_id=False)) - Timestamp: 0.000000 ID: 0000 S DLC: 0 + Timestamp: 0.000000 ID: 0000 S Rx DL: 0 >>> print(Message(is_extended_id=True)) - Timestamp: 0.000000 ID: 00000000 X DLC: 0 + Timestamp: 0.000000 ID: 00000000 X Rx DL: 0 .. note:: @@ -124,7 +124,7 @@ Message This boolean parameter indicates if the message is an error frame or not. >>> print(Message(is_error_frame=True)) - Timestamp: 0.000000 ID: 00000000 X E DLC: 0 + Timestamp: 0.000000 ID: 00000000 X Rx E DL: 0 .. attribute:: is_remote_frame @@ -135,7 +135,7 @@ Message modifies the bit in the CAN message's flags field indicating this. >>> print(Message(is_remote_frame=True)) - Timestamp: 0.000000 ID: 00000000 X R DLC: 0 + Timestamp: 0.000000 ID: 00000000 X Rx R DL: 0 .. attribute:: is_fd @@ -174,17 +174,17 @@ Message >>> from can import Message >>> test = Message() >>> print(test) - Timestamp: 0.000000 ID: 00000000 X DLC: 0 + Timestamp: 0.000000 ID: 00000000 X Rx DL: 0 >>> test2 = Message(data=[1, 2, 3, 4, 5]) >>> print(test2) - Timestamp: 0.000000 ID: 00000000 X DLC: 5 01 02 03 04 05 + Timestamp: 0.000000 ID: 00000000 X Rx DL: 5 01 02 03 04 05 The fields in the printed message are (in order): - timestamp, - arbitration ID, - flags, - - dlc, + - data length (DL), - and data. From 5e5b950228302c21529ca1bb6f234f9bd7e0a372 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Thu, 5 Jan 2023 23:36:03 +0100 Subject: [PATCH 5/7] add deprecation period to deprecated_args_alias (#1477) --- can/interface.py | 7 +- can/interfaces/ixxat/canlib_vcinpl.py | 2 + can/interfaces/ixxat/canlib_vcinpl2.py | 2 + can/interfaces/vector/canlib.py | 6 +- can/util.py | 53 +++++++++++--- test/test_util.py | 95 +++++++++++++++++++++++--- 6 files changed, 144 insertions(+), 21 deletions(-) diff --git a/can/interface.py b/can/interface.py index 2bc8820f5..04fc84ae9 100644 --- a/can/interface.py +++ b/can/interface.py @@ -100,8 +100,11 @@ class Bus(BusABC): # pylint: disable=abstract-method """ @staticmethod - @util.deprecated_args_alias( # Deprecated since python-can 4.2 - bustype="interface", context="config_context" + @util.deprecated_args_alias( + deprecation_start="4.2.0", + deprecation_end="5.0.0", + bustype="interface", + context="config_context", ) def __new__( # type: ignore cls: Any, diff --git a/can/interfaces/ixxat/canlib_vcinpl.py b/can/interfaces/ixxat/canlib_vcinpl.py index d74da2539..8304a6dd7 100644 --- a/can/interfaces/ixxat/canlib_vcinpl.py +++ b/can/interfaces/ixxat/canlib_vcinpl.py @@ -417,6 +417,8 @@ class IXXATBus(BusABC): } @deprecated_args_alias( + deprecation_start="4.0.0", + deprecation_end="5.0.0", UniqueHardwareId="unique_hardware_id", rxFifoSize="rx_fifo_size", txFifoSize="tx_fifo_size", diff --git a/can/interfaces/ixxat/canlib_vcinpl2.py b/can/interfaces/ixxat/canlib_vcinpl2.py index 2e3125e9b..b8ed916dc 100644 --- a/can/interfaces/ixxat/canlib_vcinpl2.py +++ b/can/interfaces/ixxat/canlib_vcinpl2.py @@ -417,6 +417,8 @@ class IXXATBus(BusABC): """ @deprecated_args_alias( + deprecation_start="4.0.0", + deprecation_end="5.0.0", UniqueHardwareId="unique_hardware_id", rxFifoSize="rx_fifo_size", txFifoSize="tx_fifo_size", diff --git a/can/interfaces/vector/canlib.py b/can/interfaces/vector/canlib.py index ff72d262a..7c1ffac64 100644 --- a/can/interfaces/vector/canlib.py +++ b/can/interfaces/vector/canlib.py @@ -75,7 +75,11 @@ class VectorBus(BusABC): tseg2Dbr="tseg2_dbr", ) - @deprecated_args_alias(**deprecated_args) + @deprecated_args_alias( + deprecation_start="4.0.0", + deprecation_end="5.0.0", + **deprecated_args, + ) def __init__( self, channel: Union[int, Sequence[int], str], diff --git a/can/util.py b/can/util.py index aa4a28d15..e4a45f2d5 100644 --- a/can/util.py +++ b/can/util.py @@ -295,26 +295,47 @@ def channel2int(channel: Optional[typechecking.Channel]) -> Optional[int]: return None -def deprecated_args_alias(**aliases): +def deprecated_args_alias( # type: ignore + deprecation_start: str, deprecation_end: Optional[str] = None, **aliases +): """Allows to rename/deprecate a function kwarg(s) and optionally have the deprecated kwarg(s) set as alias(es) Example:: - @deprecated_args_alias(oldArg="new_arg", anotherOldArg="another_new_arg") + @deprecated_args_alias("1.2.0", oldArg="new_arg", anotherOldArg="another_new_arg") def library_function(new_arg, another_new_arg): pass - @deprecated_args_alias(oldArg="new_arg", obsoleteOldArg=None) + @deprecated_args_alias( + deprecation_start="1.2.0", + deprecation_end="3.0.0", + oldArg="new_arg", + obsoleteOldArg=None, + ) def library_function(new_arg): pass + :param deprecation_start: + The *python-can* version, that introduced the :class:`DeprecationWarning`. + :param deprecation_end: + The *python-can* version, that marks the end of the deprecation period. + :param aliases: + keyword arguments, that map the deprecated argument names + to the new argument names or ``None``. + """ def deco(f): @functools.wraps(f) def wrapper(*args, **kwargs): - _rename_kwargs(f.__name__, kwargs, aliases) + _rename_kwargs( + func_name=f.__name__, + start=deprecation_start, + end=deprecation_end, + kwargs=kwargs, + aliases=aliases, + ) return f(*args, **kwargs) return wrapper @@ -323,21 +344,35 @@ def wrapper(*args, **kwargs): def _rename_kwargs( - func_name: str, kwargs: Dict[str, str], aliases: Dict[str, str] + func_name: str, + start: str, + end: Optional[str], + kwargs: Dict[str, str], + aliases: Dict[str, str], ) -> None: """Helper function for `deprecated_args_alias`""" for alias, new in aliases.items(): if alias in kwargs: + deprecation_notice = ( + f"The '{alias}' argument is deprecated since python-can v{start}" + ) + if end: + deprecation_notice += ( + f", and scheduled for removal in python-can v{end}" + ) + deprecation_notice += "." + value = kwargs.pop(alias) if new is not None: - warnings.warn(f"{alias} is deprecated; use {new}", DeprecationWarning) + deprecation_notice += f" Use '{new}' instead." + if new in kwargs: raise TypeError( - f"{func_name} received both {alias} (deprecated) and {new}" + f"{func_name} received both '{alias}' (deprecated) and '{new}'." ) kwargs[new] = value - else: - warnings.warn(f"{alias} is deprecated", DeprecationWarning) + + warnings.warn(deprecation_notice, DeprecationWarning) def time_perfcounter_correlation() -> Tuple[float, float]: diff --git a/test/test_util.py b/test/test_util.py index 7048d6151..70941f23f 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -3,17 +3,26 @@ import unittest import warnings -from can.util import _create_bus_config, _rename_kwargs, channel2int +import pytest + +from can.util import ( + _create_bus_config, + _rename_kwargs, + channel2int, + deprecated_args_alias, +) class RenameKwargsTest(unittest.TestCase): expected_kwargs = dict(a=1, b=2, c=3, d=4) - def _test(self, kwargs, aliases): + def _test(self, start: str, end: str, kwargs, aliases): # Test that we do get the DeprecationWarning when called with deprecated kwargs - with self.assertWarnsRegex(DeprecationWarning, "is deprecated"): - _rename_kwargs("unit_test", kwargs, aliases) + with self.assertWarnsRegex( + DeprecationWarning, "is deprecated.*?" + start + ".*?" + end + ): + _rename_kwargs("unit_test", start, end, kwargs, aliases) # Test that the aliases contains the deprecated values and # the obsolete kwargs have been removed @@ -25,30 +34,98 @@ def _test(self, kwargs, aliases): # Cause all warnings to always be triggered. warnings.simplefilter("error", DeprecationWarning) try: - _rename_kwargs("unit_test", kwargs, aliases) + _rename_kwargs("unit_test", start, end, kwargs, aliases) finally: warnings.resetwarnings() def test_rename(self): kwargs = dict(old_a=1, old_b=2, c=3, d=4) aliases = {"old_a": "a", "old_b": "b"} - self._test(kwargs, aliases) + self._test("1.0", "2.0", kwargs, aliases) def test_obsolete(self): kwargs = dict(a=1, b=2, c=3, d=4, z=10) aliases = {"z": None} - self._test(kwargs, aliases) + self._test("1.0", "2.0", kwargs, aliases) def test_rename_and_obsolete(self): kwargs = dict(old_a=1, old_b=2, c=3, d=4, z=10) aliases = {"old_a": "a", "old_b": "b", "z": None} - self._test(kwargs, aliases) + self._test("1.0", "2.0", kwargs, aliases) def test_with_new_and_alias_present(self): kwargs = dict(old_a=1, a=1, b=2, c=3, d=4, z=10) aliases = {"old_a": "a", "old_b": "b", "z": None} with self.assertRaises(TypeError): - self._test(kwargs, aliases) + self._test("1.0", "2.0", kwargs, aliases) + + +class DeprecatedArgsAliasTest(unittest.TestCase): + def test_decorator(self): + @deprecated_args_alias("1.0.0", old_a="a") + def _test_func1(a): + pass + + with pytest.warns(DeprecationWarning) as record: + _test_func1(old_a=1) + assert len(record) == 1 + assert ( + record[0].message.args[0] + == "The 'old_a' argument is deprecated since python-can v1.0.0. Use 'a' instead." + ) + + @deprecated_args_alias("1.6.0", "3.4.0", old_a="a", old_b=None) + def _test_func2(a): + pass + + with pytest.warns(DeprecationWarning) as record: + _test_func2(old_a=1, old_b=2) + assert len(record) == 2 + assert record[0].message.args[0] == ( + "The 'old_a' argument is deprecated since python-can v1.6.0, and scheduled for " + "removal in python-can v3.4.0. Use 'a' instead." + ) + assert record[1].message.args[0] == ( + "The 'old_b' argument is deprecated since python-can v1.6.0, and scheduled for " + "removal in python-can v3.4.0." + ) + + @deprecated_args_alias("1.6.0", "3.4.0", old_a="a") + @deprecated_args_alias("2.0.0", "4.0.0", old_b=None) + def _test_func3(a): + pass + + with pytest.warns(DeprecationWarning) as record: + _test_func3(old_a=1, old_b=2) + assert len(record) == 2 + assert record[0].message.args[0] == ( + "The 'old_a' argument is deprecated since python-can v1.6.0, and scheduled " + "for removal in python-can v3.4.0. Use 'a' instead." + ) + assert record[1].message.args[0] == ( + "The 'old_b' argument is deprecated since python-can v2.0.0, and scheduled " + "for removal in python-can v4.0.0." + ) + + with pytest.warns(DeprecationWarning) as record: + _test_func3(old_a=1) + assert len(record) == 1 + assert record[0].message.args[0] == ( + "The 'old_a' argument is deprecated since python-can v1.6.0, and scheduled " + "for removal in python-can v3.4.0. Use 'a' instead." + ) + + with pytest.warns(DeprecationWarning) as record: + _test_func3(a=1, old_b=2) + assert len(record) == 1 + assert record[0].message.args[0] == ( + "The 'old_b' argument is deprecated since python-can v2.0.0, and scheduled " + "for removal in python-can v4.0.0." + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + _test_func3(a=1) class TestBusConfig(unittest.TestCase): From 15b8af00af5869f7ed6b48f3e022576caab169ab Mon Sep 17 00:00:00 2001 From: Lukas Magel Date: Sat, 7 Jan 2023 07:01:47 +0100 Subject: [PATCH 6/7] Use ip link JSON output for SocketCAN find_available_interfaces (#1478) * Update find_available_interfaces to use ip link JSON output * Add unit test with known JSON output for find_available_interfaces * Add except clause for JSON decoding * Fix pylint complaints --- can/interfaces/socketcan/utils.py | 49 ++++++++++++++-------------- test/test_socketcan_helpers.py | 54 +++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 34 deletions(-) diff --git a/can/interfaces/socketcan/utils.py b/can/interfaces/socketcan/utils.py index 25f04617f..7a8538135 100644 --- a/can/interfaces/socketcan/utils.py +++ b/can/interfaces/socketcan/utils.py @@ -3,15 +3,15 @@ """ import errno +import json import logging import os -import re import struct import subprocess -from typing import cast, Iterable, Optional +from typing import cast, Optional, List -from can.interfaces.socketcan.constants import CAN_EFF_FLAG from can import typechecking +from can.interfaces.socketcan.constants import CAN_EFF_FLAG log = logging.getLogger(__name__) @@ -38,35 +38,36 @@ def pack_filters(can_filters: Optional[typechecking.CanFilters] = None) -> bytes return struct.pack(can_filter_fmt, *filter_data) -_PATTERN_CAN_INTERFACE = re.compile(r"(sl|v|vx)?can\d+") - +def find_available_interfaces() -> List[str]: + """Returns the names of all open can/vcan interfaces -def find_available_interfaces() -> Iterable[str]: - """Returns the names of all open can/vcan interfaces using - the ``ip link list`` command. If the lookup fails, an error + The function calls the ``ip link list`` command. If the lookup fails, an error is logged to the console and an empty list is returned. + + :return: The list of available and active CAN interfaces or an empty list of the command failed """ try: - # adding "type vcan" would exclude physical can devices - command = ["ip", "-o", "link", "list", "up"] - output = subprocess.check_output(command, text=True) - - except Exception as e: # pylint: disable=broad-except + command = ["ip", "-json", "link", "list", "up"] + output_str = subprocess.check_output(command, text=True) + except Exception: # pylint: disable=broad-except # subprocess.CalledProcessError is too specific - log.error("failed to fetch opened can devices: %s", e) + log.exception("failed to fetch opened can devices from ip link") return [] - else: - # log.debug("find_available_interfaces(): output=\n%s", output) - # output contains some lines like "1: vcan42: ..." - # extract the "vcan42" of each line - interfaces = [line.split(": ", 3)[1] for line in output.splitlines()] - log.debug( - "find_available_interfaces(): detected these interfaces (before filtering): %s", - interfaces, - ) - return filter(_PATTERN_CAN_INTERFACE.match, interfaces) + try: + output_json = json.loads(output_str) + except json.JSONDecodeError: + log.exception("Failed to parse ip link JSON output: %s", output_str) + return [] + + log.debug( + "find_available_interfaces(): detected these interfaces (before filtering): %s", + output_json, + ) + + interfaces = [i["ifname"] for i in output_json if i["link_type"] == "can"] + return interfaces def error_code_to_str(code: Optional[int]) -> str: diff --git a/test/test_socketcan_helpers.py b/test/test_socketcan_helpers.py index ad53836f2..29ceb11c0 100644 --- a/test/test_socketcan_helpers.py +++ b/test/test_socketcan_helpers.py @@ -4,7 +4,12 @@ Tests helpers in `can.interfaces.socketcan.socketcan_common`. """ +import gzip +from base64 import b64decode import unittest +from unittest import mock + +from subprocess import CalledProcessError from can.interfaces.socketcan.utils import find_available_interfaces, error_code_to_str @@ -26,17 +31,46 @@ def test_error_code_to_str(self): string = error_code_to_str(error_code) self.assertTrue(string) # not None or empty - @unittest.skipUnless(IS_LINUX, "socketcan is only available on Linux") + @unittest.skipUnless( + TEST_INTERFACE_SOCKETCAN, "socketcan is only available on Linux" + ) def test_find_available_interfaces(self): - result = list(find_available_interfaces()) - self.assertGreaterEqual(len(result), 0) - for entry in result: - self.assertRegex(entry, r"(sl|v|vx)?can\d+") - if TEST_INTERFACE_SOCKETCAN: - self.assertGreaterEqual(len(result), 3) - self.assertIn("vcan0", result) - self.assertIn("vxcan0", result) - self.assertIn("slcan0", result) + result = find_available_interfaces() + + self.assertGreaterEqual(len(result), 3) + self.assertIn("vcan0", result) + self.assertIn("vxcan0", result) + self.assertIn("slcan0", result) + + def test_find_available_interfaces_w_patch(self): + # Contains lo, eth0, wlan0, vcan0, mycustomCan123 + ip_output_gz_b64 = ( + "H4sIAAAAAAAAA+2UzW+CMBjG7/wVhrNL+BC29IboEqNSwzQejDEViiMC5aNsmmX/+wpZTGUwDAcP" + "y5qmh+d5++bN80u7EXpsfZRnsUTf8yMXn0TQk/u8GqEQM1EMiMjpXoAOGZM3F6mUZxAuhoY55UpL" + "fbWoKjO4Hts7pl/kLdc+pDlrrmuaqnNq4vqZU8wSkSTHOeYHIjFOM4poOevKmlpwbfF+4EfHkLil" + "PRo/G6vZkrcPKcnjwnOxh/KA8h49JQGOimAkSaq03NFz/B0PiffIOfIXkeumOCtiEiUJXG++bp8S" + "5Dooo/WVZeFnvxmYUgsM01fpBmQWfDAN256M7SqioQ2NkWm8LKvGnIU3qTN+xylrV/FdaHrJzmFk" + "gkacozuzZMnhtAGkLANFAaoKBgOgaUDXG0F6Hrje7SDVWpDvAYpuIdmJV4dn2cSx9VUuGiFCe25Y" + "fwTi4KmW4ptzG0ULGvYPLN1APSqdMN3/82TRtOeqSbW5hmcnzygJTRTJivofcEvAgrAVvgD8aLkv" + "/AcAAA==" + ) + ip_output = gzip.decompress(b64decode(ip_output_gz_b64)).decode("ascii") + + with mock.patch("subprocess.check_output") as check_output: + check_output.return_value = ip_output + ifs = find_available_interfaces() + + self.assertEqual(["vcan0", "mycustomCan123"], ifs) + + def test_find_available_interfaces_exception(self): + with mock.patch("subprocess.check_output") as check_output: + check_output.return_value = "

Not JSON

" + result = find_available_interfaces() + self.assertEqual([], result) + + check_output.side_effect = Exception("Something went wrong :-/") + result = find_available_interfaces() + self.assertEqual([], result) if __name__ == "__main__": From c4c396b9a2441606a40c21ab0dedb54278e70149 Mon Sep 17 00:00:00 2001 From: pierreluctg Date: Wed, 11 Jan 2023 10:33:45 -0500 Subject: [PATCH 7/7] Avoid using root logger in usb2can (#1483) Replace the logger used to raise import error in usb2can serial selector --- can/interfaces/usb2can/serial_selector.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/can/interfaces/usb2can/serial_selector.py b/can/interfaces/usb2can/serial_selector.py index c6b9053d6..22a95ae7c 100644 --- a/can/interfaces/usb2can/serial_selector.py +++ b/can/interfaces/usb2can/serial_selector.py @@ -4,10 +4,12 @@ import logging from typing import List +log = logging.getLogger("can.usb2can") + try: import win32com.client except ImportError: - logging.warning("win32com.client module required for usb2can") + log.warning("win32com.client module required for usb2can") raise