From d71ae66ff6bf99ab38c7329bbb44bcedb63f528d Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Tue, 27 Dec 2022 01:30:13 +0100 Subject: [PATCH 1/7] update pylint version --- requirements-lint.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a872fae8582da1846fbb0af7862ef90daea1c625 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Tue, 27 Dec 2022 02:27:33 +0100 Subject: [PATCH 2/7] add coverage.lcov to .gitignore --- .gitignore | 1 + can/io/logger.py | 10 ++++------ can/io/trc.py | 38 +++++++++++++++++++++----------------- 3 files changed, 26 insertions(+), 23 deletions(-) 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/can/io/logger.py b/can/io/logger.py index a08cf9869..51f569762 100644 --- a/can/io/logger.py +++ b/can/io/logger.py @@ -90,7 +90,7 @@ 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) @@ -100,9 +100,7 @@ def __new__( # type: ignore ) 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. @@ -184,7 +182,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 +203,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. diff --git a/can/io/trc.py b/can/io/trc.py index 0a07f01c9..e11ff97ea 100644 --- a/can/io/trc.py +++ b/can/io/trc.py @@ -7,7 +7,7 @@ Version 1.1 will be implemented as it is most commonly used """ # noqa -from typing import Generator, Optional, Union, TextIO +from typing import Generator, Optional, Union, TextIO, Callable, List from datetime import datetime, timedelta from enum import Enum from io import TextIOWrapper @@ -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,12 +214,12 @@ def __init__( """ super().__init__(file, mode="w") self.channel = channel - if type(file) is str: + if isinstance(file, str): self.filepath = os.path.abspath(file) - elif type(file) is TextIOWrapper: + elif isinstance(file, TextIOWrapper): self.filepath = "Unknown" logger.warning("TRCWriter: Text mode io can result in wrong line endings") - logger.debug( + logger.debug( # pylint: disable=logging-fstring-interpolation f"TRCWriter: Text mode io line ending setting: {file.newlines}" ) else: @@ -226,6 +229,7 @@ def __init__( 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: @@ -316,7 +320,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 From 3ff13c6c410e76eaf06961e5a21dd742924962f8 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Tue, 27 Dec 2022 02:28:08 +0100 Subject: [PATCH 3/7] check can/io with pylint --- .github/workflows/ci.yml | 1 + .pylintrc | 1 + can/io/asc.py | 11 +++++------ can/io/canutils.py | 8 ++++---- can/io/generic.py | 6 ++++-- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 577ccd97d..af4d2299c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,6 +91,7 @@ jobs: run: | pylint --rcfile=.pylintrc \ can/**.py \ + can/io \ setup.py \ doc.conf \ scripts/**.py \ diff --git a/.pylintrc b/.pylintrc index a42935e6a..73d83a852 100644 --- a/.pylintrc +++ b/.pylintrc @@ -76,6 +76,7 @@ disable=invalid-name, fixme, # We deliberately use TODO/FIXME inline abstract-class-instantiated, # Needed for can.Bus duplicate-code, # Needed due to https://github.com/PyCQA/pylint/issues/214 + keyword-arg-before-vararg, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/can/io/asc.py b/can/io/asc.py index 7cefa5b76..7a4418f07 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 @@ -93,7 +93,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 +101,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: diff --git a/can/io/canutils.py b/can/io/canutils.py index e159ecdf4..c8badacb1 100644 --- a/can/io/canutils.py +++ b/can/io/canutils.py @@ -173,11 +173,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 +193,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/generic.py b/can/io/generic.py index d5c7a2057..9f2b457a8 100644 --- a/can/io/generic.py +++ b/can/io/generic.py @@ -28,7 +28,7 @@ class BaseIOHandler(ContextManager, metaclass=ABCMeta): file: Optional[can.typechecking.FileLike] - def __init__( + def __init__( # pylint: disable=unused-argument self, file: Optional[can.typechecking.AcceptedIOType], mode: str = "rt", @@ -49,7 +49,9 @@ def __init__( # file is some path-like object self.file = cast( can.typechecking.FileLike, - open(cast(can.typechecking.StringPathLike, file), mode), + open( # pylint: disable=unspecified-encoding + cast(can.typechecking.StringPathLike, file), mode + ), ) # for multiple inheritance From ae1fb1efba51156ace9299adff456324fd118865 Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Tue, 27 Dec 2022 02:54:26 +0100 Subject: [PATCH 4/7] check can/interfaces/socketcan with pylint --- .github/workflows/ci.yml | 5 +++-- can/interfaces/socketcan/socketcan.py | 22 ++++++++-------------- can/interfaces/socketcan/utils.py | 3 ++- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af4d2299c..5c1d365e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,9 +93,10 @@ jobs: 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/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index f0545f7df..a29edf601 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.constants import * # pylint: disable=unused-wildcard-import from can.interfaces.socketcan.utils import pack_filters, find_available_interfaces @@ -391,7 +385,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) @@ -625,8 +619,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 +650,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 @@ -830,7 +824,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) @@ -899,8 +895,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 [] From 6f5e600a20dcf39deadaa8068c7e8bba05174e4b Mon Sep 17 00:00:00 2001 From: zariiii9003 <52598363+zariiii9003@users.noreply.github.com> Date: Wed, 28 Dec 2022 23:39:58 +0100 Subject: [PATCH 5/7] address review comments --- .pylintrc | 4 ++-- can/bus.py | 4 +--- can/io/asc.py | 2 -- can/io/blf.py | 2 -- can/io/canutils.py | 2 -- can/io/csv.py | 2 -- can/io/generic.py | 27 ++++++++++++--------------- can/io/logger.py | 18 ++++++++---------- can/io/player.py | 5 ++--- can/io/printer.py | 1 - can/io/sqlite.py | 2 -- can/listener.py | 10 ++++++---- can/logconvert.py | 2 +- can/logger.py | 2 +- can/message.py | 4 +--- 15 files changed, 34 insertions(+), 53 deletions(-) diff --git a/.pylintrc b/.pylintrc index 73d83a852..cc4c50d88 100644 --- a/.pylintrc +++ b/.pylintrc @@ -76,13 +76,13 @@ disable=invalid-name, fixme, # We deliberately use TODO/FIXME inline abstract-class-instantiated, # Needed for can.Bus duplicate-code, # Needed due to https://github.com/PyCQA/pylint/issues/214 - keyword-arg-before-vararg, # Enable the message, report, category or checker with the given id(s). You can # 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/io/asc.py b/can/io/asc.py index 7a4418f07..eb59c0471 100644 --- a/can/io/asc.py +++ b/can/io/asc.py @@ -39,7 +39,6 @@ def __init__( file: Union[StringPathLike, TextIO], base: str = "hex", relative_timestamp: bool = True, - *args: Any, **kwargs: Any, ) -> None: """ @@ -353,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 c8badacb1..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, ): """ 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 9f2b457a8..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, @@ -28,12 +28,11 @@ class BaseIOHandler(ContextManager, metaclass=ABCMeta): file: Optional[can.typechecking.FileLike] - def __init__( # pylint: disable=unused-argument + 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,12 +44,17 @@ def __init__( # pylint: disable=unused-argument # 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( # pylint: disable=unspecified-encoding - cast(can.typechecking.StringPathLike, file), mode + open( + cast(can.typechecking.StringPathLike, file), mode, encoding=encoding ), ) @@ -76,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 51f569762..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( @@ -93,7 +93,7 @@ def __new__( # type: ignore 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}"' @@ -152,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 @@ -232,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: @@ -321,7 +320,6 @@ def __init__( self, base_filename: StringPathLike, max_bytes: int = 0, - *args: Any, **kwargs: Any, ) -> None: """ @@ -332,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/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. From 99b63f87b6936d71779e0823a923a2ed3383f2ff Mon Sep 17 00:00:00 2001 From: zariiii9003 Date: Sat, 31 Dec 2022 14:33:25 +0100 Subject: [PATCH 6/7] replace wildcard import --- can/interfaces/socketcan/socketcan.py | 74 ++++++++++++++++----------- 1 file changed, 43 insertions(+), 31 deletions(-) diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index a29edf601..74fbe8197 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -38,7 +38,7 @@ LimitedDurationCyclicSendTaskABC, ) from can.typechecking import CanFilters -from can.interfaces.socketcan.constants import * # pylint: disable=unused-wildcard-import +from can.interfaces.socketcan import constants from can.interfaces.socketcan.utils import pack_filters, find_available_interfaces @@ -171,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 @@ -205,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) @@ -217,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""" @@ -248,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] @@ -261,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 @@ -291,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 @@ -348,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) @@ -374,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, @@ -489,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") @@ -528,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 @@ -543,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) @@ -558,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) @@ -659,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) @@ -667,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) @@ -675,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( @@ -846,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 From 41b814ef6bda9a5e5303d68e088fe2c5566aede5 Mon Sep 17 00:00:00 2001 From: zariiii9003 Date: Sat, 31 Dec 2022 16:02:02 +0100 Subject: [PATCH 7/7] fix TRCWriter line endings --- can/io/trc.py | 112 ++++++++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 64 deletions(-) diff --git a/can/io/trc.py b/can/io/trc.py index e11ff97ea..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, Callable, List 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 @@ -214,17 +214,13 @@ def __init__( """ super().__init__(file, mode="w") self.channel = channel - if isinstance(file, str): - self.filepath = os.path.abspath(file) - elif isinstance(file, TextIOWrapper): - self.filepath = "Unknown" - logger.warning("TRCWriter: Text mode io can result in wrong line endings") - logger.debug( # pylint: disable=logging-fstring-interpolation - 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 @@ -232,64 +228,52 @@ def __init__( 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: @@ -340,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: