From 60363291d4f4d0737db68d3a57ae4c5af50f7912 Mon Sep 17 00:00:00 2001 From: RaulSMS Date: Thu, 18 Jun 2026 17:17:42 +0200 Subject: [PATCH 01/13] fix(lint): fix F401/F403 import issues in __init__.py and setup.py Use explicit re-export pattern (X as X) for public API symbols in __init__.py so ruff recognises them as intentional. Suppress F403 on wildcard imports that are part of the public API. Add noqa for exec-loaded __version__ in setup.py. Co-Authored-By: Claude Sonnet 4.6 --- j1939/__init__.py | 22 +++++++++++----------- setup.py | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/j1939/__init__.py b/j1939/__init__.py index e72fda9..c96bf2d 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 .version import __version__ as __version__ +from .electronic_control_unit import ElectronicControlUnit as ElectronicControlUnit +from .controller_application import ControllerApplication as ControllerApplication +from .name import Name as Name +from .message_id import MessageId as MessageId +from .parameter_group_number import ParameterGroupNumber as ParameterGroupNumber +from .diagnostic_messages import * # noqa: F403 +from .memory_access import * # noqa: F403 +from .error_info import * # noqa: F403 +from .Dm14Query import * # noqa: F403 +from .Dm14Server import * # noqa: F403 diff --git a/setup.py b/setup.py index bb12623..acabcf2 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,4 @@ -from setuptools import setup, find_packages, Extension +from setuptools import setup, find_packages exec(open('j1939/version.py').read()) @@ -9,7 +9,7 @@ setup( name="can-j1939", url="https://github.com/juergenH87/python-can-j1939", - version=__version__, + version=__version__, # noqa: F821 — loaded via exec() above packages=find_packages(exclude=['docs', 'examples']), author="Juergen Heilgemeir", description="SAE J1939 stack implementation", From 102e48da5129e6f9227e252ae075dca84ef3bcc3 Mon Sep 17 00:00:00 2001 From: RaulSMS Date: Thu, 18 Jun 2026 17:17:48 +0200 Subject: [PATCH 02/13] fix(lint): remove dead receive/send helpers from test_ecu.py These functions referenced undefined names (on_message, pdus) and were never called. Removing them also drops the unused `time` import. Co-Authored-By: Claude Sonnet 4.6 --- test/test_ecu.py | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/test/test_ecu.py b/test/test_ecu.py index 59cd579..02ad614 100644 --- a/test/test_ecu.py +++ b/test/test_ecu.py @@ -1,33 +1,8 @@ -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) #def test_connect(self): # self.feeder.ecu.connect(bustype="virtual", channel=1) @@ -173,7 +148,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 +160,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): """ From 2fd60d7b6ad6537982acd649e32ad5b6fd957859 Mon Sep 17 00:00:00 2001 From: RaulSMS Date: Thu, 18 Jun 2026 17:17:54 +0200 Subject: [PATCH 03/13] fix(lint): replace == None/True/False with identity/truthiness checks Fix E711: use `is None` / `is not None` instead of == / != comparisons. Fix E712: use truthiness directly instead of comparing to True/False. Fix F841: remove unused local variable assignments (dtfi, excinfo, ca). Co-Authored-By: Claude Sonnet 4.6 --- j1939/controller_application.py | 4 ++-- j1939/diagnostic_messages.py | 10 ++++----- j1939/electronic_control_unit.py | 2 +- j1939/j1939_21.py | 6 +++--- j1939/j1939_22.py | 36 +++++++++++++++----------------- test/test_j1939_22.py | 1 - test/test_memory_access.py | 6 +++--- test/test_threading.py | 10 ++++----- 8 files changed, 36 insertions(+), 39 deletions(-) diff --git a/j1939/controller_application.py b/j1939/controller_application.py index bce738e..d67dd84 100644 --- a/j1939/controller_application.py +++ b/j1939/controller_application.py @@ -167,7 +167,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: @@ -224,7 +224,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 1cfb3e3..3c48d8f 100644 --- a/j1939/diagnostic_messages.py +++ b/j1939/diagnostic_messages.py @@ -145,7 +145,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 @@ -195,7 +195,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 @@ -274,12 +274,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) diff --git a/j1939/electronic_control_unit.py b/j1939/electronic_control_unit.py index 5234c33..2a59f9a 100644 --- a/j1939/electronic_control_unit.py +++ b/j1939/electronic_control_unit.py @@ -484,7 +484,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/j1939_21.py b/j1939/j1939_21.py index d6fa4f2..d174b6e 100644 --- a/j1939/j1939_21.py +++ b/j1939/j1939_21.py @@ -52,7 +52,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 +227,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 +523,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 7229905..c84e31b 100644 --- a/j1939/j1939_22.py +++ b/j1939/j1939_22.py @@ -69,15 +69,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 @@ -85,7 +81,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 @@ -176,7 +172,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 @@ -186,7 +182,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 @@ -251,13 +247,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 @@ -268,7 +264,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 @@ -424,7 +421,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 @@ -637,7 +634,7 @@ def _process_tp_dt(self, mid, dest_address, data, timestamp): return src_address = mid.source_address - dtfi = data[0] & 0xF # Data Transfer Format Indicator + 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) @@ -776,7 +773,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) Date: Thu, 18 Jun 2026 17:18:08 +0200 Subject: [PATCH 04/13] fix(lint): remove unused imports from examples (F401) Co-Authored-By: Claude Sonnet 4.6 --- examples/diagnostic_message.py | 1 - examples/j1939_21_cmdt_send_receive/j1939_receive.py | 2 -- examples/j1939_21_cmdt_send_receive/j1939_send.py | 2 -- examples/own_ca_producer.py | 1 - examples/simple_receive_global.py | 1 - examples/simple_receive_peer_to_peer.py | 1 - 6 files changed, 8 deletions(-) diff --git a/examples/diagnostic_message.py b/examples/diagnostic_message.py index 4ea18b8..1ab3915 100644 --- a/examples/diagnostic_message.py +++ b/examples/diagnostic_message.py @@ -1,6 +1,5 @@ import logging import time -import can import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) diff --git a/examples/j1939_21_cmdt_send_receive/j1939_receive.py b/examples/j1939_21_cmdt_send_receive/j1939_receive.py index b01bb0f..7c2201d 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_receive.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_receive.py @@ -1,8 +1,6 @@ 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..3ccbcbc 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_send.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_send.py @@ -1,8 +1,6 @@ import logging import time -import can import j1939 -import os from hexdump import hexdump logging.getLogger('j1939').setLevel(logging.DEBUG) diff --git a/examples/own_ca_producer.py b/examples/own_ca_producer.py index bb7034e..8287c2b 100644 --- a/examples/own_ca_producer.py +++ b/examples/own_ca_producer.py @@ -1,6 +1,5 @@ import logging import time -import can import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) diff --git a/examples/simple_receive_global.py b/examples/simple_receive_global.py index 6fd12d4..ad72709 100644 --- a/examples/simple_receive_global.py +++ b/examples/simple_receive_global.py @@ -1,6 +1,5 @@ import logging import time -import can import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) diff --git a/examples/simple_receive_peer_to_peer.py b/examples/simple_receive_peer_to_peer.py index caa62d4..d4f3431 100644 --- a/examples/simple_receive_peer_to_peer.py +++ b/examples/simple_receive_peer_to_peer.py @@ -1,6 +1,5 @@ import logging import time -import can import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) From de12fbb94615cbe87e2ae4ec97536548afb87f35 Mon Sep 17 00:00:00 2001 From: RaulSMS Date: Thu, 18 Jun 2026 17:18:14 +0200 Subject: [PATCH 05/13] ci: add ruff lint job and configure target-version Add a ruff lint job to CI that runs on Python 3.10 and enforces E and F rules. Configure target-version = py310 in setup.cfg so ruff aligns upgrade suggestions with the declared minimum Python version. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/CI.yml | 16 +++++++++++++++- setup.cfg | 6 +++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 6860df8..101c2c8 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -22,7 +22,21 @@ on: # A workflow run is made up of one or more jobs that can run sequentially or in parallel jobs: - # This workflow contains a single job called "test" + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.10' + + - name: Install ruff + run: pip install ruff + + - name: Lint + run: ruff check . + test: # The type of runner that the job will run on runs-on: ${{ matrix.os }} diff --git a/setup.cfg b/setup.cfg index 7c2b287..34a4a90 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,2 +1,6 @@ [bdist_wheel] -universal = 1 \ No newline at end of file +universal = 1 + +[ruff] +target-version = "py310" +select = ["E", "F"] \ No newline at end of file From 78b0872e8ae2dbd671e509734a4d879d6097e8fe Mon Sep 17 00:00:00 2001 From: RaulSMS Date: Fri, 19 Jun 2026 08:41:14 +0200 Subject: [PATCH 06/13] fix: add lint extra to setup.py and use it in CI Declare ruff as an installable dev dependency via pip install -e .[lint] so the lint toolchain is managed in one place. Update the CI lint job to install via the extra instead of a bare pip install ruff. Suggested-by: khauersp --- .github/workflows/CI.yml | 4 ++-- setup.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 101c2c8..5e10973 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -31,8 +31,8 @@ jobs: with: python-version: '3.10' - - name: Install ruff - run: pip install ruff + - name: Install lint dependencies + run: pip install -e ".[lint]" - name: Lint run: ruff check . diff --git a/setup.py b/setup.py index e65eef6..1d3b271 100644 --- a/setup.py +++ b/setup.py @@ -32,6 +32,9 @@ "test": [ "pytest >= 6.2.5", ], + "lint": [ + "ruff", + ], }, include_package_data=True, ) From ecc4217793eb061838aeed9e9b318afdb42ec424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 19 Jun 2026 17:00:58 +0200 Subject: [PATCH 07/13] fix/ruff settigns Ruff uses .toml file to get the settigns so actually the setup.cfg was doing anything --- pyproject.toml | 31 +++++++++++++++++++++++++++++++ setup.cfg | 6 +----- 2 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 pyproject.toml diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..e279e17 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,31 @@ +[tool.ruff] +target-version = "py310" + +[tool.ruff.lint] +select = [ + "E", # PEP 8 errors (formatting, whitespace) + "W", # PEP 8 warnings + "F", # Pyflakes (logical errors, unused imports, undefined names) + "I", # isort (import sorting & organization) + "UP", # pyupgrade (modern Python 3.10+ syntax) + "B", # flake8-bugbear (common bugs - includes B018 for unused expressions!) + "N", # pep8-naming (variable/function naming conventions) +] + +ignore = [ + "E501", # line too long (use a formatter instead) + "E701", # multiple statements on one line + "E702", # multiple statements on one line (colon) + "E703", # statement ends with semicolon + "E741", # ambiguous variable name + "N806", # variable name should be lowercase (for uppercase constants) + # FIXME: The following may introduce breaking changes, defer to later: + "N801", # invalid-class-name (5 occurrences) + "W291", # trailing-whitespace (4 occurrences) + "W293", # blank-line-with-whitespace (4 occurrences) + "B904", # raise-without-from-inside-except (3 occurrences) + "N803", # invalid-argument-name (2 occurrences) + "N999", # invalid-module-name (2 occurrences) +] + +per-file-ignores = { "__init__.py" = ["F401"] } # Allow unused imports in __init__.py diff --git a/setup.cfg b/setup.cfg index 34a4a90..7c2b287 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,2 @@ [bdist_wheel] -universal = 1 - -[ruff] -target-version = "py310" -select = ["E", "F"] \ No newline at end of file +universal = 1 \ No newline at end of file From c28901a1eb3e1fe300826fef9bbf63b617b9945d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 19 Jun 2026 17:03:37 +0200 Subject: [PATCH 08/13] fix/ new ruff findings ruff check . --statistics 25 I001 [*] unsorted-imports 12 W292 [*] missing-newline-at-end-of-file 8 UP032 [*] f-string 3 UP034 [*] extraneous-parentheses 1 B007 [ ] unused-loop-control-variable 1 UP004 [*] useless-object-inheritance 1 UP009 [*] utf8-encoding-declaration --- conftest.py | 1 + docs/conf.py | 4 ++-- examples/diagnostic_message.py | 3 ++- .../j1939_21_cmdt_send_receive/j1939_receive.py | 1 + examples/j1939_21_cmdt_send_receive/j1939_send.py | 6 ++++-- examples/j1939_22_multi_pg.py | 6 +++--- examples/j1939_22_transport_protocols.py | 6 +++--- examples/own_ca_producer.py | 5 +++-- examples/simple_receive_global.py | 5 +++-- examples/simple_receive_peer_to_peer.py | 5 +++-- j1939/Dm14Query.py | 3 ++- j1939/Dm14Server.py | 5 +++-- j1939/__init__.py | 14 +++++++------- j1939/controller_application.py | 2 ++ j1939/diagnostic_messages.py | 7 ++++--- j1939/electronic_control_unit.py | 12 +++++++----- j1939/error_info.py | 3 ++- j1939/j1939_21.py | 5 +++-- j1939/j1939_22.py | 9 +++++---- j1939/memory_access.py | 3 ++- j1939/message_id.py | 2 +- j1939/parameter_group_number.py | 1 + j1939/version.py | 2 +- setup.py | 2 +- test/helpers/feeder.py | 6 +++--- test/test_ecu.py | 3 +-- test/test_memory_access.py | 6 ++++-- test/test_threading.py | 2 +- 28 files changed, 75 insertions(+), 54 deletions(-) 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 8115e78..9dab639 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # # Configuration file for the Sphinx documentation builder. # @@ -14,6 +13,7 @@ # import os import sys + sys.path.insert(0, os.path.abspath('.')) sys.path.insert(0, os.path.abspath('../')) @@ -158,4 +158,4 @@ ] -# -- Extension configuration ------------------------------------------------- \ No newline at end of file +# -- Extension configuration ------------------------------------------------- diff --git a/examples/diagnostic_message.py b/examples/diagnostic_message.py index 1ab3915..76c7ca4 100644 --- a/examples/diagnostic_message.py +++ b/examples/diagnostic_message.py @@ -1,5 +1,6 @@ import logging import time + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -129,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 7c2201d..6791b8b 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_receive.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_receive.py @@ -1,5 +1,6 @@ import logging import time + import j1939 logging.getLogger('j1939').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 3ccbcbc..a3fe96d 100644 --- a/examples/j1939_21_cmdt_send_receive/j1939_send.py +++ b/examples/j1939_21_cmdt_send_receive/j1939_send.py @@ -1,8 +1,10 @@ import logging import time -import j1939 + from hexdump import hexdump +import j1939 + logging.getLogger('j1939').setLevel(logging.DEBUG) logging.getLogger('can').setLevel(logging.DEBUG) @@ -127,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 8287c2b..4f30765 100644 --- a/examples/own_ca_producer.py +++ b/examples/own_ca_producer.py @@ -1,5 +1,6 @@ import logging import time + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -36,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 @@ -125,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 ad72709..eb7ac96 100644 --- a/examples/simple_receive_global.py +++ b/examples/simple_receive_global.py @@ -1,5 +1,6 @@ import logging import time + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -19,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") @@ -46,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 d4f3431..546dce7 100644 --- a/examples/simple_receive_peer_to_peer.py +++ b/examples/simple_receive_peer_to_peer.py @@ -1,5 +1,6 @@ import logging import time + import j1939 logging.getLogger('j1939').setLevel(logging.DEBUG) @@ -19,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") @@ -46,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 7e426ab..bc105b9 100644 --- a/j1939/Dm14Query.py +++ b/j1939/Dm14Query.py @@ -1,5 +1,6 @@ -from enum import Enum import queue +from enum import Enum + import j1939 diff --git a/j1939/Dm14Server.py b/j1939/Dm14Server.py index 6e52caa..6cf6d95 100644 --- a/j1939/Dm14Server.py +++ b/j1939/Dm14Server.py @@ -1,6 +1,7 @@ -from enum import Enum import queue import secrets +from enum import Enum + import j1939 @@ -233,7 +234,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)) diff --git a/j1939/__init__.py b/j1939/__init__.py index c96bf2d..fec139a 100644 --- a/j1939/__init__.py +++ b/j1939/__init__.py @@ -1,11 +1,11 @@ -from .version import __version__ as __version__ -from .electronic_control_unit import ElectronicControlUnit as ElectronicControlUnit from .controller_application import ControllerApplication as ControllerApplication -from .name import Name as Name -from .message_id import MessageId as MessageId -from .parameter_group_number import ParameterGroupNumber as ParameterGroupNumber from .diagnostic_messages import * # noqa: F403 -from .memory_access import * # noqa: F403 -from .error_info 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 d67dd84..4207c41 100644 --- a/j1939/controller_application.py +++ b/j1939/controller_application.py @@ -1,5 +1,7 @@ import logging + import j1939 + from .message_id import FrameFormat logger = logging.getLogger(__name__) diff --git a/j1939/diagnostic_messages.py b/j1939/diagnostic_messages.py index 3c48d8f..84b4869 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: @@ -380,7 +381,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 @@ -445,4 +446,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 2a59f9a..3120f5b 100644 --- a/j1939/electronic_control_unit.py +++ b/j1939/electronic_control_unit.py @@ -1,15 +1,17 @@ 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__) @@ -460,7 +462,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) 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 d174b6e..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: diff --git a/j1939/j1939_22.py b/j1939/j1939_22.py index c84e31b..587aa48 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 import logging import threading import time +from .message_id import FrameFormat, MessageId +from .parameter_group_number import ParameterGroupNumber + logger = logging.getLogger(__name__) class J1939_22: @@ -324,8 +325,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']) diff --git a/j1939/memory_access.py b/j1939/memory_access.py index cfddc0d..8339b70 100644 --- a/j1939/memory_access.py +++ b/j1939/memory_access.py @@ -1,6 +1,7 @@ -from enum import Enum import logging import threading +from enum import Enum + import j1939 logger = logging.getLogger(__name__) 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 6463fd4..2fcc21e 100644 --- a/j1939/version.py +++ b/j1939/version.py @@ -1 +1 @@ -__version__ = "2.0.12" \ No newline at end of file +__version__ = "2.0.12" diff --git a/setup.py b/setup.py index 1d3b271..ec9442c 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,4 @@ -from setuptools import setup, find_packages +from setuptools import find_packages, setup exec(open('j1939/version.py').read()) 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 8d87a35..f797721 100644 --- a/test/test_ecu.py +++ b/test/test_ecu.py @@ -1,8 +1,7 @@ import can -from test.helpers.feeder import Feeder - +from test.helpers.feeder import Feeder #def test_connect(self): # self.feeder.ecu.connect(bustype="virtual", channel=1) diff --git a/test/test_memory_access.py b/test/test_memory_access.py index 6f84d16..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 = [ diff --git a/test/test_threading.py b/test/test_threading.py index 6228a9c..2eb6f02 100644 --- a/test/test_threading.py +++ b/test/test_threading.py @@ -171,7 +171,7 @@ def hammer(): "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 + from j1939.memory_access import DMState, MemoryAccess ecu = _make_ecu() ca = ecu.add_ca(name=j1939.Name( From 8a1ce3e6c3529c6682daadb8b833753c42e34e95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 19 Jun 2026 17:11:09 +0200 Subject: [PATCH 09/13] fix/ circular imports after ruff fixes python -m pytest . ImportError while loading conftest 'C:\00_Developer\GitHub\python-can-j1939\conftest.py'. conftest.py:3: in from test.helpers.feeder import Feeder test\helpers\feeder.py:6: in import j1939 j1939\__init__.py:4: in from .Dm14Server import * # noqa: F403 ^^^^^^^^^^^^^^^^^^^^^^^^^ j1939\Dm14Server.py:19: in class DM14Server: j1939\Dm14Server.py:179: in DM14Server pgn: int = j1939.ParameterGroupNumber.PGN.DM15, ^^^^^^^^^^^^^^^^^^^^^^^^^^ E AttributeError: partially initialized module 'j1939' has no attribute 'ParameterGroupNumber' (most likely due to a circular import) --- j1939/Dm14Server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/j1939/Dm14Server.py b/j1939/Dm14Server.py index 6cf6d95..37e4357 100644 --- a/j1939/Dm14Server.py +++ b/j1939/Dm14Server.py @@ -176,7 +176,7 @@ def _send_dm15( state: ResponseState, object_count: int, sa: int, - pgn: int = j1939.ParameterGroupNumber.PGN.DM15, + 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, edcp: int = None, ) -> None: From 04ed1523b163d569a4c9fd7001aa8a6e8c52e2d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 19 Jun 2026 17:15:00 +0200 Subject: [PATCH 10/13] Activate skkiped threading test With a bit more buffer for CI runners --- test/test_threading.py | 50 ++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/test/test_threading.py b/test/test_threading.py index 2eb6f02..169d0a0 100644 --- a/test/test_threading.py +++ b/test/test_threading.py @@ -50,10 +50,15 @@ def _wait_no_threads_named(name, timeout=0.5): time.sleep(0.01) return False -@pytest.mark.skip(reason=("This test is flaky and may fail on slow CI machines;\n" - "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 at consistent 50ms intervals without extreme drift. + + This test validates that the timer thread does not accumulate significant drift + over multiple firings. Rather than enforcing strict timing (which is unreliable + on slow CI machines), we check that: + - Most intervals are reasonably close to the target (within ±30ms) + - No single interval is catastrophically late (> 200ms) + """ ecu = _make_ecu() timestamps = [] done = threading.Event() @@ -66,17 +71,28 @@ def callback(cookie): return True # reschedule ecu.add_timer(0.050, callback) - fired = done.wait(timeout=3.0) + fired = done.wait(timeout=5.0) # increased timeout for slow machines ecu.stop() - assert fired, "Timer did not fire 10 times within 3 seconds" + assert fired, "Timer did not fire 10 times within 5 seconds" assert len(timestamps) == 10 intervals = [timestamps[i+1] - timestamps[i] for i in range(9)] + + # Check no catastrophic outliers (> 200ms) for idx, interval in enumerate(intervals): - assert abs(interval - 0.05) < 0.01, ( - f"Interval {idx} was {interval*1000:.1f}ms, expected ~50ms (±10ms)" + assert interval < 0.200, ( + f"Interval {idx} was {interval*1000:.1f}ms, " + "which is catastrophically late (expected < 200ms)" ) + + # Check that most intervals are within reasonable bounds (±30ms of 50ms) + # This allows for CI load variability while still validating timer correctness + reasonable_count = sum(1 for i in intervals if abs(i - 0.050) < 0.030) + assert reasonable_count >= 7, ( + f"Only {reasonable_count}/9 intervals were within ±30ms of target. " + f"Intervals: {[f'{i*1000:.1f}ms' for i in intervals]}" + ) def test_slow_callback_no_protocol_impact(feeder): @@ -167,10 +183,15 @@ def hammer(): assert not errors, f"Exceptions during concurrent timer ops: {errors}" -@pytest.mark.skip(reason=("This test is flaky and may fail on slow CI machines;\n" - "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.""" + """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() @@ -200,8 +221,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 +230,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)" ) From b73e47508920b033ce5e347aeb2ba85d7d74503c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Fri, 19 Jun 2026 17:27:24 +0200 Subject: [PATCH 11/13] fix: enable previously skipped timer and memory_access tests with relaxed CI-friendly assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_timer_no_drift: changed from strict ±10ms interval validation to checking timer completes all 10 callbacks and avoids extreme outliers (>1s) - test_memory_access_event_latency: increased timeout from 50ms to 500ms to accommodate slow CI machine scheduler variance Both tests remain meaningful while being reliable on variable-load CI systems. --- test/test_threading.py | 44 ++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/test/test_threading.py b/test/test_threading.py index 169d0a0..7cd4b5d 100644 --- a/test/test_threading.py +++ b/test/test_threading.py @@ -51,13 +51,15 @@ def _wait_no_threads_named(name, timeout=0.5): return False def test_timer_no_drift(): - """Verify heapq-based timer fires at consistent 50ms intervals without extreme drift. + """Verify heapq-based timer fires reliably and doesn't deadlock. - This test validates that the timer thread does not accumulate significant drift - over multiple firings. Rather than enforcing strict timing (which is unreliable - on slow CI machines), we check that: - - Most intervals are reasonably close to the target (within ±30ms) - - No single interval is catastrophically late (> 200ms) + 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 = [] @@ -71,27 +73,27 @@ def callback(cookie): return True # reschedule ecu.add_timer(0.050, callback) - fired = done.wait(timeout=5.0) # increased timeout for slow machines + fired = done.wait(timeout=10.0) # generous timeout for very slow CI ecu.stop() - assert fired, "Timer did not fire 10 times within 5 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)] - # Check no catastrophic outliers (> 200ms) - for idx, interval in enumerate(intervals): - assert interval < 0.200, ( - f"Interval {idx} was {interval*1000:.1f}ms, " - "which is catastrophically late (expected < 200ms)" - ) + # 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." + ) - # Check that most intervals are within reasonable bounds (±30ms of 50ms) - # This allows for CI load variability while still validating timer correctness - reasonable_count = sum(1 for i in intervals if abs(i - 0.050) < 0.030) - assert reasonable_count >= 7, ( - f"Only {reasonable_count}/9 intervals were within ±30ms of target. " - f"Intervals: {[f'{i*1000:.1f}ms' for i in intervals]}" + # 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)" ) From 6bf6bce97f5b6969292ab3bfaea9ccd2d7ce5dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Tue, 23 Jun 2026 12:48:20 +0200 Subject: [PATCH 12/13] Add optional lint dependencies helps with pip install -e ".[test,lint]" --- pyproject.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index d000735..a969913 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,10 @@ dependencies = [ test = [ "pytest >= 6.2.5", ] +lint = [ + "ruff", + "pyright", +] [project.urls] Homepage = "https://github.com/RaulSMS/python-can-j1939" From f6282f3984b867bcbfcc0929ff4e7873039f3d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Sainz-Maza=20Serna?= Date: Wed, 24 Jun 2026 10:44:25 +0200 Subject: [PATCH 13/13] fix: ruff errors introduced on PR#39 merge during the merge commit 4d1baeceb4360037d6e2f6f14ee9ae9866bc00e1 to update this branch to latest master I overlooked some already fixed errors. --- j1939/Dm14Query.py | 13 ++++++------- j1939/Dm14Server.py | 21 ++++++++++----------- j1939/controller_application.py | 5 +++-- j1939/j1939_22.py | 5 +---- j1939/memory_access.py | 9 ++++----- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/j1939/Dm14Query.py b/j1939/Dm14Query.py index 84904d3..54c51f9 100644 --- a/j1939/Dm14Query.py +++ b/j1939/Dm14Query.py @@ -1,9 +1,8 @@ from __future__ import annotations -from collections.abc import Callable -from enum import Enum -from typing import Optional + import queue +from collections.abc import Callable from enum import Enum import j1939 @@ -45,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 634011d..f8039fd 100644 --- a/j1939/Dm14Server.py +++ b/j1939/Dm14Server.py @@ -1,10 +1,9 @@ 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 @@ -30,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 = [] @@ -184,8 +183,8 @@ def _send_dm15( object_count: int, sa: int, 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, - edcp: int = None, + error: int | None = None, + edcp: int | None = None, ) -> None: """ @@ -383,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 @@ -408,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/controller_application.py b/j1939/controller_application.py index 46164f8..ff3c528 100644 --- a/j1939/controller_application.py +++ b/j1939/controller_application.py @@ -1,6 +1,7 @@ from __future__ import annotations + import logging -from typing import Optional + import j1939 from .message_id import FrameFormat @@ -55,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 diff --git a/j1939/j1939_22.py b/j1939/j1939_22.py index ea677fc..1a0811c 100644 --- a/j1939/j1939_22.py +++ b/j1939/j1939_22.py @@ -1,9 +1,7 @@ -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 @@ -638,7 +636,6 @@ def _process_tp_dt(self, mid, dest_address, data, timestamp): return src_address = mid.source_address - 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) diff --git a/j1939/memory_access.py b/j1939/memory_access.py index 7ad9e1a..890c65f 100644 --- a/j1939/memory_access.py +++ b/j1939/memory_access.py @@ -1,9 +1,8 @@ from __future__ import annotations -from collections.abc import Callable -from enum import Enum + import logging -from typing import Optional import threading +from collections.abc import Callable from enum import Enum import j1939 @@ -251,11 +250,11 @@ def _listen_for_dm14( def respond( self, proceed: bool, - data: Optional[list] = None, + data: list | None = None, error: int = 0xFFFFFF, edcp: int = 0xFF, max_timeout: int = 3, - ) -> Optional[list]: + ) -> list | None: """ Responds with requested data and error code, if applicable, to a read request