diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 168ef45..3c2159f 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -24,13 +24,16 @@ jobs: - uses: actions/setup-python@v6.3.0 with: python-version: '3.10' - cache: 'pip' + cache: 'pip' # Safely caches dependencies - name: Install dependencies - run: pip install vermin -e .[lint] + run: pip install -e .[lint] + + - name: Check style and syntax (Ruff) + run: ruff check . - - name: Check minimum Python version (must not exceed 3.10) - run: vermin --target=3.10- --backport enum j1939/ + - name: Check minimum Python version compatibility (Vermin) + run: vermin --target=3.10- --no-parse-comments --no-tips j1939/ - name: Run Pyright run: pyright diff --git a/conftest.py b/conftest.py index 5284a48..85e85f5 100644 --- a/conftest.py +++ b/conftest.py @@ -2,6 +2,7 @@ from test.helpers.feeder import Feeder + @pytest.fixture() def feeder(): # setup diff --git a/docs/conf.py b/docs/conf.py index b6c8ada..8dacada 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # # Configuration file for the Sphinx documentation builder. # @@ -233,4 +232,4 @@ def _generate_examples_rst(): ] -# -- Extension configuration ------------------------------------------------- \ No newline at end of file +# -- Extension configuration ------------------------------------------------- diff --git a/examples/diagnostic_message.py b/examples/diagnostic_message.py index 4ea18b8..76c7ca4 100644 --- a/examples/diagnostic_message.py +++ b/examples/diagnostic_message.py @@ -1,6 +1,6 @@ import logging import time -import can + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -130,4 +130,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/j1939_21_cmdt_send_receive/j1939_receive.py b/examples/j1939_21_cmdt_send_receive/j1939_receive.py index b01bb0f..6791b8b 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_receive.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_receive.py @@ -1,8 +1,7 @@ import logging import time -import can + import j1939 -from hexdump import hexdump logging.getLogger('j1939').setLevel(logging.DEBUG) logging.getLogger('can').setLevel(logging.DEBUG) diff --git a/examples/j1939_21_cmdt_send_receive/j1939_send.py b/examples/j1939_21_cmdt_send_receive/j1939_send.py index dec3a96..a3fe96d 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_send.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_send.py @@ -1,10 +1,10 @@ import logging import time -import can -import j1939 -import os + from hexdump import hexdump +import j1939 + logging.getLogger('j1939').setLevel(logging.DEBUG) logging.getLogger('can').setLevel(logging.DEBUG) @@ -129,4 +129,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/j1939_22_multi_pg.py b/examples/j1939_22_multi_pg.py index 600106a..7712334 100644 --- a/examples/j1939_22_multi_pg.py +++ b/examples/j1939_22_multi_pg.py @@ -1,9 +1,9 @@ import logging import time + import j1939 from j1939.message_id import FrameFormat - logging.getLogger('j1939').setLevel(logging.DEBUG) logging.getLogger('can').setLevel(logging.DEBUG) @@ -22,7 +22,7 @@ def on_message(priority, pgn, sa, timestamp, data): :param bytearray data: Data of the PDU """ - print("PGN {} length {}".format(pgn, len(data)), timestamp) + print(f"PGN {pgn} length {len(data)}", timestamp) def ca_timer_callback1(ca : j1939.ControllerApplication): @@ -99,4 +99,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/j1939_22_transport_protocols.py b/examples/j1939_22_transport_protocols.py index 7dd7966..9e25cd3 100644 --- a/examples/j1939_22_transport_protocols.py +++ b/examples/j1939_22_transport_protocols.py @@ -1,7 +1,7 @@ import logging import time -import j1939 +import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) logging.getLogger('can').setLevel(logging.DEBUG) @@ -21,7 +21,7 @@ def on_message(priority, pgn, sa, timestamp, data): :param bytearray data: Data of the PDU """ - print("PGN {} length {}".format(pgn, len(data)), timestamp) + print(f"PGN {pgn} length {len(data)}", timestamp) def ca_timer_callback1(ca): @@ -95,4 +95,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/own_ca_producer.py b/examples/own_ca_producer.py index bb7034e..4f30765 100644 --- a/examples/own_ca_producer.py +++ b/examples/own_ca_producer.py @@ -1,6 +1,6 @@ import logging import time -import can + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -37,7 +37,7 @@ def ca_receive(priority, pgn, source, timestamp, data): :param bytearray data: Data of the PDU """ - print("PGN {} length {}".format(pgn, len(data))) + print(f"PGN {pgn} length {len(data)}") def ca_timer_callback1(cookie): """Callback for sending messages @@ -126,4 +126,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/simple_receive_global.py b/examples/simple_receive_global.py index 6fd12d4..eb7ac96 100644 --- a/examples/simple_receive_global.py +++ b/examples/simple_receive_global.py @@ -1,6 +1,6 @@ import logging import time -import can + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -20,7 +20,7 @@ def on_message(priority, pgn, sa, timestamp, data): :param bytearray data: Data of the PDU """ - print("PGN {} length {}".format(pgn, len(data))) + print(f"PGN {pgn} length {len(data)}") def main(): print("Initializing") @@ -47,4 +47,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/examples/simple_receive_peer_to_peer.py b/examples/simple_receive_peer_to_peer.py index caa62d4..546dce7 100644 --- a/examples/simple_receive_peer_to_peer.py +++ b/examples/simple_receive_peer_to_peer.py @@ -1,6 +1,6 @@ import logging import time -import can + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -20,7 +20,7 @@ def on_message(priority, pgn, sa, timestamp, data): :param bytearray data: Data of the PDU """ - print("PGN {} length {}".format(hex(pgn), len(data))) + print(f"PGN {hex(pgn)} length {len(data)}") def main(): print("Initializing") @@ -47,4 +47,4 @@ def main(): ecu.disconnect() if __name__ == '__main__': - main() \ No newline at end of file + main() diff --git a/j1939/Dm14Query.py b/j1939/Dm14Query.py index f92ba03..54c51f9 100644 --- a/j1939/Dm14Query.py +++ b/j1939/Dm14Query.py @@ -1,8 +1,10 @@ + from __future__ import annotations + +import queue from collections.abc import Callable from enum import Enum -from typing import Optional -import queue + import j1939 @@ -42,14 +44,14 @@ def __init__(self, ca: j1939.ControllerApplication, user_level=7) -> None: self._ca = ca self.state = QueryState.IDLE - self._seed_from_key: Optional[Callable[[int], int]] = None + self._seed_from_key: Callable[[int], int] | None = None self.data_queue: queue.Queue = queue.Queue() self.mem_data = None self.exception_queue: queue.Queue = queue.Queue() self.user_level = user_level - self._dest_address: Optional[int] = None - self.address: Optional[int] = None - self.command: Optional[Command] = None + self._dest_address: int | None = None + self.address: int | None = None + self.command: Command | None = None def unsubscribe_all(self) -> None: """ diff --git a/j1939/Dm14Server.py b/j1939/Dm14Server.py index 3826840..f8039fd 100644 --- a/j1939/Dm14Server.py +++ b/j1939/Dm14Server.py @@ -1,9 +1,11 @@ + from __future__ import annotations -from collections.abc import Callable -from enum import Enum -from typing import Optional + import queue import secrets +from collections.abc import Callable +from enum import Enum + import j1939 @@ -27,13 +29,13 @@ def __init__(self, ca: j1939.ControllerApplication) -> None: self._ca = ca self._busy = False - self.sa: Optional[int] = None + self.sa: int | None = None self.state = ResponseState.IDLE - self._key_from_seed: Optional[Callable[[int], int]] = None + self._key_from_seed: Callable[[int], int] | None = None self.data_queue: queue.Queue = queue.Queue() self._seed_generator: Callable[[], int] = self.generate_seed - self._verify_key: Optional[Callable[..., bool]] = None - self.address: Optional[bytearray] = None + self._verify_key: Callable[..., bool] | None = None + self.address: bytearray | None = None self.length = 8 self.proceed = False self.data: bytearray | list = [] @@ -180,9 +182,10 @@ def _send_dm15( state: ResponseState, object_count: int, sa: int, - pgn: int = j1939.ParameterGroupNumber.PGN.DM15, - error: Optional[int] = None, - edcp: Optional[int] = None, + pgn: int = 55296, # FIXME: we should use constants, like we used to: j1939.ParameterGroupNumber.PGN.DM15, but we get into circular imports errors. https://github.com/RaulSMS/python-can-j1939/issues/24 + error: int | None = None, + edcp: int | None = None, + ) -> None: """ Send DM15 message to device, used to send the proceed message, @@ -244,7 +247,7 @@ def _send_dm16(self) -> None: data = [] byte_count = len(self.data) data.append(0xFF if byte_count > 7 else byte_count) - for i in range((byte_count)): + for i in range(byte_count): data.append(self.data[i]) data.extend([0xFF] * (self.length - byte_count - 1)) @@ -379,7 +382,7 @@ def respond( error: int = 0xFFFFFF, edcp: int = 0xFF, max_timeout: int = 3, - ) -> Optional[list]: + ) -> list | None: """ Respond to DM14 query with the requested data or confimation of operation is good to proceed :param bool proceed: whether the operation is good to proceed @@ -404,7 +407,7 @@ def respond( else: self.state = ResponseState.SEND_ERROR self._wait_for_data() - mem_data: Optional[list] = None + mem_data: list | None = None if self.state == ResponseState.WAIT_FOR_DM16: try: mem_data = self.data_queue.get(block=True, timeout=max_timeout) diff --git a/j1939/__init__.py b/j1939/__init__.py index e72fda9..fec139a 100644 --- a/j1939/__init__.py +++ b/j1939/__init__.py @@ -1,11 +1,11 @@ -from .version import __version__ -from .electronic_control_unit import ElectronicControlUnit -from .controller_application import ControllerApplication -from .name import Name -from .message_id import MessageId -from .parameter_group_number import ParameterGroupNumber -from .diagnostic_messages import * -from .memory_access import * -from .error_info import * -from .Dm14Query import * -from .Dm14Server import * +from .controller_application import ControllerApplication as ControllerApplication +from .diagnostic_messages import * # noqa: F403 +from .Dm14Query import * # noqa: F403 +from .Dm14Server import * # noqa: F403 +from .electronic_control_unit import ElectronicControlUnit as ElectronicControlUnit +from .error_info import * # noqa: F403 +from .memory_access import * # noqa: F403 +from .message_id import MessageId as MessageId +from .name import Name as Name +from .parameter_group_number import ParameterGroupNumber as ParameterGroupNumber +from .version import __version__ as __version__ diff --git a/j1939/controller_application.py b/j1939/controller_application.py index 68caeed..ff3c528 100644 --- a/j1939/controller_application.py +++ b/j1939/controller_application.py @@ -1,7 +1,9 @@ from __future__ import annotations + import logging -from typing import Optional + import j1939 + from .message_id import FrameFormat logger = logging.getLogger(__name__) @@ -54,7 +56,7 @@ def __init__(self, name, device_address_preferred=None, bypass_address_claim=Fal self._device_address_announced = j1939.ParameterGroupNumber.Address.NULL self._device_address = j1939.ParameterGroupNumber.Address.NULL self._device_address_state = ControllerApplication.State.NONE - self._ecu: Optional[j1939.ElectronicControlUnit] = None + self._ecu: j1939.ElectronicControlUnit | None = None self._subscribers_request = [] self._subscribers_acknowledge = [] self._started = False @@ -174,7 +176,7 @@ def stop(self): def _process_claim_async(self, cookie): time_to_sleep = 0.500 if self._device_address_state == ControllerApplication.State.NONE: - if self._device_address_preferred != None: + if self._device_address_preferred is not None: self._device_address_announced = self._device_address_preferred self._send_address_claimed(self._device_address_announced) if self._device_address_announced > 127 and self._device_address_announced < 248: @@ -231,7 +233,7 @@ def _process_addressclaim(self, mid, data, timestamp): # TODO: are there any state variables we have to care about? self._device_address = j1939.ParameterGroupNumber.Address.NULL # TODO: maybe we should call an overloadable function here - if self._name.arbitrary_address_capable == False: + if not self._name.arbitrary_address_capable: # bad luck logger.error("After releasing our address we are configured to stop operation (CANNOT CLAIM)") self._device_address_state = ControllerApplication.State.CANNOT_CLAIM diff --git a/j1939/diagnostic_messages.py b/j1939/diagnostic_messages.py index 6d5ede2..531262a 100644 --- a/j1939/diagnostic_messages.py +++ b/j1939/diagnostic_messages.py @@ -1,6 +1,7 @@ -import j1939 import logging +import j1939 + logger = logging.getLogger(__name__) class DTC: @@ -147,7 +148,7 @@ def get_data(self, status_dic): data = [0]*2 for idx, lamp_key in enumerate(self._KEYS): # initialize not available lamps - if status_dic.get(lamp_key) == None: + if status_dic.get(lamp_key) is None: status_dic[lamp_key] = DtcLamp.OFF elif status_dic[lamp_key] not in self._DATA_LUT: status_dic[lamp_key] = DtcLamp.OFF @@ -197,7 +198,7 @@ def subscribe(self, callback): :param callback: Function to call when Dm1 message is received. """ - if self._msg_subscriber_added == False: + if not self._msg_subscriber_added: self._ca.subscribe(self._receive) self._msg_subscriber_added = True @@ -276,12 +277,12 @@ def _send(self, cookie): # create payload - dtc for dtc_dic in self._dtc_dic_list: # not optional arguments - if dtc_dic.get('spn') == None: + if dtc_dic.get('spn') is None: continue - if dtc_dic.get('fmi') == None: + if dtc_dic.get('fmi') is None: continue # optional arguments - if dtc_dic.get('oc') == None: + if dtc_dic.get('oc') is None: dtc_dic['oc'] = 0 cm = dtc_dic.get('cm', 4) @@ -382,7 +383,7 @@ def _on_request(self, src_address, dest_address, pgn): # TODO: send acknowledge def _on_acknowledge(self, src_address, dest_address, pgn): - for subscriber in self._subscribers_ack_clear: + for _subscriber in self._subscribers_ack_clear: # TODO pass @@ -447,4 +448,4 @@ def _send_request(self, control_byte, dest_address, fmi, spn): data[7] = ((spn >> 22) & 0xE0) | (fmi & 0x1F) # send pgn - self._ca.send_pgn(0, (self._pgn >> 8) & 0xFF, dest_address & 0xFF, 6, data) \ No newline at end of file + self._ca.send_pgn(0, (self._pgn >> 8) & 0xFF, dest_address & 0xFF, 6, data) diff --git a/j1939/electronic_control_unit.py b/j1939/electronic_control_unit.py index 671a8ed..8641f6b 100644 --- a/j1939/electronic_control_unit.py +++ b/j1939/electronic_control_unit.py @@ -2,16 +2,18 @@ import heapq import logging +import queue +import threading +import time + import can from can import Listener -import time -import threading -import queue + from .controller_application import ControllerApplication -from .parameter_group_number import ParameterGroupNumber from .j1939_21 import J1939_21 from .j1939_22 import J1939_22 from .message_id import FrameFormat +from .parameter_group_number import ParameterGroupNumber logger = logging.getLogger(__name__) @@ -468,7 +470,7 @@ def _notify_subscribers(self, priority, pgn, sa, dest, timestamp, data): :param bytearray data: Data of the PDU """ - logger.debug("notify subscribers for PGN {}".format(pgn)) + logger.debug(f"notify subscribers for PGN {pgn}") # Snapshot under lock so subscribe/unsubscribe from any thread is safe. with self._subscribers_lock: snapshot = list(self._subscribers) @@ -492,7 +494,7 @@ def __init__(self, ecu : ElectronicControlUnit): self.stopped = False def on_message_received(self, msg : can.Message): - if self.stopped or msg.is_error_frame or msg.is_remote_frame or (msg.is_extended_id == False): + if self.stopped or msg.is_error_frame or msg.is_remote_frame or (not msg.is_extended_id): return try: diff --git a/j1939/error_info.py b/j1939/error_info.py index 3a5cd7d..6efb307 100644 --- a/j1939/error_info.py +++ b/j1939/error_info.py @@ -1,5 +1,6 @@ from enum import Enum + class J1939Error(Enum): """ Enum of general errors based off of SAE Mobilus guidelines @@ -90,4 +91,4 @@ class J1939Error(Enum): J1939Error.INITILIZATION_TIMEOUT.value: "Initilization timeout", J1939Error.COMPLETION_TIMEOUT.value: "Completion timeout", J1939Error.NO_INDICATOR.value: "No indicator", -} \ No newline at end of file +} diff --git a/j1939/j1939_21.py b/j1939/j1939_21.py index d6fa4f2..3322aaf 100644 --- a/j1939/j1939_21.py +++ b/j1939/j1939_21.py @@ -1,9 +1,10 @@ -from .parameter_group_number import ParameterGroupNumber -from .message_id import MessageId import logging import threading import time +from .message_id import MessageId +from .parameter_group_number import ParameterGroupNumber + logger = logging.getLogger(__name__) class J1939_21: @@ -52,7 +53,7 @@ def __init__(self, send_message, job_thread_wakeup, notify_subscribers, max_cmdt self._minimum_tp_rts_cts_dt_interval = minimum_tp_rts_cts_dt_interval # set minimum time between two tp-bam messages - if minimum_tp_bam_dt_interval == None: + if minimum_tp_bam_dt_interval is None: self._minimum_tp_bam_dt_interval = self.Timeout.Tb else: self._minimum_tp_bam_dt_interval = minimum_tp_bam_dt_interval @@ -227,7 +228,7 @@ def async_job_thread(self, now): buf['state'] = self.SendBufferState.WAITING_CTS buf['deadline'] = time.monotonic() + self.Timeout.T3 should_break = True - elif self._minimum_tp_rts_cts_dt_interval != None: + elif self._minimum_tp_rts_cts_dt_interval is not None: buf['deadline'] = time.monotonic() + self._minimum_tp_rts_cts_dt_interval should_break = True @@ -523,7 +524,7 @@ def notify(self, can_id, data, timestamp): if ca.message_acceptable(dest_address): reject = False break - if reject == True: + if reject: return if pgn_value == ParameterGroupNumber.PGN.ADDRESSCLAIM: diff --git a/j1939/j1939_22.py b/j1939/j1939_22.py index 1f534df..1a0811c 100644 --- a/j1939/j1939_22.py +++ b/j1939/j1939_22.py @@ -1,9 +1,10 @@ -from .parameter_group_number import ParameterGroupNumber -from .message_id import MessageId, FrameFormat -from enum import IntEnum import logging import threading import time +from enum import IntEnum + +from .message_id import FrameFormat, MessageId +from .parameter_group_number import ParameterGroupNumber logger = logging.getLogger(__name__) @@ -70,15 +71,11 @@ def __init__(self, send_message, job_thread_wakeup, notify_subscribers, max_cmdt # List of ControllerApplication self._cas = [] - self._LUT_FD_DLC = [] - for i in range(9): self._LUT_FD_DLC.append(i) - for _ in range(4): self._LUT_FD_DLC.append(12) - for _ in range(4): self._LUT_FD_DLC.append(16) - for _ in range(4): self._LUT_FD_DLC.append(20) - for _ in range(4): self._LUT_FD_DLC.append(24) - for _ in range(8): self._LUT_FD_DLC.append(32) - for _ in range(16): self._LUT_FD_DLC.append(48) - for _ in range(16): self._LUT_FD_DLC.append(64) + self._LUT_FD_DLC = ( + list(range(9)) + + [12] * 4 + [16] * 4 + [20] * 4 + [24] * 4 + + [32] * 8 + [48] * 16 + [64] * 16 + ) # minimum time between two tp rts/cts dt frames, not necessary for standard conforming applications, # (they would use RTS/CTS flow control), but helps to talk to others without patching the library @@ -86,7 +83,7 @@ def __init__(self, send_message, job_thread_wakeup, notify_subscribers, max_cmdt # minimum time between two tp bam dt frames, inital value is 10ms # specified time range in j1939-22: 10-200ms - if minimum_tp_bam_dt_interval == None: + if minimum_tp_bam_dt_interval is None: self._minimum_tp_bam_dt_interval = 0.010 else: self._minimum_tp_bam_dt_interval = minimum_tp_bam_dt_interval @@ -177,7 +174,7 @@ def _buffer_unhash_mpg(self, hash): def __get_bam_session(self): for idx, i in enumerate(self.__bam_session_list): - if i == True: + if i: self.__bam_session_list[idx] = False return idx return None @@ -187,7 +184,7 @@ def __put_bam_session(self, session): def __get_rts_cts_session(self): for idx, i in enumerate(self.__rts_cts_session_list): - if i == True: + if i: self.__rts_cts_session_list[idx] = False return idx return None @@ -252,13 +249,13 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d if (pdu_specific == ParameterGroupNumber.Address.GLOBAL) or ParameterGroupNumber(0, pdu_format, pdu_specific).is_pdu2_format: dest_address = ParameterGroupNumber.Address.GLOBAL session_num = self.__get_bam_session() - if session_num == None: + if session_num is None: #print('bam session not available') return False else: dest_address = pdu_specific session_num = self.__get_rts_cts_session() - if session_num == None: + if session_num is None: #print('rts/cts session not available') return False @@ -269,7 +266,8 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d num_segments = int(message_size / self.DataLength.TP ) + ((message_size % self.DataLength.TP ) != 0) # set default priority - if priority == None: priority = 7 + if priority is None: + priority = 7 # get chunks from data chunk_size = self.DataLength.TP @@ -328,8 +326,8 @@ def __send_multi_pg(self, frame_format, cpg_list, src_address, dst_address): for cpg in cpg_list: priority = min(cpg['priority'], priority) data.append( (cpg['tos'] << 5) | (cpg['tf'] << 2) | ((cpg['cpgn'] >> 16) & 0x3) ) - data.append( ((cpg['cpgn'] >> 8) & 0xFF) ) - data.append( (cpg['cpgn'] & 0xFF) ) + data.append( (cpg['cpgn'] >> 8) & 0xFF ) + data.append( cpg['cpgn'] & 0xFF ) data.append( cpg['data_length'] ) data.extend( cpg['data']) @@ -425,7 +423,7 @@ def async_job_thread(self, now): buf['state'] = self.SendBufferState.WAITING_CTS buf['deadline'] = time.monotonic() + self.Timeout.T3 break - elif self._minimum_tp_rts_cts_dt_interval != None: + elif self._minimum_tp_rts_cts_dt_interval is not None: buf['deadline'] = time.monotonic() + self._minimum_tp_rts_cts_dt_interval break @@ -638,7 +636,6 @@ def _process_tp_dt(self, mid, dest_address, data, timestamp): return src_address = mid.source_address - dtfi = data[0] & 0xF # Data Transfer Format Indicator session_num = (data[0] >> 4) & 0xF segment_num = (data[1] & 0xFF) | ((data[2] & 0xFF) << 8) | ((data[3] & 0xFF) << 16) @@ -777,7 +774,8 @@ def __send_tp_dt(self, src_address, dest_address, session_num, segment_num, data else: # padding next_valid_fd_length = self._LUT_FD_DLC[len(data)] - if next_valid_fd_length < 0: next_valid_fd_length = 0 + if next_valid_fd_length < 0: + next_valid_fd_length = 0 while len(data) Optional[list]: + ) -> list | None: """ Responds with requested data and error code, if applicable, to a read request diff --git a/j1939/message_id.py b/j1939/message_id.py index 394b917..b4d0025 100644 --- a/j1939/message_id.py +++ b/j1939/message_id.py @@ -48,4 +48,4 @@ class FrameFormat: CBFF = 0 # classical base frame format CEFF = 1 # classical extended frame format FBFF = 2 # flexible data rate base frame format - FEFF = 3 # flexible data rate extended frame format \ No newline at end of file + FEFF = 3 # flexible data rate extended frame format diff --git a/j1939/parameter_group_number.py b/j1939/parameter_group_number.py index 15f80ad..faddc48 100644 --- a/j1939/parameter_group_number.py +++ b/j1939/parameter_group_number.py @@ -1,5 +1,6 @@ import j1939 + class ParameterGroupNumber: """Parameter Group Number (PGN). diff --git a/j1939/version.py b/j1939/version.py index a68927d..3dc1f76 100644 --- a/j1939/version.py +++ b/j1939/version.py @@ -1 +1 @@ -__version__ = "0.1.0" \ No newline at end of file +__version__ = "0.1.0" diff --git a/pyproject.toml b/pyproject.toml index bce7e1a..193c8f2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,7 +42,9 @@ test = [ "pytest >= 6.2.5", ] lint = [ + "ruff", "pyright >= 1.1", + "vermin", ] [project.urls] @@ -54,4 +56,35 @@ Documentation = "https://python-can-j1939.readthedocs.io/en/latest/" version = { attr = "j1939.version.__version__" } [tool.setuptools.packages.find] -exclude = ["docs*", "examples*", "test*"] \ No newline at end of file +exclude = ["docs*", "examples*", "test*"] + +[tool.ruff] +target-version = "py310" + +[tool.ruff.lint] +select = [ + "E", + "W", + "F", + "I", + "UP", + "B", + "N", +] + +ignore = [ + "E501", + "E701", + "E702", + "E703", + "E741", + "N806", + "N801", + "W291", + "W293", + "B904", + "N803", + "N999", +] + +per-file-ignores = { "__init__.py" = ["F401"] } \ No newline at end of file diff --git a/test/helpers/feeder.py b/test/helpers/feeder.py index e4d04e2..adcebb8 100644 --- a/test/helpers/feeder.py +++ b/test/helpers/feeder.py @@ -33,7 +33,7 @@ class Feeder: expected data, and then injecting the expected rx nessage into the ECU """ - class MsgType(object): + class MsgType: CANRX = 0 CANTX = 1 PDU = 2 @@ -83,7 +83,7 @@ def _send_message(self, can_id, extended_id, data, fd_format=False): The data is fed from self.can_messages. """ logger.info( - f'send message ID: {can_id:04x}, data: {["{:02x}".format(val) for val in data]}' + f'send message ID: {can_id:04x}, data: {[f"{val:02x}" for val in data]}' ) expected_data = self.can_messages.pop(0) assert expected_data[0] == Feeder.MsgType.CANTX @@ -106,7 +106,7 @@ def _on_message(self, priority, pgn, sa, timestamp, data): Data of the PDU """ logger.info( - f'received from sa {sa:02x} pgn {pgn:04x} data: {["{:02x}".format(val) for val in data]}' + f'received from sa {sa:02x} pgn {pgn:04x} data: {[f"{val:02x}" for val in data]}' ) expected_data = self.pdus.pop(0) assert expected_data[0] == Feeder.MsgType.PDU diff --git a/test/test_ecu.py b/test/test_ecu.py index e84739e..f797721 100644 --- a/test/test_ecu.py +++ b/test/test_ecu.py @@ -1,33 +1,7 @@ -import time import can -import j1939 -from test.helpers.feeder import Feeder - - -def receive(feeder): - feeder.ecu.subscribe(on_message) - feeder.inject_messages_into_ecu() - # wait until all messages are processed asynchronously - while len(pdus)>0: - time.sleep(0.500) - # wait for final processing - time.sleep(0.100) - feeder.ecu.unsubscribe(on_message) - -def send(feeder, pdu, source, destination): - feeder.ecu.subscribe(on_message) - - # sending from 240 to 155 with prio 6 - feeder.ecu.send_pgn(0, pdu[1]>>8, destination, 6, source, pdu[2]) - - # wait until all messages are processed asynchronously - while len(feeder.can_messages)>0: - time.sleep(0.500) - # wait for final processing - time.sleep(0.100) - feeder.ecu.unsubscribe(on_message) +from test.helpers.feeder import Feeder #def test_connect(self): # self.feeder.ecu.connect(bustype="virtual", channel=1) @@ -173,7 +147,7 @@ def test_add_bus(feeder): feeder.ecu.add_bus(bus) assert feeder.ecu._bus == bus feeder.ecu.remove_bus() - assert feeder.ecu._bus == None + assert feeder.ecu._bus is None def test_add_notfier(feeder): """ @@ -185,7 +159,7 @@ def test_add_notfier(feeder): feeder.ecu.add_notifier(notifier) assert feeder.ecu._notifier == notifier feeder.ecu.remove_notifier() - assert feeder.ecu._notifier == None + assert feeder.ecu._notifier is None def test_add_bus_filters(feeder): """ diff --git a/test/test_memory_access.py b/test/test_memory_access.py index b03be38..fe908fc 100644 --- a/test/test_memory_access.py +++ b/test/test_memory_access.py @@ -1,7 +1,9 @@ +import time + import pytest -from test.helpers.feeder import Feeder + import j1939 -import time +from test.helpers.feeder import Feeder # fmt: off read_with_seed_key = [ @@ -522,7 +524,7 @@ def test_dm14_request_write_timeout(feeder): assert flag is True, "Timeout waiting for DM14 request" reset_flag() dm14.respond(True, [], 0xFFFF, 0xFF) - assert str(excinfo.value) is "No response received for DM16 data transfer" + assert str(excinfo.value) == "No response received for DM16 data transfer" feeder.process_messages() @@ -628,7 +630,7 @@ def test_dm14_read_timeout_error(feeder): Tests that the DM14 read query can react to timeout errors correctly :param feeder: can message feeder """ - with pytest.raises(RuntimeError) as excinfo: + with pytest.raises(RuntimeError): feeder.can_messages = [ ( Feeder.MsgType.CANTX, @@ -676,7 +678,7 @@ def test_dm14_write_timeout(feeder): values = [0x11223344] dm14.write(0xD4, 1, 0x91000007, values, object_byte_size=4) - assert str(excinfo.value) is "No response from server" + assert str(excinfo.value) == "No response from server" feeder.process_messages() diff --git a/test/test_threading.py b/test/test_threading.py index b5633e9..7cd4b5d 100644 --- a/test/test_threading.py +++ b/test/test_threading.py @@ -50,10 +50,17 @@ def _wait_no_threads_named(name, timeout=0.5): time.sleep(0.01) return False -@pytest.mark.skip(reason=(f"This test is flaky and may fail on slow CI machines;\n" - f"Needs to be updated to allow more generous timing or use a more robust synchronization method.")) def test_timer_no_drift(): - """Verify heapq-based timer fires at consistent 50ms intervals without drift.""" + """Verify heapq-based timer fires reliably and doesn't deadlock. + + This test validates that the timer thread: + - Fires reliably (gets all expected callbacks) + - Doesn't deadlock or hang indefinitely + - Doesn't accumulate extreme outlier delays (> 500ms) + + Note: Strict interval timing is not validated on slow CI machines. + The focus is on correctness (fire count) and absence of hangs. + """ ecu = _make_ecu() timestamps = [] done = threading.Event() @@ -66,17 +73,28 @@ def callback(cookie): return True # reschedule ecu.add_timer(0.050, callback) - fired = done.wait(timeout=3.0) + fired = done.wait(timeout=10.0) # generous timeout for very slow CI ecu.stop() - assert fired, "Timer did not fire 10 times within 3 seconds" - assert len(timestamps) == 10 + assert fired, "Timer did not fire 10 times within 10 seconds - possible deadlock" + assert len(timestamps) == 10, f"Expected 10 callbacks, got {len(timestamps)}" intervals = [timestamps[i+1] - timestamps[i] for i in range(9)] - for idx, interval in enumerate(intervals): - assert abs(interval - 0.05) < 0.01, ( - f"Interval {idx} was {interval*1000:.1f}ms, expected ~50ms (±10ms)" - ) + + # Only check for extreme outliers that would indicate a broken timer + # (e.g., long GC pause, system under extreme load, or actual deadlock) + max_interval = max(intervals) + assert max_interval < 1.0, ( + f"Max interval was {max_interval*1000:.1f}ms, which is extreme " + "(expected < 1000ms even on slow CI). Timer may be deadlocked or broken." + ) + + # Log intervals for debugging CI issues + avg_interval = sum(intervals) / len(intervals) + assert avg_interval > 0.025, ( + f"Average interval was {avg_interval*1000:.1f}ms, " + "which is too fast (timer may be firing twice per cycle)" + ) def test_slow_callback_no_protocol_impact(feeder): @@ -120,7 +138,7 @@ def on_message(priority, pgn, sa, timestamp, data): feeder.ecu.subscribe(on_message) feeder.ecu.accept_all_messages = lambda: None # already set by Feeder init - ca = feeder.accept_all_messages() + feeder.accept_all_messages() start = time.monotonic() feeder._inject_messages_into_ecu() @@ -167,11 +185,16 @@ def hammer(): assert not errors, f"Exceptions during concurrent timer ops: {errors}" -@pytest.mark.skip(reason=(f"This test is flaky and may fail on slow CI machines;\n" - f"Needs to be updated to allow more generous timing or use a more robust synchronization method.")) def test_memory_access_event_latency(): - """MemoryAccess servicer thread responds to events within 5ms.""" - from j1939.memory_access import MemoryAccess, DMState + """MemoryAccess servicer thread responds to events within reasonable latency. + + This test validates that the servicer thread wakes up and responds to events + without excessive delay. Rather than enforcing sub-10ms latency (which is + unrealistic on slow CI machines with variable scheduler load), we check that: + - The servicer thread does respond eventually (not deadlocked) + - Response is within a generous time window (< 500ms) allowing for CI variability + """ + from j1939.memory_access import DMState, MemoryAccess ecu = _make_ecu() ca = ecu.add_ca(name=j1939.Name( @@ -200,8 +223,8 @@ def notify(): set_time.append(time.monotonic()) ma._proceed_event.set() - # Give the servicer thread up to 50ms to respond - deadline = time.monotonic() + 0.050 + # Give the servicer thread up to 500ms to respond (allows for slow CI machines) + deadline = time.monotonic() + 0.500 while not callback_times and time.monotonic() < deadline: time.sleep(0.001) @@ -209,8 +232,9 @@ def notify(): assert callback_times, "notify callback was never called after _proceed_event.set()" latency = callback_times[0] - set_time[0] - assert latency < 0.01, ( - f"MemoryAccess notify latency was {latency*1000:.2f}ms, expected < 10ms" + assert latency < 0.500, ( + f"MemoryAccess notify latency was {latency*1000:.2f}ms, " + f"expected < 500ms (thread should not be blocked/deadlocked)" )