diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..696b5d7 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + + - package-ecosystem: "pip" + directory: "/" + schedule: + interval: "weekly" diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index ec5d647..a7df81b 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -1,46 +1,79 @@ -# This is a basic workflow to help you get started with Actions - name: CI -# Controls when the workflow will run +permissions: + contents: read + on: push: branches: - master - develop - tags: - - v* pull_request: branches: - "**" schedule: - # Daily at 10:55 - - cron: '55 10 * * *' - - # Allows you to run this workflow manually from the Actions tab + - cron: '55 10 * * 0' workflow_dispatch: -# 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@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@v7.0.0 + with: + python-version: '3.10' + cache: 'pip' # Safely caches dependencies + + - name: Install dependencies + run: pip install -e .[lint] + + - name: Check style and syntax (Ruff) + run: ruff check . + + - name: Check minimum Python version compatibility (Vermin) + run: vermin --target=3.10- --no-parse-comments --no-tips j1939/ + + - name: Run Pyright + run: pyright + test: - # The type of runner that the job will run on runs-on: ${{ matrix.os }} strategy: matrix: os: [ubuntu-latest, macos-latest, windows-latest] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it - - uses: actions/setup-python@v4 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@v7.0.0 with: - python-version: '3.10' - - - uses: actions/checkout@v2 + python-version: ${{ matrix.python-version }} + cache: 'pip' - name: install dependencies - run: pip3 install . + run: pip3 install -e .[test] - name: Run tests run: pytest . --pyargs + build_check: + name: Verify Package Build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@v7.0.0 + with: + python-version: '3.12' + cache: 'pip' + + - name: Install build tools + run: pip install build twine + + - name: Verify package build and metadata + run: | + python -m build + python -m twine check dist/* \ No newline at end of file diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml new file mode 100644 index 0000000..0b51c7f --- /dev/null +++ b/.github/workflows/publish.yml @@ -0,0 +1,60 @@ +name: Publish Release + +on: + push: + tags: + - v* + +permissions: + contents: read + +jobs: + build_and_publish: + name: Build, Release, and Publish + runs-on: ubuntu-latest + outputs: + hashes: ${{ steps.hash.outputs.hashes }} + permissions: + id-token: write # Required for PyPI OIDC and SLSA provenance + contents: write # Required to create a GitHub Release + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Python + uses: actions/setup-python@v7.0.0 + with: + python-version: '3.12' + + - name: Install build tools + run: pip install build + + - name: Build sdist and wheel + run: python -m build + + - name: Generate hashes for SLSA + id: hash + run: | + cd dist && echo "hashes=$(sha256sum * | base64 -w0)" >> "$GITHUB_OUTPUT" + + - name: Create GitHub Release + uses: softprops/action-gh-release@v3 + with: + files: | + dist/*.tar.gz + dist/*.whl + generate_release_notes: true + + - name: Publish package distributions to PyPI + uses: pypa/gh-action-pypi-publish@release/v1 + + provenance: + needs: [build_and_publish] + permissions: + actions: read + id-token: write + contents: write + uses: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml@v2.1.0 + with: + base64-subjects: "${{ needs.build_and_publish.outputs.hashes }}" + upload-assets: true diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 0000000..debddec --- /dev/null +++ b/.github/workflows/scorecard.yml @@ -0,0 +1,78 @@ +# This workflow uses actions that are not certified by GitHub. They are provided +# by a third-party and are governed by separate terms of service, privacy +# policy, and support documentation. + +name: Scorecard supply-chain security +on: + # For Branch-Protection check. Only the default branch is supported. See + # https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection + branch_protection_rule: + # To guarantee Maintained check is occasionally updated. See + # https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained + schedule: + - cron: '23 10 * * 4' + push: + branches: [ "master" ] + +# Declare default permissions as read only. +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-latest + # `publish_results: true` only works when run from the default branch. conditional can be removed if disabled. + if: github.event.repository.default_branch == github.ref_name || github.event_name == 'pull_request' + permissions: + # Needed to upload the results to code-scanning dashboard. + security-events: write + # Needed to publish results and get a badge (see publish_results below). + id-token: write + # Uncomment the permissions below if installing in a private repository. + # contents: read + # actions: read + + steps: + - name: "Checkout code" + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: "Run analysis" + uses: ossf/scorecard-action@2d1146689b8cda280b9bc96326124645441f03bc # v2.4.4 + with: + results_file: results.sarif + results_format: sarif + # (Optional) "write" PAT token. Uncomment the `repo_token` line below if: + # - you want to enable the Branch-Protection check on a *public* repository, or + # - you are installing Scorecard on a *private* repository + # To create the PAT, follow the steps in https://github.com/ossf/scorecard-action?tab=readme-ov-file#authentication-with-fine-grained-pat-optional. + # repo_token: ${{ secrets.SCORECARD_TOKEN }} + + # Public repositories: + # - Publish results to OpenSSF REST API for easy access by consumers + # - Allows the repository to include the Scorecard badge. + # - See https://github.com/ossf/scorecard-action#publishing-results. + # For private repositories: + # - `publish_results` will always be set to `false`, regardless + # of the value entered here. + publish_results: true + + # (Optional) Uncomment file_mode if you have a .gitattributes with files marked export-ignore + # file_mode: git + + # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF + # format to the repository Actions tab. + - name: "Upload artifact" + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + # Upload the results to GitHub's code scanning dashboard (optional). + # Commenting out will disable upload of results to your repo's Code Scanning dashboard + - name: "Upload to code-scanning" + uses: github/codeql-action/upload-sarif@v4 + with: + sarif_file: results.sarif diff --git a/.gitignore b/.gitignore index f70d166..2729017 100644 --- a/.gitignore +++ b/.gitignore @@ -63,6 +63,9 @@ instance/ # Sphinx documentation docs/_build/ +docs/examples.rst +docs/source/j1939.rst +docs/source/modules.rst # PyBuilder target/ diff --git a/.readthedocs.yaml b/.readthedocs.yaml new file mode 100644 index 0000000..32f1a01 --- /dev/null +++ b/.readthedocs.yaml @@ -0,0 +1,20 @@ +# Read the Docs configuration file +# See https://docs.readthedocs.io/en/stable/config-file/v2.html for details + +# Required +version: 2 + +# Set the OS, Python version, and other tools you might need +build: + os: ubuntu-24.04 + tools: + python: "3.12" + +# Build documentation in the "docs/" directory with Sphinx +sphinx: + configuration: docs/conf.py + +# Declare the Python requirements required to build your documentation +python: + install: + - requirements: docs/requirements.txt diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..8fa2e5e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,78 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project + +`can-j1939` is a Python implementation of the SAE J1939 protocol stack on top of +[python-can](https://python-can.readthedocs.org/). It supports both J1939-21 and J1939-22 (J1939-FD) +data link layers, including transport protocols (BAM, CMDT / RTS-CTS), address claiming, and a +number of diagnostic messages (DM1, DM11, DM14, DM22). + +## Common commands + +```bash +# Install the package (editable for development) +pip install -e . + +# Run the full test suite (matches CI) +pytest . --pyargs + +# Run a single test file / test +pytest test/test_ecu.py +pytest test/test_memory_access.py::TestMemoryAccess::test_some_name -v +``` + +CI runs `pytest . --pyargs` on Python 3.10 across Ubuntu/macOS/Windows +(`.github/workflows/CI.yml`). + +## Architecture + +The stack is layered: an **ECU** owns a **data-link layer** object and one or more +**ControllerApplications**. Background work runs on a dedicated job thread. + +- `j1939/electronic_control_unit.py` — `ElectronicControlUnit` is the entry point. It owns the + `can.Bus`, a `MessageListener`, a job thread (`_async_job_thread`) that drives timers and + transport-protocol timeouts, and a list of subscribers. The `data_link_layer` constructor arg + (`'j1939-21'` or `'j1939-22'`) selects which DLL is instantiated. The ECU passes the DLL a + small surface of callbacks: `send_message`, `_job_thread_wakeup`, `_notify_subscribers`, + `_is_message_acceptable`. For tests, `send_message=` can be injected to bypass real CAN I/O. +- `j1939/j1939_21.py` and `j1939/j1939_22.py` — the two DLL implementations. They share the + callback signature above and implement the transport protocols (TP-BAM, TP-CMDT / RTS-CTS, + and for J1939-22 the FD multi-session variants and Multi-PG / FEFF). Changes that touch + protocol behaviour usually need parallel updates in both files. +- `j1939/controller_application.py` — `ControllerApplication` (CA) implements J1939/81 address + claiming, state machine (`NONE` → `WAITING_VETO` → `NORMAL` / `CANNOT_CLAIM`), per-CA + subscriptions, and `send_pgn` (which dispatches to the ECU's DLL). +- `j1939/name.py`, `j1939/parameter_group_number.py`, `j1939/message_id.py` — value objects for + the J1939 NAME, PGN encoding, and 29-bit CAN identifier framing. +- `j1939/diagnostic_messages.py`, `j1939/memory_access.py`, `j1939/Dm14Query.py`, + `j1939/Dm14Server.py`, `j1939/error_info.py` — diagnostic-message support (DM1/DM11/DM14/DM22), + including the DM14 memory-access client (`Dm14Query`) and server (`Dm14Server`). +- `j1939/__init__.py` is the public API surface — anything users are expected to import lives + here. + +### Threading model + +All I/O and protocol timing flows through the ECU's job thread. The DLL never blocks on I/O +itself — it enqueues work and calls `_job_thread_wakeup` to nudge the thread. Callbacks +registered via `ca.subscribe(...)` or `ca.add_timer(...)` run on that job thread, so they +must not block. + +### Tests + +- `test/` holds unit tests. `test/helpers/feeder.py` provides the `Feeder` fixture (registered + in `test/conftest.py`) which is the standard way to drive the stack from tests: it + replaces `ElectronicControlUnit.send_message` with a simulated bus, lets the test queue + expected RX/TX messages and PDUs in order, and asserts that the stack produces the expected + TX sequence. New protocol-level tests should follow that pattern instead of mocking + `python-can` directly. +- `test/helpers/feeder.AcceptAllCA` is a CA subclass with `message_acceptable` overridden to + accept everything — use it when a test needs to receive peer-to-peer messages without setting + up a real claim. + +### Examples + +`examples/` contains runnable scripts mirroring the README quick-start (simple receive, own CA +producer, transport protocols, multi-PG, diagnostic messages). When adding a new public +feature, prefer extending an existing example over inventing a new pattern in the docs. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..80c816e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,92 @@ +# Contributing to python-can-j1939 + +Thank you for your interest in contributing! Please read this guide before opening a pull request. + +## Requirements + +- **Python 3.10 or later** — the codebase uses `match`/`case` (PEP 622) and `X | Y` union types (PEP 604). + +## Setting up a development environment + +```bash +git clone https://github.com/RaulSMS/python-can-j1939.git +cd python-can-j1939 + +# Install the package in editable mode with test and lint dependencies +pip install -e ".[test,lint]" +``` + +## Running the tests + +```bash +pytest . --pyargs +``` + +All tests must pass before submitting a pull request. CI runs the full suite on Python 3.10–3.13 across Ubuntu, macOS, and Windows. + +## Code style + +This project uses [ruff](https://docs.astral.sh/ruff/) to enforce a consistent style (rules `E` and `F`). + +Check your changes before committing: + +```bash +ruff check . +``` + +Fix violations before opening a PR — the CI lint job will reject any remaining issues. + +Key rules enforced: + +- Use `is None` / `is not None` instead of `== None` / `!= None`. +- Use truthiness checks (`if x:`) instead of `== True` / `== False`. +- Remove unused imports and variables. +- No multiple statements on one line (no `if x: do_something()`). + +## Branching and commits + +- Branch off `master` for new features and bug fixes. +- Use descriptive branch names: `fix/some-bug`, `feature/new-thing`, `docs/update-readme`. +- Keep commits focused — one logical change per commit. +- Write commit messages in the imperative mood: `fix transport protocol timeout`, not `fixed timeout`. + +## Type checking + +This project uses [Pyright](https://github.com/microsoft/pyright) for static type analysis. + +```bash +pyright +``` + +Configuration is in `pyrightconfig.json` (covers `j1939/` only, `basic` mode). Fix any new errors introduced by your change before opening a PR. + +## Pull request checklist + +- [ ] Tests pass: `pytest . --pyargs` +- [ ] No lint violations: `ruff check .` +- [ ] No type errors: `pyright` +- [ ] New protocol behaviour is covered by tests in `test/` using the `Feeder` fixture (see `test/helpers/feeder.py`). +- [ ] Changes that affect both J1939-21 and J1939-22 are applied to **both** `j1939/j1939_21.py` and `j1939/j1939_22.py`. +- [ ] Public API additions are exported from `j1939/__init__.py`. + +## Release Process + +Releases are fully automated via GitHub Actions CI/CD pipelines but are strictly gated to maintainers to preserve package security. + +### Requesting a New Release +If you are a contributor and believe a new version should be published (e.g., after a significant feature addition or bug fix has landed on master): +1. Open a new Issue on GitHub requesting a release. +2. Assign the issue to the project maintainer (RaulSMS). +3. The maintainer will review the state of the master branch and initiate the release deployment sequence. + +### Maintainer Deployment Sequence (For Reference) +Only RaulSMS has permission to publish releases to PyPI. The steps are: +1. Update the version string inside j1939/version.py on the master branch. +2. Push a semantic version tag matching the v* pattern: + git tag v2.1.0 + git push origin v2.1.0 +3. The CI/CD system will automatically catch the tag push, execute all tests, generate a GitHub Release with an automated changelog, and securely upload the package distributions to PyPI. + +## Architecture overview + +See [CLAUDE.md](CLAUDE.md) for a detailed description of the layered architecture (ECU → DLL → ControllerApplication), the threading model, and pointers to each module. diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 0000000..c1a7121 --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,2 @@ +include LICENSE +include README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..bde0939 --- /dev/null +++ b/README.md @@ -0,0 +1,298 @@ +# SAE J1939 for Python + +[![Latest Version on PyPi](https://img.shields.io/pypi/v/python-can-j1939)](https://pypi.org/project/python-can-j1939/) +[![Documentation build Status](https://readthedocs.org/projects/python-can-j1939/badge/?version=latest)](https://python-can-j1939.readthedocs.io/en/latest/) + +An implementation of the CAN SAE J1939 standard for Python. This is the +first J1939-22 (J1939-FD) implementation! + +If you experience a problem or think the stack would not behave +properly, do not hesitate to open a ticket or write an email. +Pull Requests (PR) are of course even more welcome! + +The project uses the +[python-can](https://python-can.readthedocs.org/en/stable/) package to +support multiple hardware drivers. At the time of writing the supported +interfaces are + +- CAN over Serial +- CAN over Serial / SLCAN +- CANalyst-II +- IXXAT Virtual CAN Interface +- Kvasers CANLIB +- NEOVI Interface +- NI-CAN +- PCAN Basic API +- Socketcan +- SYSTEC interface +- USB2CAN Interface +- Vector +- Virtual +- isCAN + +## Overview + +An SAE J1939 CAN Network consists of multiple Electronic Control Units +(ECUs). Each ECU can have one or more Controller Applications (CAs). +Each CA has its own (unique) Address on the bus. This address is either +acquired within the address claiming procedure or set to a fixed value. +In the latter case, the CA has to announce its address to the bus to +check whether it is free. + +The CAN messages in a SAE J1939 network are called Protocol Data Units +(PDUs). This definition is not completely correct, but close enough to +think of PDUs as the CAN messages. + +## Features + +- one ElectronicControlUnit (ECU) can hold multiple + ControllerApplications (CA) +- ECU (CA) Naming according SAE J1939/81 +- full featured address claiming procedure according SAE J1939/81 +- full support of transport protocol (up to 1785 bytes) according SAE + J1939/21 for sending and receiving + - Connection Mode Data Transfers (CMDT) + - Broadcast Announce Message (BAM) +- support of Multi-PG according SAE J1939/22 + - currently FEFF (Flexible Data Rate Extended Frame Format) + supported only +- full support of fd-transport protocol according SAE J1939/22 + (J1939-FD) for sending and receiving + - RTS/CTS (Destination Specific) Transfer with up to 8 concurrent + sessions and up to 16777215 bytes of data per session + - Broadcast Announce Message (BAM) with up to 4 concurrent + sessions and up to 15300 bytes of data per session +- Requests (global and specific) +- correct timeout and deadline handling +- (under construction) almost complete testcoverage +- diagnostic messages (see + ) + - support of DM1 Tool and ECU functionaliy (all four SAE J1939-73 + SPN conversion methods: 1, 2, 3, 4) + - support of DM11 Tool functionaliy + - support of DM22 Tool functionaliy + +## Installation + +Requires **Python 3.10 or later** and python-can_ >= 4.2.0. + +Install python-can-j1939 with pip: + + pip install python-can-j1939 + +or do the trick with: + + git clone https://github.com/RaulSMS/python-can-j1939.git + cd python-can-j1939 + pip install . + +## Upgrade + +Upgrade an already installed python-can-j1939 package: + + pip install --upgrade python-can-j1939 + +## Quick start + +To simply receive all passing (public) messages on the bus you can +subscribe to the ECU object. + +``` python +import logging +import time +import can +import j1939 + +logging.getLogger('j1939').setLevel(logging.DEBUG) +logging.getLogger('can').setLevel(logging.DEBUG) + +def on_message(priority, pgn, sa, timestamp, data): + """Receive incoming messages from the bus + + :param int priority: + Priority of the message + :param int pgn: + Parameter Group Number of the message + :param int sa: + Source Address of the message + :param int timestamp: + Timestamp of the message + :param bytearray data: + Data of the PDU + """ + print("PGN {} length {}".format(pgn, len(data))) + +def main(): + print("Initializing") + + # create the ElectronicControlUnit (one ECU can hold multiple ControllerApplications) + ecu = j1939.ElectronicControlUnit() + + # Connect to the CAN bus + # Arguments are passed to python-can's can.interface.Bus() constructor + # (see https://python-can.readthedocs.io/en/stable/bus.html). + # ecu.connect(bustype='socketcan', channel='can0') + # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) + ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) + # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + + # subscribe to all (global) messages on the bus + ecu.subscribe(on_message) + + time.sleep(120) + + print("Deinitializing") + ecu.disconnect() + +if __name__ == '__main__': + main() +``` + +A more sophisticated example in which the CA class was overloaded to +include its own functionality: + +``` python +import logging +import time +import can +import j1939 + +logging.getLogger('j1939').setLevel(logging.DEBUG) +logging.getLogger('can').setLevel(logging.DEBUG) + +# compose the name descriptor for the new ca +name = j1939.Name( + arbitrary_address_capable=0, + industry_group=j1939.Name.IndustryGroup.Industrial, + vehicle_system_instance=1, + vehicle_system=1, + function=1, + function_instance=1, + ecu_instance=1, + manufacturer_code=666, + identity_number=1234567 + ) + +# create the ControllerApplications +ca = j1939.ControllerApplication(name, 128) + + +def ca_receive(priority, pgn, source, timestamp, data): + """Feed incoming message to this CA. + (OVERLOADED function) + :param int priority: + Priority of the message + :param int pgn: + Parameter Group Number of the message + :param intsa: + Source Address of the message + :param int timestamp: + Timestamp of the message + :param bytearray data: + Data of the PDU + """ + print("PGN {} length {}".format(pgn, len(data))) + +def ca_timer_callback1(cookie): + """Callback for sending messages + + This callback is registered at the ECU timer event mechanism to be + executed every 500ms. + + :param cookie: + A cookie registered at 'add_timer'. May be None. + """ + # wait until we have our device_address + if ca.state != j1939.ControllerApplication.State.NORMAL: + # returning true keeps the timer event active + return True + + # create data with 8 bytes + data = [j1939.ControllerApplication.FieldValue.NOT_AVAILABLE_8] * 8 + + # sending normal broadcast message + ca.send_pgn(0, 0xFD, 0xED, 6, data) + + # sending normal peer-to-peer message, destintion address is 0x04 + ca.send_pgn(0, 0xE0, 0x04, 6, data) + + # returning true keeps the timer event active + return True + + +def ca_timer_callback2(cookie): + """Callback for sending messages + + This callback is registered at the ECU timer event mechanism to be + executed every 500ms. + + :param cookie: + A cookie registered at 'add_timer'. May be None. + """ + # wait until we have our device_address + if ca.state != j1939.ControllerApplication.State.NORMAL: + # returning true keeps the timer event active + return True + + # create data with 100 bytes + data = [j1939.ControllerApplication.FieldValue.NOT_AVAILABLE_8] * 100 + + # sending multipacket message with TP-BAM + ca.send_pgn(0, 0xFE, 0xF6, 6, data) + + # sending multipacket message with TP-CMDT, destination address is 0x05 + ca.send_pgn(0, 0xD0, 0x05, 6, data) + + # returning true keeps the timer event active + return True + +def main(): + print("Initializing") + + # create the ElectronicControlUnit (one ECU can hold multiple ControllerApplications) + ecu = j1939.ElectronicControlUnit() + + # Connect to the CAN bus + # Arguments are passed to python-can's can.interface.Bus() constructor + # (see https://python-can.readthedocs.io/en/stable/bus.html). + # ecu.connect(bustype='socketcan', channel='can0') + # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) + ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) + # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + # ecu.connect('testchannel_1', bustype='virtual') + + # add CA to the ECU + ecu.add_ca(controller_application=ca) + ca.subscribe(ca_receive) + # callback every 0.5s + ca.add_timer(0.500, ca_timer_callback1) + # callback every 5s + ca.add_timer(5, ca_timer_callback2) + # by starting the CA it starts the address claiming procedure on the bus + ca.start() + + time.sleep(120) + + print("Deinitializing") + ca.stop() + ecu.disconnect() + +if __name__ == '__main__': + main() +``` + +## Credits + +This package is a fork of +[can-j1939](https://github.com/juergenH87/python-can-j1939) by Juergen +Heilgemeir, who greatly extended the original work and added J1939-22 +(J1939-FD) support. + +The original implementation was taken from + by Frank Benkert. + +Thanks to all contributors for their great work! diff --git a/README.rst b/README.rst deleted file mode 100644 index 478b555..0000000 --- a/README.rst +++ /dev/null @@ -1,303 +0,0 @@ -SAE J1939 for Python -==================== - -|release| |docs| - -.. |release| image:: https://img.shields.io/pypi/v/can-j1939 - :target: https://pypi.python.org/pypi/can-j1939/ - :alt: Latest Version on PyPi - -.. |docs| image:: https://readthedocs.org/projects/j1939/badge/?version=latest - :target: https://j1939.readthedocs.io/en/latest/ - :alt: Documentation build Status - - -An implementation of the CAN SAE J1939 standard for Python. -This is the first J1939-22 (J1939-FD) implementation! - -If you experience a problem or think the stack would not behave properly, do -not hesitate to open a ticket or write an email. -Pullrequests are of course even more welcome! - -The project uses the python-can_ package to support multiple hardware drivers. -At the time of writing the supported interfaces are - -* CAN over Serial -* CAN over Serial / SLCAN -* CANalyst-II -* IXXAT Virtual CAN Interface -* Kvasers CANLIB -* NEOVI Interface -* NI-CAN -* PCAN Basic API -* Socketcan -* SYSTEC interface -* USB2CAN Interface -* Vector -* Virtual -* isCAN - -Overview --------- - -An SAE J1939 CAN Network consists of multiple Electronic Control Units (ECUs). -Each ECU can have one or more Controller Applications (CAs). Each CA has its -own (unique) Address on the bus. This address is either acquired within the -address claiming procedure or set to a fixed value. In the latter case, the CA -has to announce its address to the bus to check whether it is free. - -The CAN messages in a SAE J1939 network are called Protocol Data Units (PDUs). -This definition is not completely correct, but close enough to think of PDUs -as the CAN messages. - - -Features --------- - -* one ElectronicControlUnit (ECU) can hold multiple ControllerApplications (CA) -* ECU (CA) Naming according SAE J1939/81 -* full featured address claiming procedure according SAE J1939/81 -* full support of transport protocol (up to 1785 bytes) according SAE J1939/21 for sending and receiving - - - Connection Mode Data Transfers (CMDT) - - Broadcast Announce Message (BAM) -* support of Multi-PG according SAE J1939/22 - - currently FEFF (Flexible Data Rate Extended Frame Format) supported only -* full support of fd-transport protocol according SAE J1939/22 (J1939-FD) for sending and receiving - - - RTS/CTS (Destination Specific) Transfer with up to 8 concurrent sessions and up to 16777215 bytes of data per session - - Broadcast Announce Message (BAM) with up to 4 concurrent sessions and up to 15300 bytes of data per session - -* Requests (global and specific) -* correct timeout and deadline handling -* (under construction) almost complete testcoverage -* diagnostic messages (see https://github.com/juergenH87/python-can-j1939/tree/master/examples/diagnostic_message.py) - - support of DM1 Tool and ECU functionaliy - - support of DM11 Tool functionaliy - - support of DM22 Tool functionaliy - - -Installation ------------- - -Install can-j1939 with pip:: - - $ pip install can-j1939 - -or do the trick with:: - - $ git clone https://github.com/juergenH87/can-j1939.git - $ cd j1939 - $ pip install . - -Upgrade ------------- - -Upgrade an already installed can-j1939 package:: - - $ pip install --upgrade can-j1939 - - -Quick start ------------ - -To simply receive all passing (public) messages on the bus you can subscribe to the ECU object. - -.. code-block:: python - - import logging - import time - import can - import j1939 - - logging.getLogger('j1939').setLevel(logging.DEBUG) - logging.getLogger('can').setLevel(logging.DEBUG) - - def on_message(priority, pgn, sa, timestamp, data): - """Receive incoming messages from the bus - - :param int priority: - Priority of the message - :param int pgn: - Parameter Group Number of the message - :param int sa: - Source Address of the message - :param int timestamp: - Timestamp of the message - :param bytearray data: - Data of the PDU - """ - print("PGN {} length {}".format(pgn, len(data))) - - def main(): - print("Initializing") - - # create the ElectronicControlUnit (one ECU can hold multiple ControllerApplications) - ecu = j1939.ElectronicControlUnit() - - # Connect to the CAN bus - # Arguments are passed to python-can's can.interface.Bus() constructor - # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) - - # subscribe to all (global) messages on the bus - ecu.subscribe(on_message) - - time.sleep(120) - - print("Deinitializing") - ecu.disconnect() - - if __name__ == '__main__': - main() - -A more sophisticated example in which the CA class was overloaded to include its own functionality: - -.. code-block:: python - - import logging - import time - import can - import j1939 - - logging.getLogger('j1939').setLevel(logging.DEBUG) - logging.getLogger('can').setLevel(logging.DEBUG) - - # compose the name descriptor for the new ca - name = j1939.Name( - arbitrary_address_capable=0, - industry_group=j1939.Name.IndustryGroup.Industrial, - vehicle_system_instance=1, - vehicle_system=1, - function=1, - function_instance=1, - ecu_instance=1, - manufacturer_code=666, - identity_number=1234567 - ) - - # create the ControllerApplications - ca = j1939.ControllerApplication(name, 128) - - - def ca_receive(priority, pgn, source, timestamp, data): - """Feed incoming message to this CA. - (OVERLOADED function) - :param int priority: - Priority of the message - :param int pgn: - Parameter Group Number of the message - :param intsa: - Source Address of the message - :param int timestamp: - Timestamp of the message - :param bytearray data: - Data of the PDU - """ - print("PGN {} length {}".format(pgn, len(data))) - - def ca_timer_callback1(cookie): - """Callback for sending messages - - This callback is registered at the ECU timer event mechanism to be - executed every 500ms. - - :param cookie: - A cookie registered at 'add_timer'. May be None. - """ - # wait until we have our device_address - if ca.state != j1939.ControllerApplication.State.NORMAL: - # returning true keeps the timer event active - return True - - # create data with 8 bytes - data = [j1939.ControllerApplication.FieldValue.NOT_AVAILABLE_8] * 8 - - # sending normal broadcast message - ca.send_pgn(0, 0xFD, 0xED, 6, data) - - # sending normal peer-to-peer message, destintion address is 0x04 - ca.send_pgn(0, 0xE0, 0x04, 6, data) - - # returning true keeps the timer event active - return True - - - def ca_timer_callback2(cookie): - """Callback for sending messages - - This callback is registered at the ECU timer event mechanism to be - executed every 500ms. - - :param cookie: - A cookie registered at 'add_timer'. May be None. - """ - # wait until we have our device_address - if ca.state != j1939.ControllerApplication.State.NORMAL: - # returning true keeps the timer event active - return True - - # create data with 100 bytes - data = [j1939.ControllerApplication.FieldValue.NOT_AVAILABLE_8] * 100 - - # sending multipacket message with TP-BAM - ca.send_pgn(0, 0xFE, 0xF6, 6, data) - - # sending multipacket message with TP-CMDT, destination address is 0x05 - ca.send_pgn(0, 0xD0, 0x05, 6, data) - - # returning true keeps the timer event active - return True - - def main(): - print("Initializing") - - # create the ElectronicControlUnit (one ECU can hold multiple ControllerApplications) - ecu = j1939.ElectronicControlUnit() - - # Connect to the CAN bus - # Arguments are passed to python-can's can.interface.Bus() constructor - # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) - # ecu.connect('testchannel_1', bustype='virtual') - - # add CA to the ECU - ecu.add_ca(controller_application=ca) - ca.subscribe(ca_receive) - # callback every 0.5s - ca.add_timer(0.500, ca_timer_callback1) - # callback every 5s - ca.add_timer(5, ca_timer_callback2) - # by starting the CA it starts the address claiming procedure on the bus - ca.start() - - time.sleep(120) - - print("Deinitializing") - ca.stop() - ecu.disconnect() - - if __name__ == '__main__': - main() - - -Credits -------- -This implementation was taken from https://github.com/benkfra/j1939, as no further development took place. - -Thanks for your great work! - - - -.. _python-can: https://python-can.readthedocs.org/en/stable/ -.. _Copperhill technologies: http://copperhilltech.com/a-brief-introduction-to-the-sae-j1939-protocol/ diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..85e85f5 --- /dev/null +++ b/conftest.py @@ -0,0 +1,17 @@ +import pytest + +from test.helpers.feeder import Feeder + + +@pytest.fixture() +def feeder(): + # setup + feeder = Feeder() + try: + yield feeder + finally: + # teardown — guarantee cleanup even if the test raises + try: + feeder.stop() + except Exception: + pass diff --git a/docs/conf.py b/docs/conf.py index 8115e78..8dacada 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- # # Configuration file for the Sphinx documentation builder. # @@ -12,10 +11,53 @@ # add these directories to sys.path here. If the directory is relative to the # documentation root, use os.path.abspath to make it absolute, like shown here. # -import os +import subprocess import sys -sys.path.insert(0, os.path.abspath('.')) -sys.path.insert(0, os.path.abspath('../')) +from pathlib import Path + +HERE = Path(__file__).parent + +sys.path.insert(0, str(HERE)) +sys.path.insert(0, str(HERE.parent)) + +# Auto-generate API docs from the j1939 package so new modules appear automatically. +subprocess.run( + [sys.executable, '-m', 'sphinx.ext.apidoc', + '-o', str(HERE / 'source'), + str(HERE.parent / 'j1939'), + '--force', '--module-first'], + check=True, +) + +# Auto-generate examples.rst from all .py files found under examples/. +def _generate_examples_rst(): + examples_dir = HERE.parent / 'examples' + lines = [ + 'Examples', + '========', + '', + 'Example scripts demonstrating how to use the python-can-j1939 library.', + '', + ] + for path in sorted(examples_dir.rglob('*.py')): + rel = path.relative_to(HERE.parent) + title = path.stem.replace('_', ' ').title() + lines += [ + title, + '-' * len(title), + '', + f'.. literalinclude:: ../{rel.as_posix()}', + ' :language: python', + f' :caption: {path.name}', + '', + ] + (HERE / 'examples.rst').write_text('\n'.join(lines), encoding='utf-8') + +_generate_examples_rst() + +# Mock dependencies to allow autodoc to work without external packages +# This is needed because python-can requires sqlite3 which may not be available +autodoc_mock_imports = ['can', 'can.typechecking', 'numpy'] # -- Project information ----------------------------------------------------- @@ -41,8 +83,40 @@ # ones. extensions = [ 'sphinx.ext.autodoc', + 'sphinx.ext.napoleon', # Support for Google/NumPy style docstrings + 'sphinx.ext.viewcode', # Add links to source code + 'sphinx.ext.intersphinx', # Link to other project's documentation ] +# Autodoc configuration for comprehensive API documentation +autodoc_default_options = { + 'members': True, + 'undoc-members': True, + 'show-inheritance': True, + 'special-members': '__init__', + 'inherited-members': True, + 'member-order': 'bysource', +} + +# Include both class docstring and __init__ docstring +autoclass_content = 'both' + +# Napoleon settings for docstring parsing +napoleon_google_docstring = True +napoleon_numpy_docstring = True +napoleon_include_init_with_doc = True +napoleon_include_private_with_doc = False +napoleon_include_special_with_doc = True +napoleon_use_param = True +napoleon_use_rtype = True +napoleon_type_aliases = None + +# Intersphinx mapping +intersphinx_mapping = { + 'python': ('https://docs.python.org/3', None), + 'can': ('https://python-can.readthedocs.io/en/stable/', None), +} + # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -158,4 +232,4 @@ ] -# -- Extension configuration ------------------------------------------------- \ No newline at end of file +# -- Extension configuration ------------------------------------------------- diff --git a/docs/index.rst b/docs/index.rst index 9ea39ad..a2aef3a 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -6,7 +6,9 @@ CAN SAE J1939 for Python :caption: Contents: readme - + examples + source/modules + Indices and tables ================== diff --git a/docs/requirements.txt b/docs/requirements.txt new file mode 100644 index 0000000..019cf92 --- /dev/null +++ b/docs/requirements.txt @@ -0,0 +1,3 @@ +sphinx>=8.1.3 +sphinx_rtd_theme>=3.1.0 +python-can>=4.0 diff --git a/docs/source/j1939.rst b/docs/source/j1939.rst deleted file mode 100644 index ef1dfd0..0000000 --- a/docs/source/j1939.rst +++ /dev/null @@ -1,62 +0,0 @@ -j1939 package -============= - -Submodules ----------- - -j1939.controller\_application module ------------------------------------- - -.. automodule:: j1939.controller_application - :members: - :undoc-members: - :show-inheritance: - -j1939.electronic\_control\_unit module --------------------------------------- - -.. automodule:: j1939.electronic_control_unit - :members: - :undoc-members: - :show-inheritance: - -j1939.message\_id module ------------------------- - -.. automodule:: j1939.message_id - :members: - :undoc-members: - :show-inheritance: - -j1939.name module ------------------ - -.. automodule:: j1939.name - :members: - :undoc-members: - :show-inheritance: - -j1939.parameter\_group\_number module -------------------------------------- - -.. automodule:: j1939.parameter_group_number - :members: - :undoc-members: - :show-inheritance: - -j1939.version module --------------------- - -.. automodule:: j1939.version - :members: - :undoc-members: - :show-inheritance: - - -Module contents ---------------- - -.. automodule:: j1939 - :members: - :undoc-members: - :show-inheritance: diff --git a/docs/source/modules.rst b/docs/source/modules.rst deleted file mode 100644 index 4f9eebd..0000000 --- a/docs/source/modules.rst +++ /dev/null @@ -1,7 +0,0 @@ -j1939 -===== - -.. toctree:: - :maxdepth: 4 - - j1939 diff --git a/examples/diagnostic_message.py b/examples/diagnostic_message.py index b76fa34..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) @@ -54,7 +54,7 @@ def dm1_before_send(): :return: list of dictionaries of all DTCs included in DM1 - :rtype: list of dic: 'spn', 'fmi', 'oc' + :rtype: list of dic: 'spn', 'fmi', 'oc', 'cm' """ lamp_status = {} # get lamp status (optional, if status not enter, lamp is switched off) @@ -65,9 +65,12 @@ def dm1_before_send(): # add all active DTCs # if no DTC is active return empty list + # 'cm' is the SAE J1939-73 SPN conversion method (1, 2, 3, or 4). + # Defaults to 4 (current standard) when omitted. dtc_list = [] - dtc_list.append({'spn': 123, 'fmi': 31}) # occurrence counter is set to 0 - dtc_list.append({'spn': 456, 'fmi': 1, 'oc': 132}) # with optional occurrence counter + dtc_list.append({'spn': 123, 'fmi': 31}) # CM defaults to 4 + dtc_list.append({'spn': 456, 'fmi': 1, 'oc': 132}) # with occurrence counter + dtc_list.append({'spn': 789, 'fmi': 2, 'oc': 5, 'cm': 3}) # legacy CM 3 layout return lamp_status, dtc_list @@ -81,12 +84,12 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + # ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) # subscribe to all (global) messages on the bus ecu.subscribe(on_message) @@ -127,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 98ffcff..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) @@ -50,12 +49,12 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - # ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + # ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) # add CA to the ECU ecu.add_ca(controller_application=ca) diff --git a/examples/j1939_21_cmdt_send_receive/j1939_send.py b/examples/j1939_21_cmdt_send_receive/j1939_send.py index 16a7556..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) @@ -104,12 +104,12 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - # ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + # ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) # add CA to the ECU ecu.add_ca(controller_application=ca) @@ -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 4e56e58..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): @@ -66,7 +66,7 @@ def main(): ecu = j1939.ElectronicControlUnit(data_link_layer='j1939-22', max_cmdt_packets=200) # can fd Baud: 500k/2M - ecu.connect(bustype='pcan', channel='PCAN_USBBUS3', fd=True, + ecu.connect(interface='pcan', channel='PCAN_USBBUS3', fd=True, f_clock_mhz=80, nom_brp=10, nom_tseg1=12, nom_tseg2=3, nom_sjw=1, data_brp=4, data_tseg1=7, data_tseg2=2, data_sjw=1) # subscribe to all (global) messages on the bus @@ -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 65a7608..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): @@ -62,7 +62,7 @@ def main(): ecu = j1939.ElectronicControlUnit(data_link_layer='j1939-22', max_cmdt_packets=200) # can fd Baud: 500k/2M - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', fd=True, + ecu.connect(interface='pcan', channel='PCAN_USBBUS1', fd=True, f_clock_mhz=80, nom_brp=10, nom_tseg1=12, nom_tseg2=3, nom_sjw=1, data_brp=4, data_tseg1=7, data_tseg2=2, data_sjw=1) # subscribe to all (global) messages on the bus @@ -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 157c304..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 @@ -101,13 +101,13 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) - # ecu.connect('testchannel_1', bustype='virtual') + # ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) + # ecu.connect('testchannel_1', interface='virtual') # add CA to the ECU ecu.add_ca(controller_application=ca) @@ -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 fe0d2e0..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") @@ -31,12 +31,12 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=250000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + # ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=250000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) # subscribe to all (global) messages on the bus ecu.subscribe(on_message) @@ -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 5332d2e..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") @@ -31,12 +31,12 @@ def main(): # Connect to the CAN bus # Arguments are passed to python-can's can.interface.Bus() constructor # (see https://python-can.readthedocs.io/en/stable/bus.html). - # ecu.connect(bustype='socketcan', channel='can0') - # ecu.connect(bustype='kvaser', channel=0, bitrate=250000) - ecu.connect(bustype='pcan', channel='PCAN_USBBUS1', bitrate=500000) - # ecu.connect(bustype='ixxat', channel=0, bitrate=250000) - # ecu.connect(bustype='vector', app_name='CANalyzer', channel=0, bitrate=250000) - # ecu.connect(bustype='nican', channel='CAN0', bitrate=250000) + # ecu.connect(interface='socketcan', channel='can0') + # ecu.connect(interface='kvaser', channel=0, bitrate=250000) + ecu.connect(interface='pcan', channel='PCAN_USBBUS1', bitrate=500000) + # ecu.connect(interface='ixxat', channel=0, bitrate=250000) + # ecu.connect(interface='vector', app_name='CANalyzer', channel=0, bitrate=250000) + # ecu.connect(interface='nican', channel='CAN0', bitrate=250000) # subscribe to all global and peer-to-peer messages with destination 0xFA ecu.subscribe(on_message, 0xFA) @@ -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 7e426ab..54c51f9 100644 --- a/j1939/Dm14Query.py +++ b/j1939/Dm14Query.py @@ -1,5 +1,10 @@ -from enum import Enum + +from __future__ import annotations + import queue +from collections.abc import Callable +from enum import Enum + import j1939 @@ -39,11 +44,14 @@ def __init__(self, ca: j1939.ControllerApplication, user_level=7) -> None: self._ca = ca self.state = QueryState.IDLE - self._seed_from_key = None - self.data_queue = queue.Queue() + 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() + self.exception_queue: queue.Queue = queue.Queue() self.user_level = user_level + self._dest_address: int | None = None + self.address: int | None = None + self.command: Command | None = None def unsubscribe_all(self) -> None: """ @@ -101,6 +109,12 @@ def _send_dm14(self, key_or_user_level: int) -> None: :param int key_or_user_level: key or user level """ + if self.address is None: + raise RuntimeError("address must be set before sending DM14") + if self.command is None: + raise RuntimeError("command must be set before sending DM14") + if self._dest_address is None: + raise RuntimeError("destination address must be set before sending DM14") self._pgn = j1939.ParameterGroupNumber.PGN.DM14 pointer = self.address.to_bytes(length=4, byteorder="little") data = [] @@ -120,6 +134,8 @@ def _send_dm16(self) -> None: """ Send DM16 message to device, used to send data to the device """ + if self._dest_address is None: + raise RuntimeError("destination address must be set before sending DM16") self._pgn = j1939.ParameterGroupNumber.PGN.DM16 data = [] byte_count = len(self.bytes) @@ -207,15 +223,16 @@ def _parse_dm16( self._ca.subscribe(self._parse_dm15) self.state = QueryState.WAIT_FOR_OPER_COMPLETE - def _values_to_bytes(self, values: list) -> bytearray: + def _values_to_bytes(self, values: list) -> list: """ - convert values to bytes for sending to device + convert values to a flat list of bytes for sending to device :param list values: values to be converted to bytes + :return: flat list of ints representing the byte encoding """ - bytes = [] + result = [] for val in values: - bytes.extend(val.to_bytes(self.object_byte_size, byteorder="little")) - return bytes + result.extend(val.to_bytes(self.object_byte_size, byteorder="little")) + return result def _bytes_to_values(self, raw_bytes: bytearray) -> list: """ @@ -323,9 +340,9 @@ def write( raise RuntimeError("No response from server") pass # expect empty queue for write - def set_seed_key_algorithm(self, algorithm: callable) -> None: + def set_seed_key_algorithm(self, algorithm: Callable[[int], int]) -> None: """ set seed-key algorithm to be used for key generation - :param callable algorithm: seed-key algorithm + :param algorithm: seed-key algorithm """ self._seed_from_key = algorithm diff --git a/j1939/Dm14Server.py b/j1939/Dm14Server.py index 6e52caa..f8039fd 100644 --- a/j1939/Dm14Server.py +++ b/j1939/Dm14Server.py @@ -1,6 +1,11 @@ -from enum import Enum + +from __future__ import annotations + import queue import secrets +from collections.abc import Callable +from enum import Enum + import j1939 @@ -24,16 +29,16 @@ def __init__(self, ca: j1939.ControllerApplication) -> None: self._ca = ca self._busy = False - self.sa = None + self.sa: int | None = None self.state = ResponseState.IDLE - self._key_from_seed = None - self.data_queue = queue.Queue() - self._seed_generator = self.generate_seed - self._verify_key = None - self.address = 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: Callable[..., bool] | None = None + self.address: bytearray | None = None self.length = 8 self.proceed = False - self.data = [] + self.data: bytearray | list = [] self.error = 0x00 self.edcp = 0x07 self.status = j1939.Dm15Status.PROCEED.value @@ -44,6 +49,8 @@ def _wait_for_data(self) -> None: Determines whether to send data or wait to receive data based on the command type. If the command is a read command, then the data requested is sent. """ + if self.sa is None: + raise RuntimeError("sa must be set before waiting for data") self._ca.subscribe(self._parse_dm16) self._send_dm15( self.length, @@ -146,7 +153,7 @@ def parse_dm14( self.status, self.state, self.object_count, - self.sa, + sa, ) else: self.state = ResponseState.SEND_PROCEED @@ -175,9 +182,10 @@ def _send_dm15( state: ResponseState, object_count: int, sa: int, - pgn: int = j1939.ParameterGroupNumber.PGN.DM15, - error: int = None, - edcp: 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, @@ -212,6 +220,10 @@ def _send_dm15( self.state = ResponseState.WAIT_OPERATION_COMPLETE case ResponseState.SEND_ERROR: + if error is None: + raise RuntimeError("error must be provided for SEND_ERROR state") + if edcp is None: + raise RuntimeError("edcp must be provided for SEND_ERROR state") status = j1939.Dm15Status.OPERATION_FAILED.value data[0] = 0x00 data[1] = (direct << 4) + (status << 1) + 1 @@ -229,11 +241,13 @@ def _send_dm16(self) -> None: """ Send DM16 message to device, used to send requested data """ + if self.sa is None: + raise RuntimeError("sa must be set before sending DM16") self._pgn = j1939.ParameterGroupNumber.PGN.DM16 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)) @@ -268,7 +282,7 @@ def _parse_dm16( self.status, self.state, self.object_count, - self.sa, + sa, ) def bytes_to_int(self, data: bytearray) -> int: @@ -294,24 +308,24 @@ def generate_seed(self) -> int: seed = 0xBEEF return seed - def set_seed_key_algorithm(self, algorithm: callable) -> None: + def set_seed_key_algorithm(self, algorithm: Callable[[int], int]) -> None: """ Set seed key algorithm to be used for key generation - :param callable algorithm: seed-key algorithm + :param algorithm: seed-key algorithm """ self._key_from_seed = algorithm - def set_seed_generator(self, algorithm: callable) -> None: + def set_seed_generator(self, algorithm: Callable[[], int]) -> None: """ Sets seed generation algorithm to be used for generating a seed value - :param callable algorithm: seed generation algorithm + :param algorithm: seed generation algorithm """ self._seed_generator = algorithm - def set_verify_key(self, algorithm: callable) -> None: + def set_verify_key(self, algorithm: Callable[..., bool]) -> None: """ Set key verification algorithm to be used for key verification - :param callable algorithm: key verification algorithm + :param algorithm: key verification algorithm """ self._verify_key = algorithm @@ -325,9 +339,13 @@ def verify_key(self, seed: int, key: int) -> bool: # TODO: add ability to dynamically pass arguments to verification function if needed, # if this is breaking can just add **kwargs to function defintion used to set the verification function # this will allow for the reception of additional arguments if needed + if self.address is None: + raise RuntimeError("address must be set before verifying key") return self._verify_key( seed=seed, key=key, address=self.bytes_to_int(self.address), sa=self.sa ) + if self._key_from_seed is None: + raise RuntimeError("no key-from-seed algorithm set; call set_seed_key_algorithm first") return self._key_from_seed(seed) == key def unsubscribe_all(self) -> None: @@ -364,7 +382,7 @@ def respond( error: int = 0xFFFFFF, edcp: int = 0xFF, max_timeout: int = 3, - ) -> 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 @@ -389,7 +407,7 @@ def respond( else: self.state = ResponseState.SEND_ERROR self._wait_for_data() - mem_data = 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 bdf54ca..84d34cb 100644 --- a/j1939/controller_application.py +++ b/j1939/controller_application.py @@ -1,5 +1,9 @@ +from __future__ import annotations + import logging + import j1939 + from .message_id import FrameFormat logger = logging.getLogger(__name__) @@ -52,18 +56,23 @@ 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 = None + self._ecu: j1939.ElectronicControlUnit | None = None self._subscribers_request = [] self._subscribers_acknowledge = [] self._started = False + @property + def _ecu_ref(self) -> j1939.ElectronicControlUnit: + if self._ecu is None: + raise RuntimeError("CA is not associated with an ECU") + return self._ecu + def associate_ecu(self, ecu): """Binds this CA to the ECU given :param ecu: The ECU this CA should be bound to. A j1939 :class:`j1939.ElectronicControlUnit` instance """ - self._ecu : j1939.ElectronicControlUnit self._ecu = ecu def remove_ecu(self): @@ -75,14 +84,14 @@ def subscribe(self, callback): :param callback: Function to call when message is received. """ - self._ecu.subscribe(callback, self.message_acceptable) + self._ecu_ref.subscribe(callback, self.message_acceptable) def unsubscribe(self, callback): """Stop listening for message. :param callback: Function to call when message is received. """ - self._ecu.unsubscribe(callback) + self._ecu_ref.unsubscribe(callback) def subscribe_request(self, callback): """Add the given callback to the request notification stream. @@ -114,14 +123,36 @@ def add_timer(self, delta_time, callback, cookie=None): :param callback: The callback function to call """ - self._ecu.add_timer(delta_time, callback, cookie) + self._ecu_ref.add_timer(delta_time, callback, cookie) def remove_timer(self, callback): """Removes ALL entries from the timer event list for the given callback :param callback: The callback to be removed from the timer event list """ - self._ecu.remove_timer(callback) + self._ecu_ref.remove_timer(callback) + + def register_dependent(self, dependent): + """Register a helper whose ``stop()`` should be called on ECU shutdown. + + Convenience forwarder to :meth:`ElectronicControlUnit.register_dependent` + for helpers that only hold a reference to a CA. + + :param dependent: + Any object exposing a no-arg ``stop()`` method. + """ + self._ecu_ref.register_dependent(dependent) + + def unregister_dependent(self, dependent): + """Remove a previously-registered dependent. + + Convenience forwarder to + :meth:`ElectronicControlUnit.unregister_dependent`. + + :param dependent: + The object previously passed to :meth:`register_dependent`. + """ + self._ecu_ref.unregister_dependent(dependent) def start(self, claim_delay=0.5): """Starts the CA @@ -132,7 +163,7 @@ def start(self, claim_delay=0.5): # check if we are not already started and there is an ecu connected if self._ecu and not self.started: self._started = True - self._ecu.add_timer(claim_delay, self._process_claim_async) + self._ecu_ref.add_timer(claim_delay, self._process_claim_async) def stop(self): """Stops the CA @@ -140,12 +171,12 @@ def stop(self): # check if we are already started and there is an ecu connected if self._ecu and self.started: self._started = False - self._ecu.remove_timer(self._process_claim_async) + self._ecu_ref.remove_timer(self._process_claim_async) 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: @@ -166,7 +197,7 @@ def _process_claim_async(self, cookie): # do nothing pass # add new event with (possibly) new timeout value - self._ecu.add_timer(time_to_sleep, self._process_claim_async) + self._ecu_ref.add_timer(time_to_sleep, self._process_claim_async) # returning false deletes the event from the list return False @@ -202,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 @@ -226,6 +257,85 @@ def _process_addressclaim(self, mid, data, timestamp): # we are in the middle of the claim-process self._send_address_claimed(self._device_address_announced) + def accepts_commanded_address(self): + """Whether this CA honors a Commanded Address (J1939-81). + + Defaults to the NAME's Arbitrary Address Capable bit; override to + support other address-configurable device classes that the NAME alone + cannot represent (e.g. Command Configurable). + """ + return bool(self._name.arbitrary_address_capable) + + def _process_commanded_address(self, src_address, data, timestamp): + """Processes a Commanded Address message (J1939-81, PGN 65240). + + The Commanded Address assigns a specific source address to the device + identified by the embedded 64-bit NAME. If the NAME matches ours and we + accept the command, run the address-claim procedure at the commanded + address. + + :param int src_address: + The source address the Commanded Address was sent from. + :param bytearray data: + The reassembled 9-byte payload (bytes 0-7: NAME, byte 8: new SA). + :param float timestamp: + The timestamp the message was received in fractions of Epoch-Seconds. + """ + if len(data) < 9: + return + commanded_name = j1939.Name(bytes=bytes(data[0:8])) + new_address = data[8] + if commanded_name.value != self._name.value: + # not addressed to this CA + return + if not self.accepts_commanded_address(): + logger.info("Ignoring Commanded Address for SA '%d': not accepted by policy", new_address) + return + logger.info("Received Commanded Address: claiming new address '%d'", new_address) + self._begin_address_claim(new_address) + + def _begin_address_claim(self, new_address): + """Initiate the J1939-81 address-claim procedure at the given address. + + Reuses the existing claim state machine: an Address Claimed message is + transmitted immediately at the new source address. Addresses in the + 128..247 range enter WAIT_VETO (resolved to NORMAL by the + :meth:`_process_claim_async` timer, contention by + :meth:`_process_addressclaim`); all other addresses claim immediately. + + :param int new_address: + The source address to claim. Must be a valid (claimable) source + address in the range 0..253; NULL (254) and GLOBAL (255) are + rejected. + + :return: + True if the claim procedure was started, otherwise False. + """ + # Only 0..253 are valid (claimable) source addresses. NULL (254) and + # GLOBAL (255) must never be claimed - doing so would put the CA into an + # invalid state. + if new_address < 0 or new_address > 253: + logger.warning("Ignoring address claim for invalid source address '%d'", new_address) + return False + + self._device_address_preferred = new_address + self._device_address_announced = new_address + self._send_address_claimed(new_address) + if new_address > 127 and new_address < 248: + self._device_address_state = ControllerApplication.State.WAIT_VETO + # Re-arm the veto timeout so the WAIT_VETO -> NORMAL transition + # happens after the veto window rather than waiting for the next + # periodic claim tick. Only relevant when the periodic claim timer + # is already running (i.e. the CA has been started). + if self.started: + self._ecu_ref.remove_timer(self._process_claim_async) + self._ecu_ref.add_timer(ControllerApplication.ClaimTimeout.VETO, self._process_claim_async) + else: + # addresses from 0..127 and 248..253 claim immediately + self._device_address = new_address + self._device_address_state = ControllerApplication.State.NORMAL + return True + def _process_request(self, mid, dest_address, data, timestamp): """Processes a REQUEST message :param j1939.MessageId mid: @@ -259,7 +369,7 @@ def send_message(self, priority, parameter_group_number, data): raise RuntimeError("Could not send message unless address claiming has finished") mid = j1939.MessageId(priority=priority, parameter_group_number=parameter_group_number, source_address=self._device_address) - self._ecu.send_message(mid.can_id, True, data) + self._ecu_ref.send_message(mid.can_id, True, data) def send_pgn(self, data_page, pdu_format, pdu_specific, priority, data, time_limit=0, frame_format=FrameFormat.FEFF): """send a pgn @@ -275,7 +385,7 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, data, time_lim if self.state != ControllerApplication.State.NORMAL: raise RuntimeError("Could not send message unless address claiming has finished") - return self._ecu.send_pgn(data_page, pdu_format, pdu_specific, priority, self._device_address, data, time_limit, frame_format) + return self._ecu_ref.send_pgn(data_page, pdu_format, pdu_specific, priority, self._device_address, data, time_limit, frame_format) def send_request(self, data_page, pgn, destination): """send a request message @@ -291,7 +401,7 @@ def send_request(self, data_page, pgn, destination): source_address = self._device_address data = [(pgn & 0xFF), ((pgn >> 8) & 0xFF), ((pgn >> 16) & 0xFF)] - self._ecu.send_pgn(data_page, (j1939.ParameterGroupNumber.PGN.REQUEST >> 8) & 0xFF, destination & 0xFF, 6, source_address, data) + self._ecu_ref.send_pgn(data_page, (j1939.ParameterGroupNumber.PGN.REQUEST >> 8) & 0xFF, destination & 0xFF, 6, source_address, data) def _send_address_claimed(self, address): # TODO: Normally the (initial) address claimed message must not be an auto repeat message. @@ -300,7 +410,7 @@ def _send_address_claimed(self, address): pgn = j1939.ParameterGroupNumber(0, 238, j1939.ParameterGroupNumber.Address.GLOBAL) mid = j1939.MessageId(priority=6, parameter_group_number=pgn.value, source_address=address) data = self._name.bytes - self._ecu.send_message(mid.can_id, True, data) + self._ecu_ref.send_message(mid.can_id, True, data) def on_request(self, src_address, dest_address, pgn): """Callback for PGN requests diff --git a/j1939/diagnostic_messages.py b/j1939/diagnostic_messages.py index 9d6ac6a..531262a 100644 --- a/j1939/diagnostic_messages.py +++ b/j1939/diagnostic_messages.py @@ -1,27 +1,73 @@ -import j1939 import logging +import j1939 + logger = logging.getLogger(__name__) class DTC: """ - Parser for J1939 DTC (Diagnostic Trouble Code) + Parser/encoder for J1939 DTC (Diagnostic Trouble Code). + + Supports the four SAE J1939-73 SPN conversion methods: + - CM 1: SPN MSBs in byte 1, mid in byte 2, LSBs+FMI in byte 3, CM bit = 1 + - CM 2: SPN mid in byte 1, MSBs in byte 2, LSBs+FMI in byte 3, CM bit = 1 + - CM 3: SPN LSBs/mid/MSBs in bytes 1/2/3 (modern layout), CM bit = 1 + - CM 4: same byte layout as CM 3, CM bit = 0 (current standard) + + The on-wire CM bit only distinguishes {1,2,3} (bit=1) from {4} (bit=0). + CM 1 vs CM 2 vs CM 3 are not separable from the bytes alone; when + decoding raw bytes with CM bit = 1, the caller must indicate which one + was used (defaults to CM 3 — the most common legacy layout). """ - def __init__(self, dtc=None, spn=None, fmi=None, oc=0): - if dtc != None: + def __init__(self, dtc=None, spn=None, fmi=None, oc=0, cm=4): + if dtc is not None: + self._cm = cm self._dtc = dtc - self._spn = ((dtc & 0xFFFF) | ((dtc >> 5) & 0x70000)) - self._fmi = ((dtc >> 16) & 0x1F) - self._oc = ((dtc >> 24) & 0x7f) - self._cm = ((dtc >> 31) & 0x01) - if self._cm != 0: - logger.error("DM01: deprecated spn conversion modes are not supported") + self._oc = ((dtc >> 24) & 0x7F) + cm_bit = ((dtc >> 31) & 0x01) + b1 = dtc & 0xFF + b2 = (dtc >> 8) & 0xFF + b3 = (dtc >> 16) & 0xFF + self._fmi = b3 & 0x1F + spn_low3 = (b3 >> 5) & 0x07 + if cm in (3, 4): + self._spn = b1 | (b2 << 8) | (spn_low3 << 16) + elif cm == 1: + # b1 = SPN[18:11], b2 = SPN[10:3], b3[7:5] = SPN[2:0] + self._spn = (b1 << 11) | (b2 << 3) | spn_low3 + elif cm == 2: + # b1 = SPN[10:3], b2 = SPN[18:11], b3[7:5] = SPN[2:0] + self._spn = (b2 << 11) | (b1 << 3) | spn_low3 + else: + raise ValueError(f"Invalid conversion method: {cm}. Must be 1, 2, 3, or 4.") + # Sanity-check the CM bit against the requested method + expected_cm_bit = 0 if cm == 4 else 1 + if cm_bit != expected_cm_bit: + logger.warning("DM01: CM bit %d does not match requested conversion method %d", cm_bit, cm) else: - self._dtc = ((spn & 0xFFFF) | ((spn & 0x70000) << 5) | ((fmi & 0x1F) << 16) | ((oc & 0x7F) << 24)) + if cm not in (1, 2, 3, 4): + raise ValueError(f"Invalid conversion method: {cm}. Must be 1, 2, 3, or 4.") + if spn is None or fmi is None: + raise ValueError("spn and fmi must be provided when dtc is None") self._spn = spn self._fmi = fmi self._oc = oc - self._cm = 0 + self._cm = cm + if cm == 1: + b1 = (spn >> 11) & 0xFF + b2 = (spn >> 3) & 0xFF + elif cm == 2: + b1 = (spn >> 3) & 0xFF + b2 = (spn >> 11) & 0xFF + else: # cm in (3, 4) + b1 = spn & 0xFF + b2 = (spn >> 8) & 0xFF + b3 = (((spn >> 16) & 0x07) << 5) | (fmi & 0x1F) if cm in (3, 4) \ + else ((spn & 0x07) << 5) | (fmi & 0x1F) + b4 = oc & 0x7F + if cm != 4: + b4 |= 0x80 + self._dtc = b1 | (b2 << 8) | (b3 << 16) | (b4 << 24) @property def spn(self): @@ -57,7 +103,7 @@ def oc(self): def cm(self): """ :return: - SPN conversion mode + SPN conversion method (1, 2, 3, or 4 per SAE J1939-73) :rtype: int """ @@ -102,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 @@ -127,16 +173,24 @@ class Dm1: """ _msg_subscriber_added = False - def __init__(self, ca: j1939.ControllerApplication): + def __init__(self, ca: j1939.ControllerApplication, rx_cm_bit_set: int = 3): """ :param obj ca: j1939 controller application + :param int rx_cm_bit_set: + SPN conversion method (1, 2, or 3) to assume when a received DTC + has its CM bit set. The on-wire CM bit cannot distinguish CMs 1, + 2 and 3 — only between {1,2,3} (bit=1) and 4 (bit=0). Defaults to + 3 (the most common legacy layout). CM 4 is auto-detected. """ + if rx_cm_bit_set not in (1, 2, 3): + raise ValueError(f"rx_cm_bit_set must be 1, 2, or 3 (got {rx_cm_bit_set})") self._pgn = j1939.ParameterGroupNumber.PGN.DM01 self._lamp_status = {} self._dtc_dic_list = [] self._data = [] self._subscribers = [] self._ca = ca + self._rx_cm_bit_set = rx_cm_bit_set def subscribe(self, callback): """Add the given callback to the Dm1 message notification stream. @@ -144,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 @@ -223,15 +277,16 @@ 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) - dtc = DTC(spn=dtc_dic['spn'], fmi=dtc_dic['fmi'], oc=dtc_dic['oc']).dtc + dtc = DTC(spn=dtc_dic['spn'], fmi=dtc_dic['fmi'], oc=dtc_dic['oc'], cm=cm).dtc self._data.append(dtc & 0xFF) self._data.append((dtc >> 8) & 0xFF) self._data.append((dtc >> 16) & 0xFF) @@ -290,8 +345,9 @@ def _parse_dm1_receive_data(self): # so we should not add this to the dtc list since it is not a valid dtc continue - dtc = DTC(dtc=dtc_int) - self._dtc_dic_list.append( {'spn': dtc.spn, 'fmi': dtc.fmi, 'oc': dtc.oc } ) + cm = 4 if ((dtc_int >> 31) & 0x01) == 0 else self._rx_cm_bit_set + dtc = DTC(dtc=dtc_int, cm=cm) + self._dtc_dic_list.append( {'spn': dtc.spn, 'fmi': dtc.fmi, 'oc': dtc.oc, 'cm': dtc.cm } ) def _notify_subscribers(self, sa, timestamp): for callback in self._subscribers: @@ -327,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 @@ -392,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 cbb5abc..6efa776 100644 --- a/j1939/electronic_control_unit.py +++ b/j1939/electronic_control_unit.py @@ -1,32 +1,66 @@ +from __future__ import annotations + +import heapq import logging +import queue +import threading +import time +import warnings + import can from can import Listener -import time -import sys -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__) + class ElectronicControlUnit: """ElectronicControlUnit (ECU) holding one or more ControllerApplications (CAs).""" - - def __init__(self, data_link_layer='j1939-21', max_cmdt_packets=1, minimum_tp_rts_cts_dt_interval=None, minimum_tp_bam_dt_interval=None, send_message=None): + def __init__( + self, + data_link_layer="j1939-21", + max_cmdt_packets=1, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=None, + send_message=None, + bus: can.BusABC | None = None, + dispatch_queue_size: int = 1000, + ): """ :param data_link_layer: specify data-link-layer, 'j1939-21' or 'j1939-22' + :param max_cmdt_packets: + maximum number of segments that can be sent in one transport protocol session (1-255) + :param minimum_tp_rts_cts_dt_interval: + minimum time in seconds between RTS/CTS/DT messages (default: None, which means 0.05s for j1939-21 and 0.01s for j1939-22) + :param minimum_tp_bam_dt_interval: + minimum time in seconds between BAM/DT messages (default: None, which means 0.05s for j1939-21 and 0.01s for j1939-22) + :param send_message: + optional callback function to send a raw CAN message to the bus. If not provided, the default implementation will be used, which sends messages via the python-can bus. + :param bus: + optional python-can :class:`can.BusABC` instance. If not provided, the ECU will not be connected to a bus until :meth:`connect` is called. + :param int dispatch_queue_size: + Maximum number of CAN frames that may be buffered in the dispatch + queue between the python-can Notifier thread and the ECU dispatch + thread. If the queue is full when a new frame arrives the frame is + dropped and a warning is logged (suppressed until the queue drains). + Defaults to 1000. """ if send_message: self.send_message = send_message #: A python-can :class:`can.BusABC` instance - self._bus = None + self._bus = bus + # TODO: remove this once the deprecated connect() path is removed. This is only used to track if the bus was created by this ECU or passed in by the user. + self._bus_created = ( + False # True if the bus was created by this ECU (deprecated connect() path) + ) # Locking object for send self._send_lock = threading.Lock() @@ -34,40 +68,176 @@ def __init__(self, data_link_layer='j1939-21', max_cmdt_packets=1, minimum_tp_rt raise ValueError("max number of segments that can be sent is 0xFF") # set data link layer - if data_link_layer == 'j1939-21': - self.j1939_dll = J1939_21(self.send_message, self._job_thread_wakeup, self._notify_subscribers, max_cmdt_packets, minimum_tp_rts_cts_dt_interval, minimum_tp_bam_dt_interval, self._is_message_acceptable) - elif data_link_layer == 'j1939-22': - self.j1939_dll = J1939_22(self.send_message, self._job_thread_wakeup, self._notify_subscribers, max_cmdt_packets, minimum_tp_rts_cts_dt_interval, minimum_tp_bam_dt_interval, self._is_message_acceptable) + if data_link_layer == "j1939-21": + self.j1939_dll = J1939_21( + self.send_message, + self._protocol_wakeup, + self._notify_subscribers, + max_cmdt_packets, + minimum_tp_rts_cts_dt_interval, + minimum_tp_bam_dt_interval, + self._is_message_acceptable, + ) + elif data_link_layer == "j1939-22": + self.j1939_dll = J1939_22( + self.send_message, + self._protocol_wakeup, + self._notify_subscribers, + max_cmdt_packets, + minimum_tp_rts_cts_dt_interval, + minimum_tp_bam_dt_interval, + self._is_message_acceptable, + ) else: - raise ValueError("either 'j1939-21' or 'j1939-22' must be provided for data link layer") + raise ValueError( + "either 'j1939-21' or 'j1939-22' must be provided for data link layer" + ) #: Includes at least MessageListener. self._listeners = [MessageListener(self)] self._notifier = None + self._subscribers = [] + self._subscribers_lock = threading.RLock() - # List of timer events the job thread should care of + # Heap-based timer event list: (deadline, seq, callback, cookie, delta_time) self._timer_events = [] + self._timer_seq = 0 + self._timer_events_lock = threading.RLock() - self._job_thread_end = threading.Event() - logger.info("Starting ECU async thread") - self._job_thread_wakeup_queue = queue.Queue() - self._job_thread = threading.Thread(target=self._async_job_thread, name='j1939.ecu job_thread') - # A thread can be flagged as a "daemon thread". The significance of - # this flag is that the entire Python program exits when only daemon - # threads are left. - self._job_thread.daemon = True - self._job_thread.start() + # Dependent lifecycle registry. Any object that needs to be stopped before the ECU's own threads should be registered here. See :meth:`register_dependent`. + self._dependents = [] + self._dependents_lock = threading.RLock() + self._stopping = False + self._job_thread_end = threading.Event() - def stop(self): + # Protocol thread: owns TP/BAM timeout management only — no user callbacks + logger.info("Starting ECU protocol thread") + self._protocol_wakeup_queue = queue.Queue() + self._protocol_thread = threading.Thread( + target=self._protocol_job_thread, name="j1939.ecu protocol_thread" + ) + self._protocol_thread.daemon = True + + # Timer thread: owns application cyclic callbacks only + logger.info("Starting ECU timer thread") + self._timer_wakeup_queue = queue.Queue() + self._timer_thread = threading.Thread( + target=self._timer_job_thread, name="j1939.ecu timer_thread" + ) + self._timer_thread.daemon = True + + # Dispatch thread: drains incoming frames from the Notifier thread and + # calls j1939_dll.notify() (and therefore _notify_subscribers) serially. + # Ordering is preserved: the queue is FIFO and the single dispatch + # thread processes frames serially — identical semantics to before. + # + # The queue is bounded (maxsize=dispatch_queue_size, default 1000). + # When full, notify() drops the incoming frame and logs a warning. + # The warning is suppressed after the first drop and re-emitted as a + # summary once the queue has room again, to avoid log flooding. + # Drop-tracking state is only accessed from the python-can Notifier + # thread (the sole caller of notify()), so no locking is required. + logger.info("Starting ECU dispatch thread") + self._dispatch_queue: queue.Queue = queue.Queue(maxsize=dispatch_queue_size) + # Number of frames dropped since the last time the queue drained. + self._dispatch_queue_drop_count: int = 0 + # True while the queue is at capacity and frames are being dropped. + self._dispatch_queue_dropped: bool = False + self._dispatch_thread = threading.Thread( + target=self._dispatch_job_thread, name="j1939.ecu dispatch_thread" + ) + self._dispatch_thread.daemon = True + + self._protocol_thread.start() + self._timer_thread.start() + self._dispatch_thread.start() + + def stop(self, dispatch_join_timeout: float = 3.0): """Stops the ECU background handling - This Function explicitely stops the background handling of the ECU. + This Function explicitly stops the background handling of the ECU. + + Before stopping the ECU's own protocol/timer threads, every registered + dependent (see :meth:`register_dependent`) has its ``stop()`` method + invoked in LIFO order. Exceptions raised by a dependent's ``stop()`` + are logged and swallowed so a single misbehaving dependent cannot + prevent the rest of the shutdown from completing. + + :param float dispatch_join_timeout: + Maximum seconds to wait for the dispatch thread to finish its + current subscriber callback before giving up and continuing + shutdown. The dispatch thread is daemonic so it will not prevent + interpreter exit, but a warning is logged if it is still alive + after this timeout. Defaults to 3s """ + # Snapshot dependents under lock, then mark the ECU as stopping so any + # late registrations are rejected. + with self._dependents_lock: + self._stopping = True + dependents = list(self._dependents) + self._dependents.clear() + + # LIFO: most-recently registered first. + for dep in reversed(dependents): + try: + dep.stop() + except Exception: + logger.exception("Error stopping dependent %r", dep) + self._job_thread_end.set() - self._job_thread_wakeup() - self._job_thread.join() + self._protocol_wakeup_queue.put(1) + self._timer_wakeup_queue.put(1) + self._protocol_thread.join() + self._timer_thread.join() + self._dispatch_thread.join(timeout=dispatch_join_timeout) + if self._dispatch_thread.is_alive(): + logger.warning( + "dispatch_thread did not exit within %.1f s — a subscriber " + "callback may be blocking. Continuing shutdown.", + dispatch_join_timeout, + ) + + def register_dependent(self, dependent): + """Register a helper whose ``stop()`` should be called by :meth:`stop`. + + Any helper object that owns threads, timers, or other resources tied + to this ECU should call this during construction. ``ecu.stop()`` will + invoke ``dependent.stop()`` in LIFO order before tearing down its own + threads. + + Duplicate registrations of the same object (by identity) are silently + ignored. + + :param dependent: + Any object exposing a no-arg ``stop()`` method. + + :raises RuntimeError: + If called while the ECU is shutting down. + :raises TypeError: + If ``dependent`` does not expose a callable ``stop`` attribute. + """ + if not callable(getattr(dependent, "stop", None)): + raise TypeError("dependent must expose a callable stop() method") + with self._dependents_lock: + if self._stopping: + raise RuntimeError( + "Cannot register a dependent while the ECU is stopping" + ) + for existing in self._dependents: + if existing is dependent: + return + self._dependents.append(dependent) + + def unregister_dependent(self, dependent): + """Remove a previously-registered dependent. + + :param dependent: + The object previously passed to :meth:`register_dependent`. + """ + with self._dependents_lock: + self._dependents = [d for d in self._dependents if d is not dependent] def add_timer(self, delta_time, callback, cookie=None): """Adds a callback to the list of timer events @@ -77,16 +247,14 @@ def add_timer(self, delta_time, callback, cookie=None): :param callback: The callback function to call """ - - d = { - 'delta_time': delta_time, - 'callback': callback, - 'deadline': (time.time() + delta_time), - 'cookie': cookie, - } - - self._timer_events.append( d ) - self._job_thread_wakeup() + deadline = time.monotonic() + delta_time + with self._timer_events_lock: + heapq.heappush( + self._timer_events, + (deadline, self._timer_seq, callback, cookie, delta_time), + ) + self._timer_seq += 1 + self._timer_wakeup_queue.put(1) def remove_timer(self, callback): """Removes ALL entries from the timer event list for the given callback @@ -94,10 +262,10 @@ def remove_timer(self, callback): :param callback: The callback to be removed from the timer event list """ - for event in self._timer_events: - if event['callback'] == callback: - self._timer_events.remove( event ) - self._job_thread_wakeup() + with self._timer_events_lock: + self._timer_events = [e for e in self._timer_events if e[2] != callback] + heapq.heapify(self._timer_events) + self._timer_wakeup_queue.put(1) def connect(self, *args, **kwargs): """Connect to CAN bus using python-can. @@ -107,8 +275,8 @@ def connect(self, *args, **kwargs): :param channel: Backend specific channel for the CAN interface. - :param str bustype: - Name of the interface. See + :param str interface: + Name of the interface (formerly ``bustype``, renamed in python-can v4.2). See `python-can manual `__ for full list of supported interfaces. :param int bitrate: @@ -117,7 +285,22 @@ def connect(self, *args, **kwargs): :raises can.CanError: When connection fails. """ - self._bus = can.interface.Bus(*args, **kwargs) + # TODO: since bus creation has been an existing feature, keeping backwards compatibility with the old way of creating a bus. + # But this should be refactored in the future to use a more explicit way of creating a bus. + if self._bus is None: + warnings.warn( + "Creating a bus in connect() is deprecated; pass a bus instance to the constructor instead", + category=DeprecationWarning, + stacklevel=2, + ) + self._bus = can.interface.Bus(*args, **kwargs) + self._bus_created = True + elif args or kwargs: + raise ValueError( + "connect() was called with bus configuration arguments but a bus " + "instance was already provided to the constructor. Pass arguments " + "to the constructor instead, or call connect() with no arguments." + ) logger.info("Connected to '%s'", self._bus.channel_info) self._notifier = can.Notifier(self._bus, self._listeners, 1) return self._bus @@ -127,8 +310,17 @@ def disconnect(self): Must be overridden in a subclass if a custom interface is used. """ + if self._notifier is None: + raise RuntimeError( + "notifier is not set; call connect() before disconnect()" + ) + if self._bus is None: + raise RuntimeError("bus is not set; call connect() before disconnect()") self._notifier.stop() - self._bus.shutdown() + self._notifier = None + if self._bus_created: + self._bus.shutdown() + self._bus_created = False self._bus = None def subscribe(self, callback, device_address=None): @@ -142,7 +334,8 @@ def subscribe(self, callback, device_address=None): Only one device address can be entered. Multiple device addresses are only possible with controller applications. Note: TP.CMDT will only be received if the destination address is bound to a controller application. """ - self._subscribers.append({'cb': callback, 'dev_adr':device_address}) + with self._subscribers_lock: + self._subscribers.append({"cb": callback, "dev_adr": device_address}) def unsubscribe(self, callback): """Stop listening for message. @@ -150,10 +343,8 @@ def unsubscribe(self, callback): :param callback: Function to call when message is received. """ - for dic in self._subscribers: - if dic['cb'] == callback: - self._subscribers.remove(dic) - + with self._subscribers_lock: + self._subscribers = [d for d in self._subscribers if d["cb"] != callback] def add_ca(self, **kwargs): """Add a ControllerApplication to the ECU. @@ -172,13 +363,15 @@ def add_ca(self, **kwargs): :rtype: r3964.ControllerApplication """ - if 'controller_application' in kwargs: - ca = kwargs['controller_application'] + if "controller_application" in kwargs: + ca = kwargs["controller_application"] else: - if 'name' not in kwargs: - raise ValueError("either 'controller_application' or 'name' must be provided") - name = kwargs.get('name') - da = kwargs.get('device_address', None) + if "name" not in kwargs: + raise ValueError( + "either 'controller_application' or 'name' must be provided" + ) + name = kwargs.get("name") + da = kwargs.get("device_address", None) ca = ControllerApplication(name, da) self.j1939_dll.add_ca(ca) @@ -212,21 +405,41 @@ def add_notifier(self, notifier): """ self._notifier = notifier for listener in self._listeners: + # A listener may have been permanently marked stopped by a + # previous can.Notifier.stop() call (e.g. a notifier shared with + # other consumers via a ref-counted registry, torn down and + # recreated while this ECU itself stayed alive). This ECU's + # listeners are created once in __init__ and reused for its + # whole lifetime, so re-adding to a (possibly new) notifier must + # also clear that flag -- otherwise on_message_received() keeps + # silently dropping every frame even though the listener is + # registered on a live notifier. + listener.stopped = False self._notifier.add_listener(listener) - + def remove_bus(self): - """Remove the bus from the ECU. - """ + """Remove the bus from the ECU.""" self._bus = None - + def remove_notifier(self): - """Remove the notifier from the ECU. - """ + """Remove the notifier from the ECU.""" + if self._notifier is None: + return for listener in self._listeners: self._notifier.remove_listener(listener) self._notifier = None - def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, data, time_limit=0, frame_format=FrameFormat.FEFF): + def send_pgn( + self, + data_page, + pdu_format, + pdu_specific, + priority, + src_address, + data, + time_limit=0, + frame_format=FrameFormat.FEFF, + ): """send a pgn :param int data_page: data page :param int pdu_format: pdu format @@ -238,7 +451,16 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d after this time, the multi-pg will be sent. several pgs can thus be combined in one multi-pg. 0 or no time-limit means immediate sending. """ - return self.j1939_dll.send_pgn(data_page, pdu_format, pdu_specific, priority, src_address, data, time_limit, frame_format) + return self.j1939_dll.send_pgn( + data_page, + pdu_format, + pdu_specific, + priority, + src_address, + data, + time_limit, + frame_format, + ) def send_message(self, can_id, extended_id, data, fd_format=False): """Send a raw CAN message to the bus. @@ -260,15 +482,18 @@ def send_message(self, can_id, extended_id, data, fd_format=False): if not self._bus: raise RuntimeError("Not connected to CAN bus") - msg = can.Message(is_extended_id=extended_id, - arbitration_id=can_id, - data=data, - is_fd=fd_format, - bitrate_switch=fd_format - ) + msg = can.Message( + is_extended_id=extended_id, + arbitration_id=can_id, + data=data, + is_fd=fd_format, + bitrate_switch=fd_format, + ) with self._send_lock: - self._bus.send(msg) - # TODO: check error receivement + try: + self._bus.send(msg) + except can.CanError as e: + logger.error(f'not able to send message because {e}') def notify(self, can_id, data, timestamp): """Feed incoming CAN message into this ecu. @@ -276,6 +501,16 @@ def notify(self, can_id, data, timestamp): If a custom interface is used, this function must be called for each 29-bit standard message read from the CAN bus. + The frame is enqueued onto the dispatch queue and processed by the + dedicated dispatch thread. This returns quickly to the caller + (typically the python-can Notifier thread) without waiting for + subscriber callbacks to complete. + + If the dispatch queue is full the frame is dropped rather than + blocking. A warning is logged on the first drop and suppressed until + the queue drains, at which point a summary of the total drop count is + logged. + :param int can_id: CAN-ID of the message (always 29-bit) :param bytearray data: @@ -286,74 +521,144 @@ def notify(self, can_id, data, timestamp): seconds. Where possible this will be timestamped in hardware. """ - self.j1939_dll.notify(can_id, data, timestamp) + if self._job_thread_end.is_set(): + # ECU is stopping or stopped; the dispatch thread has exited or is + # draining. Drop the frame rather than growing the queue + # with no consumer. + return + try: + self._dispatch_queue.put_nowait((can_id, data, timestamp)) + if self._dispatch_queue_dropped: + # Queue has room again — emit the suppressed summary and reset. + logger.warning( + "dispatch_queue drained: %d frame(s) were dropped while the queue was full", + self._dispatch_queue_drop_count, + ) + self._dispatch_queue_dropped = False + self._dispatch_queue_drop_count = 0 + except queue.Full: + self._dispatch_queue_drop_count += 1 + if not self._dispatch_queue_dropped: + logger.warning( + "dispatch_queue full (maxsize=%d): dropping incoming frames until queue drains", + self._dispatch_queue.maxsize, + ) + self._dispatch_queue_dropped = True def add_bus_filters(self, filters: can.typechecking.CanFilters | None): """Add bus filters to the underlying CAN bus. - :param filters: - An iterable of dictionaries each containing a "can_id", - a "can_mask", and an optional "extended" key + :param filters: + An iterable of dictionaries each containing a "can_id", + a "can_mask", and an optional "extended" key """ if self._bus is None: raise RuntimeError("Not connected to CAN bus") self._bus.set_filters(filters) - def _async_job_thread(self): - """Asynchronous thread for handling various jobs + def _dispatch_job_thread(self): + """Dispatch thread: drains the incoming frame queue and calls the DLL. - This Thread handles various tasks: - - Event trigger for associated CAs - - Timeout monitoring of communication objects + Loops while the ECU is running, blocking on the dispatch queue with a + short timeout so it can observe ``_job_thread_end`` being set by + :meth:`stop`. Uses the same ``while not self._job_thread_end.is_set()`` + exit condition as the protocol and timer threads — no sentinel value + or second exit mechanism needed. - To construct a blocking wait with timeout the task waits on a - queue-object. When other tasks are adding timer-events they can - wakeup the timeout handler to recalculate the new sleep-time - to awake at the new events. + After the loop exits any frames that arrived concurrently with the stop + signal are drained so that in-flight TP reassembly is not truncated. """ - system = sys.platform - while not self._job_thread_end.is_set(): - - now = time.time() - + try: + can_id, data, timestamp = self._dispatch_queue.get(timeout=0.1) + except queue.Empty: + continue + try: + self.j1939_dll.notify(can_id, data, timestamp) + except Exception: + logger.exception("Exception in dispatch thread") + + # Drain any frames that arrived between the last get() and stop() so + # that in-flight TP sessions are not truncated mid-reassembly. + while True: + try: + can_id, data, timestamp = self._dispatch_queue.get_nowait() + except queue.Empty: + break + try: + self.j1939_dll.notify(can_id, data, timestamp) + except Exception: + logger.exception("Exception in dispatch thread (drain)") + + def _protocol_job_thread(self): + """Protocol thread: handles TP/BAM timeout management only. + + This thread is isolated from application timer callbacks so that slow + user callbacks cannot delay protocol-level timeouts (which would cause + spurious ABORT messages on the bus). + """ + while not self._job_thread_end.is_set(): + now = time.monotonic() next_wakeup = self.j1939_dll.async_job_thread(now) + time_to_sleep = next_wakeup - time.monotonic() + if time_to_sleep > 0: + try: + self._protocol_wakeup_queue.get(True, time_to_sleep) + except queue.Empty: + pass + + def _timer_job_thread(self): + """Timer thread: handles application cyclic callbacks only. - # check timer events - for event in self._timer_events: - if event['deadline'] > now: - if next_wakeup > event['deadline']: - next_wakeup = event['deadline'] - else: - # deadline reached - logger.debug("Deadline for event reached") - if event['callback']( event['cookie'] ) == True: - # "true" means the callback wants to be called again - while event['deadline'] < now: - # just to take care of overruns - event['deadline'] += event['delta_time'] - # recalc next wakeup - if next_wakeup > event['deadline']: - next_wakeup = event['deadline'] - else: - # remove from list - self._timer_events.remove( event ) - - time_to_sleep = next_wakeup - time.time() + Uses a heapq (min-heap keyed by deadline) for O(log n) scheduling. + Woken early via _timer_wakeup_queue whenever a timer is added/removed. + Callbacks returning True are rescheduled; returning False are removed. + """ + while not self._job_thread_end.is_set(): + now = time.monotonic() + next_wakeup = now + 5.0 + + with self._timer_events_lock: + while self._timer_events and self._timer_events[0][0] <= now: + deadline, seq, cb, cookie, delta = heapq.heappop(self._timer_events) + logger.debug("Deadline for timer event reached") + try: + reschedule = cb(cookie) is True + except Exception: + # TODO: is there a better way to handle exceptions in user callbacks? + # We don't want one bad callback to break the timer thread, + # but we also don't want to just swallow it silently. + logger.exception("Timer callback failed: %r", cb) + reschedule = False + if reschedule: + # reschedule: advance deadline past now to avoid burst catch-up + new_deadline = deadline + delta + while new_deadline < now: + new_deadline += delta + heapq.heappush( + self._timer_events, + (new_deadline, self._timer_seq, cb, cookie, delta), + ) + self._timer_seq += 1 + # returning False (or None) means remove — already popped, nothing to do + + if self._timer_events: + next_wakeup = self._timer_events[0][0] + + time_to_sleep = next_wakeup - time.monotonic() if time_to_sleep > 0: try: - self._job_thread_wakeup_queue.get(True, time_to_sleep) + self._timer_wakeup_queue.get(True, time_to_sleep) except queue.Empty: - # do nothing pass - def _job_thread_wakeup(self): - """Wakeup the async job thread + def _protocol_wakeup(self): + """Wakeup the protocol job thread. - By calling this function we wakeup the asyncronous job thread to - force a recalculation of his next wakeup event. + Called by the DLL (j1939_21/j1939_22) when TP state changes require + immediate re-evaluation of protocol deadlines. """ - self._job_thread_wakeup_queue.put(1) + self._protocol_wakeup_queue.put(1) def _notify_subscribers(self, priority, pgn, sa, dest, timestamp, data): """Feed incoming message to subscribers. @@ -371,21 +676,29 @@ 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)) - # notify only the CA for which the message is intended - # each CA receives all broadcast messages - - # TODO: this is ineffecient but there exists a possibility of removing subscribers during callback - # and adding new ones in while this is going and it can impact message receivement - for dic in self._subscribers.copy(): - if (dic['dev_adr'] == None) or (dest == ParameterGroupNumber.Address.GLOBAL) or (callable(dic['dev_adr']) and dic['dev_adr'](dest)) or (dest == dic['dev_adr']): - dic['cb'](priority, pgn, sa, timestamp, data) + 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) + for dic in snapshot: + if ( + (dic["dev_adr"] is None) + or (dest == ParameterGroupNumber.Address.GLOBAL) + or (callable(dic["dev_adr"]) and dic["dev_adr"](dest)) + or (dest == dic["dev_adr"]) + ): + dic["cb"](priority, pgn, sa, timestamp, data) def _is_message_acceptable(self, dest): - for dic in self._subscribers: - if dic['dev_adr'] == dest: - return True - return False + # Ownership / active-participation check only: does a subscriber own this + # exact destination address (simple peer-to-peer reception)? This gate + # decides whether the stack actively engages the directed transport + # protocol (RTS/CTS/EOM-ACK). Passive wildcard (``device_address=None``) + # and callable subscribers are handled separately by _notify_subscribers() + # so a monitor never causes the stack to answer on the bus. + with self._subscribers_lock: + return any(d["dev_adr"] == dest for d in self._subscribers) + class MessageListener(Listener): """Listens for messages on CAN bus and feeds them to an ECU instance. @@ -394,12 +707,17 @@ class MessageListener(Listener): The ECU to notify on new messages. """ - def __init__(self, ecu : ElectronicControlUnit): + def __init__(self, ecu: ElectronicControlUnit): self.ecu = ecu 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): + def on_message_received(self, msg: can.Message): + 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 75553eb..0e2071e 100644 --- a/j1939/j1939_21.py +++ b/j1939/j1939_21.py @@ -1,8 +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: @@ -19,6 +21,8 @@ class ConnectionAbortReason: TIMEOUT = 3 # A timeout occured # 4..250 Reserved by SAE CTS_WHILE_DT = 4 # according AUTOSAR: CTS messages received when data transfer is in progress + BAD_SEQUENCE = 7 + DUPLICATE_SEQUENCE = 8 # 251..255 Per J1939/71 definitions - but there are none? class Timeout: @@ -51,7 +55,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 @@ -59,6 +63,10 @@ def __init__(self, send_message, job_thread_wakeup, notify_subscribers, max_cmdt # number of packets that can be sent/received with CMDT (Connection Mode Data Transfer) self._max_cmdt_packets = max_cmdt_packets + # Lock protecting _rcv_buffer and _snd_buffer — accessed from both the + # Notifier thread (notify/process_tp_*) and the protocol job thread (async_job_thread). + self._buffer_lock = threading.Lock() + self.__job_thread_wakeup = job_thread_wakeup self.__send_message = send_message self.__notify_subscribers = notify_subscribers @@ -106,49 +114,59 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d # init sequence # known limitation: only one BAM can be sent in parallel to a destination node buffer_hash = self._buffer_hash(src_address, dest_address) - if buffer_hash in self._snd_buffer: - # There is already a sequence active for this pair - return False message_size = len(data) num_packets = int(message_size / 7) if (message_size % 7 == 0) else int(message_size / 7) + 1 # if the PF is between 240 and 255, the message can only be broadcast if dest_address == ParameterGroupNumber.Address.GLOBAL: - # send BAM + # send BAM before acquiring the lock — CAN I/O must not be + # held under _buffer_lock to avoid priority inversion with the + # protocol thread. + with self._buffer_lock: + if buffer_hash in self._snd_buffer: + # There is already a sequence active for this pair + return False self.__send_tp_bam(src_address, priority, pgn.value, message_size, num_packets) # init new buffer for this connection - self._snd_buffer[buffer_hash] = { - "pgn": pgn.value, - "priority": priority, - "message_size": message_size, - "num_packages": num_packets, - "data": data, - "state": self.SendBufferState.SENDING_BM, - "deadline": time.time() + self._minimum_tp_bam_dt_interval, - 'src_address' : src_address, - 'dest_address' : ParameterGroupNumber.Address.GLOBAL, - 'next_packet_to_send' : 0, - } + with self._buffer_lock: + self._snd_buffer[buffer_hash] = { + "pgn": pgn.value, + "priority": priority, + "message_size": message_size, + "num_packages": num_packets, + "data": data, + "state": self.SendBufferState.SENDING_BM, + "deadline": time.monotonic() + self._minimum_tp_bam_dt_interval, + 'src_address' : src_address, + 'dest_address' : ParameterGroupNumber.Address.GLOBAL, + 'next_packet_to_send' : 0, + } else: # send RTS/CTS pgn.pdu_specific = 0 # this is 0 for peer-to-peer transfer - # init new buffer for this connection - self._snd_buffer[buffer_hash] = { - "pgn": pgn.value, - "priority": priority, - "message_size": message_size, - "num_packages": num_packets, - "data": data, - "state": self.SendBufferState.WAITING_CTS, - "deadline": time.time() + self.Timeout.T3, - 'src_address' : src_address, - 'dest_address' : pdu_specific, - 'next_packet_to_send' : 0, - 'next_wait_on_cts': 0, - } + with self._buffer_lock: + if buffer_hash in self._snd_buffer: + # There is already a sequence active for this pair + return False self.__send_tp_rts(src_address, pdu_specific, priority, pgn.value, message_size, num_packets, min(self._max_cmdt_packets, num_packets)) + # init new buffer for this connection + with self._buffer_lock: + self._snd_buffer[buffer_hash] = { + "pgn": pgn.value, + "priority": priority, + "message_size": message_size, + "num_packages": num_packets, + "data": data, + "state": self.SendBufferState.WAITING_CTS, + "deadline": time.monotonic() + self.Timeout.T3, + 'src_address' : src_address, + 'dest_address' : pdu_specific, + 'next_packet_to_send' : 0, + 'next_wait_on_cts': 0, + } + self.__job_thread_wakeup() return True @@ -158,107 +176,106 @@ def async_job_thread(self, now): next_wakeup = now + 5.0 # wakeup in 5 seconds - # check receive buffers for timeout - # using "list(x)" to prevent "RuntimeError: dictionary changed size during iteration" - for bufid in list(self._rcv_buffer): - buf = self._rcv_buffer[bufid] - if buf['deadline'] != 0: - if buf['deadline'] > now: - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - else: - # deadline reached - logger.info("Deadline reached for rcv_buffer src 0x%02X dst 0x%02X", buf['src_address'], buf['dest_address'] ) - if buf['dest_address'] != ParameterGroupNumber.Address.GLOBAL: - # TODO: should we handle retries? - self.__send_tp_abort(buf['dest_address'], buf['src_address'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) - # TODO: should we notify our CAs about the cancelled transfer? - del self._rcv_buffer[bufid] - - # check send buffers - # using "list(x)" to prevent "RuntimeError: dictionary changed size during iteration" - for bufid in list(self._snd_buffer): - buf = self._snd_buffer[bufid] - if buf['deadline'] != 0: - if buf['deadline'] > now: - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - else: - # deadline reached - if buf['state'] == self.SendBufferState.WAITING_CTS: - logger.info("Deadline WAITING_CTS reached for snd_buffer src 0x%02X dst 0x%02X", buf['src_address'], buf['dest_address'] ) - self.__send_tp_abort(buf['src_address'], buf['dest_address'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) + with self._buffer_lock: + # check receive buffers for timeout + for bufid in list(self._rcv_buffer): + buf = self._rcv_buffer[bufid] + if buf['deadline'] != 0: + if buf['deadline'] > now: + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + else: + # deadline reached + logger.info("Deadline reached for rcv_buffer src 0x%02X dst 0x%02X", buf['src_address'], buf['dest_address'] ) + if buf['dest_address'] != ParameterGroupNumber.Address.GLOBAL: + # TODO: should we handle retries? + self.__send_tp_abort(buf['dest_address'], buf['src_address'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) # TODO: should we notify our CAs about the cancelled transfer? - del self._snd_buffer[bufid] - elif buf['state'] == self.SendBufferState.SENDING_IN_CTS: - while buf['next_packet_to_send'] < buf['num_packages']: - package = buf['next_packet_to_send'] - offset = package * 7 + del self._rcv_buffer[bufid] + + # check send buffers + for bufid in list(self._snd_buffer): + buf = self._snd_buffer[bufid] + if buf['deadline'] != 0: + if buf['deadline'] > now: + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + else: + # deadline reached + if buf['state'] == self.SendBufferState.WAITING_CTS: + logger.info("Deadline WAITING_CTS reached for snd_buffer src 0x%02X dst 0x%02X", buf['src_address'], buf['dest_address'] ) + self.__send_tp_abort(buf['src_address'], buf['dest_address'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) + # TODO: should we notify our CAs about the cancelled transfer? + del self._snd_buffer[bufid] + elif buf['state'] == self.SendBufferState.SENDING_IN_CTS: + while buf['next_packet_to_send'] < buf['num_packages']: + package = buf['next_packet_to_send'] + offset = package * 7 + data = buf['data'][offset:] + if len(data)>7: + data = data[:7] + else: + while len(data)<7: + data.append(255) + data.insert(0, package+1) + + # modify the snd_buffer state in anticipation + # of the message we are about to transmit + + buf['next_packet_to_send'] += 1 + + should_break = False + if package == buf['next_wait_on_cts']: + # wait on next cts + buf['state'] = self.SendBufferState.WAITING_CTS + buf['deadline'] = time.monotonic() + self.Timeout.T3 + should_break = True + 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 + + # state is ready for recv - Now send the message + self.__send_tp_dt(buf['src_address'], buf['dest_address'], data) + if should_break: + break + + # recalc next wakeup + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + + elif buf['state'] == self.SendBufferState.SENDING_BM: + # send next broadcast message... + offset = buf['next_packet_to_send'] * 7 data = buf['data'][offset:] if len(data)>7: data = data[:7] else: while len(data)<7: data.append(255) - data.insert(0, package+1) + data.insert(0, buf['next_packet_to_send']+1) # modify the snd_buffer state in anticipation # of the message we are about to transmit buf['next_packet_to_send'] += 1 - should_break = False - if package == buf['next_wait_on_cts']: - # wait on next cts - buf['state'] = self.SendBufferState.WAITING_CTS - buf['deadline'] = time.time() + self.Timeout.T3 - should_break = True - elif self._minimum_tp_rts_cts_dt_interval != None: - buf['deadline'] = time.time() + self._minimum_tp_rts_cts_dt_interval - should_break = True - - # state is ready for recv - Now send the message - self.__send_tp_dt(buf['src_address'], buf['dest_address'], data) - if should_break: - break - - # recalc next wakeup - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - - elif buf['state'] == self.SendBufferState.SENDING_BM: - # send next broadcast message... - offset = buf['next_packet_to_send'] * 7 - data = buf['data'][offset:] - if len(data)>7: - data = data[:7] - else: - while len(data)<7: - data.append(255) - data.insert(0, buf['next_packet_to_send']+1) - - # modify the snd_buffer state in anticipation - # of the message we are about to transmit - - buf['next_packet_to_send'] += 1 + if buf['next_packet_to_send'] < buf['num_packages']: + buf['deadline'] = time.monotonic() + self._minimum_tp_bam_dt_interval + # recalc next wakeup + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + else: + # done + del self._snd_buffer[bufid] - if buf['next_packet_to_send'] < buf['num_packages']: - buf['deadline'] = time.time() + self._minimum_tp_bam_dt_interval - # recalc next wakeup - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] + # state is updated and ready for recv - now send data + self.__send_tp_dt(buf['src_address'], buf['dest_address'], data) + elif buf['state'] == self.SendBufferState.TRANSMISSION_FINISHED: + del self._snd_buffer[bufid] else: - # done + logger.critical("unknown SendBufferState %d", buf['state']) del self._snd_buffer[bufid] - # state is updated and ready for recv - now send data - self.__send_tp_dt(buf['src_address'], buf['dest_address'], data) - elif buf['state'] == self.SendBufferState.TRANSMISSION_FINISHED: - del self._snd_buffer[bufid] - else: - logger.critical("unknown SendBufferState %d", buf['state']) - del self._snd_buffer[bufid] - return next_wakeup @@ -279,109 +296,111 @@ def _process_tp_cm(self, mid, dest_address, data, timestamp): src_address = mid.source_address - if control_byte == self.ConnectionMode.RTS: - message_size = data[1] | (data[2] << 8) - num_packages = data[3] - max_num_packages = data[4] # Maximum number of segments that can be sent in response to one CTS. - buffer_hash = self._buffer_hash(src_address, dest_address) - if buffer_hash in self._rcv_buffer: - # according SAE J1939-21 we have to send an ABORT if an active - # transmission is already established - self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.BUSY, pgn) - return + with self._buffer_lock: + if control_byte == self.ConnectionMode.RTS: + message_size = data[1] | (data[2] << 8) + num_packages = data[3] + max_num_packages = data[4] # Maximum number of segments that can be sent in response to one CTS. + buffer_hash = self._buffer_hash(src_address, dest_address) + if buffer_hash in self._rcv_buffer: + # according SAE J1939-21 we have to send an ABORT if an active + # transmission is already established + self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.BUSY, pgn) + return - # limit max number segments - max_num_packages = min(max_num_packages, num_packages) - - # open new buffer for this connection - self._rcv_buffer[buffer_hash] = { - 'pgn': pgn, - 'message_size': message_size, - 'num_packages': num_packages, - 'next_packet': min(self._max_cmdt_packets, max_num_packages), - 'max_cmdt_packages': self._max_cmdt_packets, - 'num_packages_max_rec': min(self._max_cmdt_packets, max_num_packages), - 'data': [], - 'deadline': time.time() + self.Timeout.T2, - 'src_address' : src_address, - 'dest_address' : dest_address, - } - - self.__send_tp_cts(dest_address, src_address, self._rcv_buffer[buffer_hash]['num_packages_max_rec'], 1, pgn) - self.__job_thread_wakeup() - elif control_byte == self.ConnectionMode.CTS: - num_packages = data[1] - next_package_number = data[2] - 1 - buffer_hash = self._buffer_hash(dest_address, src_address) - if buffer_hash not in self._snd_buffer: - self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.RESOURCES, pgn) - return - if num_packages == 0: - # SAE J1939/21 - # receiver requests a pause - self._snd_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.Th - self.__job_thread_wakeup() - return + # limit max number segments + max_num_packages = min(max_num_packages, num_packages) + + # open new buffer for this connection + self._rcv_buffer[buffer_hash] = { + 'pgn': pgn, + 'message_size': message_size, + 'num_packages': num_packages, + 'next_expected_packet': 1, + 'next_packet': min(self._max_cmdt_packets, max_num_packages), + 'max_cmdt_packages': self._max_cmdt_packets, + 'num_packages_max_rec': min(self._max_cmdt_packets, max_num_packages), + 'data': [], + 'deadline': time.monotonic() + self.Timeout.T2, + 'src_address' : src_address, + 'dest_address' : dest_address, + } - num_packages_all = self._snd_buffer[buffer_hash]["num_packages"] - if num_packages > num_packages_all: - logger.debug("CTS: Allowed more packets %d than complete transmission %d", num_packages, num_packages_all) - num_packages = num_packages_all - if next_package_number + num_packages > num_packages_all: - logger.debug("CTS: Allowed more packets %d than needed to complete transmission %d", num_packages, num_packages_all - next_package_number) - num_packages = num_packages_all - next_package_number + self.__send_tp_cts(dest_address, src_address, self._rcv_buffer[buffer_hash]['num_packages_max_rec'], 1, pgn) + self.__job_thread_wakeup() + elif control_byte == self.ConnectionMode.CTS: + num_packages = data[1] + next_package_number = data[2] - 1 + buffer_hash = self._buffer_hash(dest_address, src_address) + if buffer_hash not in self._snd_buffer: + self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.RESOURCES, pgn) + return + if num_packages == 0: + # SAE J1939/21 + # receiver requests a pause + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.Th + self.__job_thread_wakeup() + return - self._snd_buffer[buffer_hash]['next_wait_on_cts'] = self._snd_buffer[buffer_hash]['next_packet_to_send'] + num_packages - 1 + num_packages_all = self._snd_buffer[buffer_hash]["num_packages"] + if num_packages > num_packages_all: + logger.debug("CTS: Allowed more packets %d than complete transmission %d", num_packages, num_packages_all) + num_packages = num_packages_all + if next_package_number + num_packages > num_packages_all: + logger.debug("CTS: Allowed more packets %d than needed to complete transmission %d", num_packages, num_packages_all - next_package_number) + num_packages = num_packages_all - next_package_number - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.SENDING_IN_CTS - self._snd_buffer[buffer_hash]['deadline'] = time.time() - self.__job_thread_wakeup() + self._snd_buffer[buffer_hash]['next_wait_on_cts'] = self._snd_buffer[buffer_hash]['next_packet_to_send'] + num_packages - 1 + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.SENDING_IN_CTS + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() + self.__job_thread_wakeup() - elif control_byte == self.ConnectionMode.EOM_ACK: - buffer_hash = self._buffer_hash(dest_address, src_address) - if buffer_hash not in self._snd_buffer: - self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.RESOURCES, pgn) - return - # TODO: should we inform the application about the successful transmission? - # Notify subscribers here to be used for the memory access server to know when to send operation complete - self.__notify_subscribers(mid.priority,pgn,mid.source_address,dest_address,timestamp,data) + elif control_byte == self.ConnectionMode.EOM_ACK: + buffer_hash = self._buffer_hash(dest_address, src_address) + if buffer_hash not in self._snd_buffer: + self.__send_tp_abort(dest_address, src_address, self.ConnectionAbortReason.RESOURCES, pgn) + return + # TODO: should we inform the application about the successful transmission? + # Notify subscribers here to be used for the memory access server to know when to send operation complete + self.__notify_subscribers(mid.priority,pgn,mid.source_address,dest_address,timestamp,data) - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED - self._snd_buffer[buffer_hash]['deadline'] = time.time() - self.__job_thread_wakeup() - elif control_byte == self.ConnectionMode.BAM: - message_size = data[1] | (data[2] << 8) - num_packages = data[3] - buffer_hash = self._buffer_hash(src_address, dest_address) - if buffer_hash in self._rcv_buffer: - # TODO: should we deliver the partly received message to our CAs? - del self._rcv_buffer[buffer_hash] + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() self.__job_thread_wakeup() + elif control_byte == self.ConnectionMode.BAM: + message_size = data[1] | (data[2] << 8) + num_packages = data[3] + buffer_hash = self._buffer_hash(src_address, dest_address) + if buffer_hash in self._rcv_buffer: + # TODO: should we deliver the partly received message to our CAs? + del self._rcv_buffer[buffer_hash] + self.__job_thread_wakeup() - # init new buffer for this connection - self._rcv_buffer[buffer_hash] = { - "pgn": pgn, - "message_size": message_size, - "num_packages": num_packages, - "next_packet": 1, - "max_cmdt_packages": self._max_cmdt_packets, - "data": [], - "deadline": time.time() + self.Timeout.T1, - 'src_address' : src_address, - 'dest_address' : dest_address, - } - self.__job_thread_wakeup() - elif control_byte == self.ConnectionMode.ABORT: - # if abort received before transmission established -> cancel transmission - buffer_hash = self._buffer_hash(dest_address, src_address) - if buffer_hash in self._snd_buffer and self._snd_buffer[buffer_hash]['state'] == self.SendBufferState.WAITING_CTS: - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED - self._snd_buffer[buffer_hash]['deadline'] = time.time() - # TODO: any more abort responses? - pass - else: - raise RuntimeError("Received TP.CM with unknown control_byte %d", control_byte) + # init new buffer for this connection + self._rcv_buffer[buffer_hash] = { + "pgn": pgn, + "message_size": message_size, + "num_packages": num_packages, + "next_expected_packet": 1, + "next_packet": 1, + "max_cmdt_packages": self._max_cmdt_packets, + "data": [], + "deadline": time.monotonic() + self.Timeout.T1, + 'src_address' : src_address, + 'dest_address' : dest_address, + } + self.__job_thread_wakeup() + elif control_byte == self.ConnectionMode.ABORT: + # if abort received before transmission established -> cancel transmission + buffer_hash = self._buffer_hash(dest_address, src_address) + if buffer_hash in self._snd_buffer and self._snd_buffer[buffer_hash]['state'] == self.SendBufferState.WAITING_CTS: + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() + # TODO: any more abort responses? + pass + else: + raise RuntimeError("Received TP.CM with unknown control_byte %d", control_byte) def _process_tp_dt(self, mid, dest_address, data, timestamp): sequence_number = data[0] @@ -389,44 +408,75 @@ def _process_tp_dt(self, mid, dest_address, data, timestamp): src_address = mid.source_address buffer_hash = self._buffer_hash(src_address, dest_address) - if buffer_hash not in self._rcv_buffer: - # TODO: LOG/TRACE/EXCEPTION? - return - # get data - self._rcv_buffer[buffer_hash]['data'].extend(data[1:]) - - # message is complete with sending an acknowledge - if len(self._rcv_buffer[buffer_hash]['data']) >= self._rcv_buffer[buffer_hash]['message_size']: - logger.info("finished RCV of PGN {} with size {}".format(self._rcv_buffer[buffer_hash]['pgn'], self._rcv_buffer[buffer_hash]['message_size'])) - # shorten data to message_size - self._rcv_buffer[buffer_hash]['data'] = self._rcv_buffer[buffer_hash]['data'][:self._rcv_buffer[buffer_hash]['message_size']] - # finished reassembly - if dest_address != ParameterGroupNumber.Address.GLOBAL: - self.__send_tp_eom_ack(dest_address, src_address, self._rcv_buffer[buffer_hash]['message_size'], self._rcv_buffer[buffer_hash]['num_packages'], self._rcv_buffer[buffer_hash]['pgn']) - self.__notify_subscribers(mid.priority, self._rcv_buffer[buffer_hash]['pgn'], src_address, dest_address, timestamp, self._rcv_buffer[buffer_hash]['data']) - del self._rcv_buffer[buffer_hash] - self.__job_thread_wakeup() - return + with self._buffer_lock: + if buffer_hash not in self._rcv_buffer: + # TODO: LOG/TRACE/EXCEPTION? + return - # clear to send - if (dest_address != ParameterGroupNumber.Address.GLOBAL) and (sequence_number >= self._rcv_buffer[buffer_hash]['next_packet']): + expected_sequence_number = self._rcv_buffer[buffer_hash]['next_expected_packet'] + if sequence_number != expected_sequence_number: + abort_reason = ( + self.ConnectionAbortReason.DUPLICATE_SEQUENCE + if 0 < sequence_number < expected_sequence_number + else self.ConnectionAbortReason.BAD_SEQUENCE + ) + if dest_address != ParameterGroupNumber.Address.GLOBAL: + self.__send_tp_abort( + dest_address, + src_address, + abort_reason, + self._rcv_buffer[buffer_hash]['pgn'], + ) + del self._rcv_buffer[buffer_hash] + self.__job_thread_wakeup() + raise ValueError( + "J1939-21 TP.DT packet out of sequence: " + f"expected {expected_sequence_number}, received {sequence_number}" + ) + + # get data + self._rcv_buffer[buffer_hash]['data'].extend(data[1:]) + self._rcv_buffer[buffer_hash]['next_expected_packet'] += 1 + + # message is complete with sending an acknowledge + if len(self._rcv_buffer[buffer_hash]['data']) >= self._rcv_buffer[buffer_hash]['message_size']: + logger.info("finished RCV of PGN {} with size {}".format(self._rcv_buffer[buffer_hash]['pgn'], self._rcv_buffer[buffer_hash]['message_size'])) + # shorten data to message_size + self._rcv_buffer[buffer_hash]['data'] = self._rcv_buffer[buffer_hash]['data'][:self._rcv_buffer[buffer_hash]['message_size']] + # finished reassembly + if dest_address != ParameterGroupNumber.Address.GLOBAL: + self.__send_tp_eom_ack(dest_address, src_address, self._rcv_buffer[buffer_hash]['message_size'], self._rcv_buffer[buffer_hash]['num_packages'], self._rcv_buffer[buffer_hash]['pgn']) + if self._rcv_buffer[buffer_hash]['pgn'] == ParameterGroupNumber.PGN.COMMANDED_ADDRESS: + # route Commanded Address (J1939-81) to the registered CAs and + # consume it (do not forward to generic subscribers, consistent + # with ADDRESSCLAIM/REQUEST handling in notify()) + for ca in self._cas: + ca._process_commanded_address(src_address, self._rcv_buffer[buffer_hash]['data'], timestamp) + else: + self.__notify_subscribers(mid.priority, self._rcv_buffer[buffer_hash]['pgn'], src_address, dest_address, timestamp, self._rcv_buffer[buffer_hash]['data']) + del self._rcv_buffer[buffer_hash] + self.__job_thread_wakeup() + return - # send cts - number_of_packets_that_can_be_sent = min( self._rcv_buffer[buffer_hash]['num_packages_max_rec'], self._rcv_buffer[buffer_hash]['num_packages'] - self._rcv_buffer[buffer_hash]['next_packet'] ) - next_packet_to_be_sent = self._rcv_buffer[buffer_hash]['next_packet'] + 1 - self.__send_tp_cts(dest_address, src_address, number_of_packets_that_can_be_sent, next_packet_to_be_sent, self._rcv_buffer[buffer_hash]['pgn']) + # clear to send + if (dest_address != ParameterGroupNumber.Address.GLOBAL) and (sequence_number >= self._rcv_buffer[buffer_hash]['next_packet']): - # calculate next packet number at which a CTS is to be sent - self._rcv_buffer[buffer_hash]['next_packet'] = min(self._rcv_buffer[buffer_hash]['next_packet'] + self._rcv_buffer[buffer_hash]['num_packages_max_rec'], - self._rcv_buffer[buffer_hash]['num_packages']) + # send cts + number_of_packets_that_can_be_sent = min( self._rcv_buffer[buffer_hash]['num_packages_max_rec'], self._rcv_buffer[buffer_hash]['num_packages'] - self._rcv_buffer[buffer_hash]['next_packet'] ) + next_packet_to_be_sent = self._rcv_buffer[buffer_hash]['next_packet'] + 1 + self.__send_tp_cts(dest_address, src_address, number_of_packets_that_can_be_sent, next_packet_to_be_sent, self._rcv_buffer[buffer_hash]['pgn']) - self._rcv_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.T2 - self.__job_thread_wakeup() - return + # calculate next packet number at which a CTS is to be sent + self._rcv_buffer[buffer_hash]['next_packet'] = min(self._rcv_buffer[buffer_hash]['next_packet'] + self._rcv_buffer[buffer_hash]['num_packages_max_rec'], + self._rcv_buffer[buffer_hash]['num_packages']) - self._rcv_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.T1 - self.__job_thread_wakeup() + self._rcv_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.T2 + self.__job_thread_wakeup() + return + + self._rcv_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.T1 + self.__job_thread_wakeup() def __send_tp_dt(self, src_address, dest_address, data): pgn = ParameterGroupNumber(0, 235, dest_address) @@ -499,29 +549,44 @@ def notify(self, can_id, data, timestamp): pgn_value = pgn.value & 0x1FF00 dest_address = pgn.pdu_specific # may be Address.GLOBAL - # iterate all CAs to check if we have to handle this destination address - if dest_address != ParameterGroupNumber.Address.GLOBAL: - if not self.__ecu_is_message_acceptable(dest_address): # simple peer-to-peer reception without adding a controller-application - reject = True + # Does this node OWN the destination address, i.e. should it actively + # participate in the directed transport protocol (RTS/CTS/EOM-ACK)? + # Ownership is decided by an exact peer-to-peer subscriber address or a + # registered ControllerApplication. Passive wildcard/callable subscribers + # must be able to observe directed traffic without the stack answering on + # the bus, so they do NOT grant ownership here. + owns_dest = (dest_address == ParameterGroupNumber.Address.GLOBAL) + if not owns_dest: + if self.__ecu_is_message_acceptable(dest_address): # simple peer-to-peer reception without adding a controller-application + owns_dest = True + else: for ca in self._cas: if ca.message_acceptable(dest_address): - reject = False + owns_dest = True break - if reject == True: - return if pgn_value == ParameterGroupNumber.PGN.ADDRESSCLAIM: for ca in self._cas: ca._process_addressclaim(mid, data, timestamp) + # Address claims are broadcast and observable by any node on the bus; + # forward them to subscribers as well so passive monitors can see the + # NAME/source-address of other nodes. + self.__notify_subscribers(mid.priority, pgn_value, mid.source_address, dest_address, timestamp, data) elif pgn_value == ParameterGroupNumber.PGN.REQUEST: for ca in self._cas: if ca.message_acceptable(dest_address): ca._process_request(mid, dest_address, data, timestamp) elif pgn_value == ParameterGroupNumber.PGN.TP_CM: - self._process_tp_cm(mid, dest_address, data, timestamp) + # only participate in the transport protocol for owned destinations + if owns_dest: + self._process_tp_cm(mid, dest_address, data, timestamp) elif pgn_value == ParameterGroupNumber.PGN.DATATRANSFER: - self._process_tp_dt(mid, dest_address, data, timestamp) + if owns_dest: + self._process_tp_dt(mid, dest_address, data, timestamp) else: + # simple single-frame peer-to-peer PDU1: passive delivery only. + # _notify_subscribers honors wildcard/callable/exact subscribers, so a + # monitor receives directed frames addressed to any node without the + # stack having to own the destination address. self.__notify_subscribers(mid.priority, pgn_value, mid.source_address, dest_address, timestamp, data) return - diff --git a/j1939/j1939_22.py b/j1939/j1939_22.py index c55cf93..3722cbb 100644 --- a/j1939/j1939_22.py +++ b/j1939/j1939_22.py @@ -1,13 +1,15 @@ -from .parameter_group_number import ParameterGroupNumber -from .message_id import MessageId, FrameFormat import logging +import threading import time -import numpy as np +from enum import IntEnum + +from .message_id import FrameFormat, MessageId +from .parameter_group_number import ParameterGroupNumber logger = logging.getLogger(__name__) class J1939_22: - class TpControlType: + class TpControlType(IntEnum): RTS = 0 # Destination Specific Request_To_Send CTS = 1 # Destination Specific Clear_To_Send EOM_STATUS = 2 # Destination Specific or Global Destination End_of_Message Status @@ -15,7 +17,7 @@ class TpControlType: BAM = 4 # Global Destination Broadcast Announce Message ABORT = 15 # Destination Specific Connection Abort - class Adt: # assurance data type + class Adt(IntEnum): # assurance data type NO_ADT = 0 # no assurance Data MS_CS = 1 # Manufacturer specific cybersecurity assurance data MS_FS = 2 # Manufacturer specific functional safety assurance @@ -69,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 @@ -85,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 @@ -99,6 +97,10 @@ def __init__(self, send_message, job_thread_wakeup, notify_subscribers, max_cmdt # number of packets that can be sent/received with CMDT (Connection Mode Data Transfer) self._max_cmdt_packets = max_cmdt_packets + # Lock protecting _rcv_buffer, _snd_buffer, and _multi_pg_snd_buffer — accessed from + # both the Notifier thread (notify/process_tp_*) and the protocol job thread (async_job_thread). + self._buffer_lock = threading.Lock() + self.__job_thread_wakeup = job_thread_wakeup self.__send_message = send_message self.__notify_subscribers = notify_subscribers @@ -172,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 @@ -182,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 @@ -219,41 +221,41 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d self.__send_multi_pg(frame_format, [cpg], src_address, dst_address) else: session = 0 - deadline = time.time() + time_limit - while True: - hash = self._buffer_hash_mpg(frame_format, session, src_address, dst_address) - #hash = self._buffer_hash(session, src_address, dst_address) - if hash not in self._multi_pg_snd_buffer: - self._multi_pg_snd_buffer[hash] = {'deadline': deadline, 'cpg': [cpg], 'fill_level': 4 + data_length} - break - elif (self._multi_pg_snd_buffer[hash]['fill_level'] <= (self.DataLength.TP - data_length)): - # update fill level - self._multi_pg_snd_buffer[hash]['fill_level'] += 4 + data_length - # update deadline - if self._multi_pg_snd_buffer[hash]['deadline'] > deadline: - self._multi_pg_snd_buffer[hash]['deadline'] = deadline - # append c-pg - self._multi_pg_snd_buffer[hash]['cpg'].append(cpg) - break - else: - # trigger sending - self._multi_pg_snd_buffer[hash]['deadline'] = time.time() - self.__job_thread_wakeup() - # get next buffer - session += 1 + deadline = time.monotonic() + time_limit + with self._buffer_lock: + while True: + hash = self._buffer_hash_mpg(frame_format, session, src_address, dst_address) + if hash not in self._multi_pg_snd_buffer: + self._multi_pg_snd_buffer[hash] = {'deadline': deadline, 'cpg': [cpg], 'fill_level': 4 + data_length} + break + elif (self._multi_pg_snd_buffer[hash]['fill_level'] <= (self.DataLength.TP - data_length)): + # update fill level + self._multi_pg_snd_buffer[hash]['fill_level'] += 4 + data_length + # update deadline + if self._multi_pg_snd_buffer[hash]['deadline'] > deadline: + self._multi_pg_snd_buffer[hash]['deadline'] = deadline + # append c-pg + self._multi_pg_snd_buffer[hash]['cpg'].append(cpg) + break + else: + # trigger sending + self._multi_pg_snd_buffer[hash]['deadline'] = time.monotonic() + self.__job_thread_wakeup() + # get next buffer + session += 1 else: # if the PF is between 0 and 239, the message is destination dependent when pdu_specific != 255 # if the PF is between 240 and 255, the message can only be broadcast 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 @@ -264,16 +266,12 @@ 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 - full_tp_size_packages = int(data_length/self.DataLength.TP) - arr = np.array(data) - list_of_arr = np.split(arr, [full_tp_size_packages*self.DataLength.TP]) - arr = np.reshape(list_of_arr[0], (-1,self.DataLength.TP)) - data_list = arr.tolist() - if len(list_of_arr) > 1: - data_list.append(list_of_arr[1].tolist()) + chunk_size = self.DataLength.TP + data_list = [list(data[i:i + chunk_size]) for i in range(0, data_length, chunk_size)] # if the PF is between 240 and 255, the message can only be broadcast if dest_address == ParameterGroupNumber.Address.GLOBAL: @@ -282,37 +280,39 @@ def send_pgn(self, data_page, pdu_format, pdu_specific, priority, src_address, d self.__send_tp_bam(priority, src_address, session_num, pgn.value, message_size, num_segments) # init new buffer for this connection - self._snd_buffer[buffer_hash] = { - 'pgn': pgn.value, - 'priority': priority, - 'session': session_num, - 'message_size': message_size, - 'num_segments': num_segments, - 'data': data_list, - 'state': self.SendBufferState.SENDING_BAM, - 'deadline': time.time() + self._minimum_tp_bam_dt_interval, - 'src_address' : src_address, - 'dest_address' : ParameterGroupNumber.Address.GLOBAL, - 'next_packet_to_send' : 0, - } + with self._buffer_lock: + self._snd_buffer[buffer_hash] = { + 'pgn': pgn.value, + 'priority': priority, + 'session': session_num, + 'message_size': message_size, + 'num_segments': num_segments, + 'data': data_list, + 'state': self.SendBufferState.SENDING_BAM, + 'deadline': time.monotonic() + self._minimum_tp_bam_dt_interval, + 'src_address' : src_address, + 'dest_address' : ParameterGroupNumber.Address.GLOBAL, + 'next_packet_to_send' : 0, + } else: # send RTS/CTS pgn.pdu_specific = 0 # this is 0 for peer-to-peer transfer # init new buffer for this connection - self._snd_buffer[buffer_hash] = { - 'pgn': pgn.value, - 'priority': priority, - 'session': session_num, - 'message_size': message_size, - 'num_segments': num_segments, - 'data': data_list, - 'state': self.SendBufferState.WAITING_CTS, - 'deadline': time.time() + self.Timeout.T3, - 'src_address' : src_address, - 'dest_address' : pdu_specific, - 'next_packet_to_send' : 0, - 'next_wait_on_cts': 0, - } + with self._buffer_lock: + self._snd_buffer[buffer_hash] = { + 'pgn': pgn.value, + 'priority': priority, + 'session': session_num, + 'message_size': message_size, + 'num_segments': num_segments, + 'data': data_list, + 'state': self.SendBufferState.WAITING_CTS, + 'deadline': time.monotonic() + self.Timeout.T3, + 'src_address' : src_address, + 'dest_address' : pdu_specific, + 'next_packet_to_send' : 0, + 'next_wait_on_cts': 0, + } self.__send_tp_rts(priority, src_address, pdu_specific, session_num, pgn.value, message_size, num_segments, min(self._max_cmdt_packets, num_segments)) self.__job_thread_wakeup() @@ -326,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']) @@ -358,124 +358,119 @@ def async_job_thread(self, now): next_wakeup = now + 5.0 # wakeup in 5 seconds - # check receive buffers for timeout - # using 'list(x)' to prevent 'RuntimeError: dictionary changed size during iteration' - for bufid in list(self._rcv_buffer): - buf = self._rcv_buffer[bufid] - if buf['deadline'] != 0: + with self._buffer_lock: + # check receive buffers for timeout + for bufid in list(self._rcv_buffer): + buf = self._rcv_buffer[bufid] + if buf['deadline'] != 0: + if buf['deadline'] > now: + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + else: + # deadline reached + logger.info('Deadline reached for rcv_buffer src 0x%02X dst 0x%02X', buf['src_address'], buf['dest_address'] ) + if buf['dest_address'] != ParameterGroupNumber.Address.GLOBAL: + self.__send_tp_abort(buf['dest_address'], buf['src_address'], buf['session'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) + del self._rcv_buffer[bufid] + self.__put_rts_cts_session(buf['session']) + else: + del self._rcv_buffer[bufid] + self.__put_bam_session(buf['session']) + # TODO: should we notify our CAs about the cancelled transfer? + + # check multi-pg send buffers for timeout + for bufid in list(self._multi_pg_snd_buffer): + buf = self._multi_pg_snd_buffer[bufid] if buf['deadline'] > now: if next_wakeup > buf['deadline']: next_wakeup = buf['deadline'] else: # deadline reached - logger.info('Deadline reached for rcv_buffer src 0x%02X dst 0x%02X', buf['src_address'], buf['dest_address'] ) - if buf['dest_address'] != ParameterGroupNumber.Address.GLOBAL: - self.__send_tp_abort(buf['dest_address'], buf['src_address'], buf['session'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) - del self._rcv_buffer[bufid] - self.__put_rts_cts_session(buf['session']) + frame_format, session_num, src_address, dst_address = self._buffer_unhash_mpg(bufid) + self.__send_multi_pg(frame_format, buf['cpg'], src_address, dst_address) + del self._multi_pg_snd_buffer[bufid] + + # check send buffers + for bufid in list(self._snd_buffer): + buf = self._snd_buffer[bufid] + if buf['deadline'] != 0: + if buf['deadline'] > now: + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] else: - del self._rcv_buffer[bufid] - self.__put_bam_session(buf['session']) - # TODO: should we notify our CAs about the cancelled transfer? - - # check multi-pg send buffers for timeout - # using 'list(x)' to prevent 'RuntimeError: dictionary changed size during iteration' - for bufid in list(self._multi_pg_snd_buffer): - buf = self._multi_pg_snd_buffer[bufid] - if buf['deadline'] > now: - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - else: - # deadline reached - frame_format, session_num, src_address, dst_address = self._buffer_unhash_mpg(bufid) - - self.__send_multi_pg(frame_format, buf['cpg'], src_address, dst_address) + # deadline reached + if buf['state'] == self.SendBufferState.WAITING_CTS: + logger.info('Deadline WAITING_CTS reached for snd_buffer src 0x%02X dst 0x%02X', buf['src_address'], buf['dest_address'] ) + self.__send_tp_abort(buf['src_address'], buf['dest_address'], buf['session'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) + del self._snd_buffer[bufid] + self.__put_rts_cts_session(buf['session']) + # TODO: should we notify our CAs about the cancelled transfer? + + elif buf['state'] == self.SendBufferState.SENDING_RTS_CTS: + while buf['next_packet_to_send'] < buf['num_segments']: + package = buf['next_packet_to_send'] + self.__send_tp_dt(buf['src_address'], buf['dest_address'], buf['session'], package+1, buf['data'][package]) + + buf['next_packet_to_send'] += 1 + # send end of message status + if (package+1) == buf['num_segments']: + self.__send_tp_eom_status(buf['src_address'], buf['dest_address'], buf['session'], buf['message_size'], buf['num_segments'], buf['pgn']) + buf['deadline'] = time.monotonic() + self.Timeout.T5 + buf['state'] = self.SendBufferState.WAITING_EOM_ACK + break + elif package == buf['next_wait_on_cts']: + # wait on next cts + buf['state'] = self.SendBufferState.WAITING_CTS + buf['deadline'] = time.monotonic() + self.Timeout.T3 + break + elif self._minimum_tp_rts_cts_dt_interval is not None: + buf['deadline'] = time.monotonic() + self._minimum_tp_rts_cts_dt_interval + break - del self._multi_pg_snd_buffer[bufid] + # recalc next wakeup + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + elif buf['state'] == self.SendBufferState.WAITING_EOM_ACK: + # TODO: should we inform the application about the eom ack timeout? + del self._snd_buffer[bufid] + self.__put_rts_cts_session(buf['session']) - # check send buffers - # using 'list(x)' to prevent 'RuntimeError: dictionary changed size during iteration' - for bufid in list(self._snd_buffer): - buf = self._snd_buffer[bufid] - if buf['deadline'] != 0: - if buf['deadline'] > now: - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - else: - # deadline reached - if buf['state'] == self.SendBufferState.WAITING_CTS: - logger.info('Deadline WAITING_CTS reached for snd_buffer src 0x%02X dst 0x%02X', buf['src_address'], buf['dest_address'] ) - self.__send_tp_abort(buf['src_address'], buf['dest_address'], buf['session'], self.ConnectionAbortReason.TIMEOUT, buf['pgn']) - del self._snd_buffer[bufid] - self.__put_rts_cts_session(buf['session']) - # TODO: should we notify our CAs about the cancelled transfer? + elif buf['state'] == self.SendBufferState.EOM_ACK_RECEIVED: + # TODO: should we inform the application about the successful transmission? + del self._snd_buffer[bufid] + self.__put_rts_cts_session(buf['session']) - elif buf['state'] == self.SendBufferState.SENDING_RTS_CTS: - while buf['next_packet_to_send'] < buf['num_segments']: + elif buf['state'] == self.SendBufferState.SENDING_BAM: + # send next broadcast message... package = buf['next_packet_to_send'] self.__send_tp_dt(buf['src_address'], buf['dest_address'], buf['session'], package+1, buf['data'][package]) - buf['next_packet_to_send'] += 1 - # send end of message status - if (package+1) == buf['num_segments']: - self.__send_tp_eom_status(buf['src_address'], buf['dest_address'], buf['session'], buf['message_size'], buf['num_segments'], buf['pgn']) - buf['deadline'] = time.time() + self.Timeout.T5 - buf['state'] = self.SendBufferState.WAITING_EOM_ACK - break - elif package == buf['next_wait_on_cts']: - # wait on next cts - buf['state'] = self.SendBufferState.WAITING_CTS - buf['deadline'] = time.time() + self.Timeout.T3 - break - elif self._minimum_tp_rts_cts_dt_interval != None: - buf['deadline'] = time.time() + self._minimum_tp_rts_cts_dt_interval - break - - # recalc next wakeup - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - - elif buf['state'] == self.SendBufferState.WAITING_EOM_ACK: - # TODO: should we inform the application about the eom ack timeout? - del self._snd_buffer[bufid] - self.__put_rts_cts_session(buf['session']) - - elif buf['state'] == self.SendBufferState.EOM_ACK_RECEIVED: - # TODO: should we inform the application about the successful transmission? - del self._snd_buffer[bufid] - self.__put_rts_cts_session(buf['session']) - elif buf['state'] == self.SendBufferState.SENDING_BAM: - # send next broadcast message... - package = buf['next_packet_to_send'] - self.__send_tp_dt(buf['src_address'], buf['dest_address'], buf['session'], package+1, buf['data'][package]) - buf['next_packet_to_send'] += 1 - - if buf['next_packet_to_send'] < buf['num_segments']: - buf['deadline'] = time.time() + self._minimum_tp_bam_dt_interval - # recalc next wakeup - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] + if buf['next_packet_to_send'] < buf['num_segments']: + buf['deadline'] = time.monotonic() + self._minimum_tp_bam_dt_interval + # recalc next wakeup + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + else: + buf['state'] = self.SendBufferState.SENDING_EOM_STATUS + # recalc next wakeup + buf['deadline'] = time.monotonic() + self._minimum_tp_bam_dt_interval + if next_wakeup > buf['deadline']: + next_wakeup = buf['deadline'] + + elif buf['state'] == self.SendBufferState.SENDING_EOM_STATUS: + # done + self.__send_tp_eom_status(buf['src_address'], buf['dest_address'], + buf['session'], + buf['message_size'], buf['num_segments'], buf['pgn']) + del self._snd_buffer[bufid] + self.__put_bam_session(buf['session']) + elif buf['state'] == self.SendBufferState.TRANSMISSION_FINISHED: + del self._snd_buffer[bufid] else: - buf['state'] = self.SendBufferState.SENDING_EOM_STATUS - # recalc next wakeup - buf['deadline'] = time.time() + self._minimum_tp_bam_dt_interval - if next_wakeup > buf['deadline']: - next_wakeup = buf['deadline'] - - elif buf['state'] == self.SendBufferState.SENDING_EOM_STATUS: - # done - self.__send_tp_eom_status(buf['src_address'], buf['dest_address'], - buf['session'], - buf['message_size'], buf['num_segments'], buf['pgn']) - del self._snd_buffer[bufid] - self.__put_bam_session(buf['session']) - elif buf['state'] == self.SendBufferState.TRANSMISSION_FINISHED: - del self._snd_buffer[bufid] - else: - logger.critical('unknown SendBufferState %d', buf['state']) - del self._snd_buffer[bufid] + logger.critical('unknown SendBufferState %d', buf['state']) + del self._snd_buffer[bufid] return next_wakeup @@ -505,132 +500,140 @@ def _process_tp_cm(self, mid, dest_address, data, timestamp): segment_num = (data[4] & 0xFF) | ((data[5] & 0xFF) << 8) | ((data[6] & 0xFF) << 16) pgn = (data[9] & 0xFF) | ((data[10] & 0xFF) << 8) | ((data[11] & 0xFF) << 16) - if control_byte == self.TpControlType.RTS: - buffer_hash = self._buffer_hash(session_num, src_address, dest_address) - num_segments = data[7] # Maximum number of segments that can be sent in response to one CTS. + with self._buffer_lock: + if control_byte == self.TpControlType.RTS: + buffer_hash = self._buffer_hash(session_num, src_address, dest_address) + num_segments = data[7] # Maximum number of segments that can be sent in response to one CTS. - if buffer_hash in self._rcv_buffer: - # according SAE J1939-22 we have to send an ABORT if an active - # transmission is already established - self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.BUSY, pgn) - self.__put_rts_cts_session(session_num) - return + if buffer_hash in self._rcv_buffer: + # according SAE J1939-22 we have to send an ABORT if an active + # transmission is already established + self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.BUSY, pgn) + self.__put_rts_cts_session(session_num) + return - # limit max number segments - num_segments = min(num_segments, segment_num) - - # open new buffer for this connection - self._rcv_buffer[buffer_hash] = { - 'pgn': pgn, - 'session': session_num, - 'message_size': message_size, # total message size, number of bytes - 'num_segments': segment_num, # total number of segments - 'next_packet': 1, - 'next_cts_border': min(self._max_cmdt_packets, num_segments), - 'num_segments_max_rec': min(self._max_cmdt_packets, num_segments), - 'data': [], - 'deadline': time.time() + self.Timeout.T2, - 'src_address' : src_address, - 'dest_address' : dest_address, - } - self.__send_tp_cts(dest_address, src_address, session_num, self._rcv_buffer[buffer_hash]['num_segments_max_rec'], 1, pgn) - self.__job_thread_wakeup() + # limit max number segments + num_segments = min(num_segments, segment_num) - elif control_byte == self.TpControlType.CTS: - buffer_hash = self._buffer_hash(session_num, dest_address, src_address) - num_segments = data[7] # Maximum number of segments that can be sent - if buffer_hash not in self._snd_buffer: - self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) - self.__put_rts_cts_session(session_num) - return - if num_segments == 0: - # SAE J1939/22 - # receiver requests a pause - self._snd_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.Th + # open new buffer for this connection + self._rcv_buffer[buffer_hash] = { + 'pgn': pgn, + 'session': session_num, + 'message_size': message_size, # total message size, number of bytes + 'num_segments': segment_num, # total number of segments + 'next_packet': 1, + 'next_cts_border': min(self._max_cmdt_packets, num_segments), + 'num_segments_max_rec': min(self._max_cmdt_packets, num_segments), + 'data': [], + 'deadline': time.monotonic() + self.Timeout.T2, + 'src_address' : src_address, + 'dest_address' : dest_address, + } + self.__send_tp_cts(dest_address, src_address, session_num, self._rcv_buffer[buffer_hash]['num_segments_max_rec'], 1, pgn) self.__job_thread_wakeup() - return - num_segments_all = self._snd_buffer[buffer_hash]['num_segments'] - self._snd_buffer[buffer_hash]['next_packet_to_send'] = segment_num - 1 - segments_to_be_sent = num_segments_all - self._snd_buffer[buffer_hash]['next_packet_to_send'] - if num_segments > num_segments_all: - logger.debug("CTS: Allowed more packets %d than complete transmission %d", num_segments, num_segments_all) - num_segments = num_segments_all - if num_segments > self._max_cmdt_packets: - logger.debug("CTS: Allowed more packets %d than transmitters max-cmdt-number %d", num_segments, self._max_cmdt_packets) - num_segments = self._max_cmdt_packets - if num_segments > segments_to_be_sent: - logger.debug("CTS: Allowed more packets %d than needed to complete transmission %d", num_segments, segments_to_be_sent) - num_segments = segments_to_be_sent - - self._snd_buffer[buffer_hash]['next_wait_on_cts'] = self._snd_buffer[buffer_hash]['next_packet_to_send'] + num_segments - 1 - - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.SENDING_RTS_CTS - self._snd_buffer[buffer_hash]['deadline'] = time.time() # wake up immediately - self.__job_thread_wakeup() + elif control_byte == self.TpControlType.CTS: + buffer_hash = self._buffer_hash(session_num, dest_address, src_address) + num_segments = data[7] # Maximum number of segments that can be sent + if buffer_hash not in self._snd_buffer: + self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) + self.__put_rts_cts_session(session_num) + return + if num_segments == 0: + # SAE J1939/22 + # receiver requests a pause + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.Th + self.__job_thread_wakeup() + return - elif control_byte == self.TpControlType.EOM_STATUS: - buffer_hash = self._buffer_hash(session_num, src_address, dest_address) - if buffer_hash not in self._rcv_buffer: - self.__put_rts_cts_session(session_num) - return - pgn = self._rcv_buffer[buffer_hash]['pgn'] - if (self._rcv_buffer[buffer_hash]['message_size'] == message_size) and (self._rcv_buffer[buffer_hash]['num_segments'] == segment_num): - self.__notify_subscribers(mid.priority, pgn, src_address, dest_address, timestamp, self._rcv_buffer[buffer_hash]['data']) - if dest_address != ParameterGroupNumber.Address.GLOBAL: - self.__send_tp_eom_ack(dest_address, src_address, session_num, message_size, segment_num, pgn) - else: - self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) - del self._rcv_buffer[buffer_hash] - self.__put_rts_cts_session(session_num) - - elif control_byte == self.TpControlType.EOM_ACK: - buffer_hash = self._buffer_hash(session_num, dest_address, src_address) - if buffer_hash not in self._snd_buffer: - self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) - self.__put_rts_cts_session(session_num) - return - # TODO: should we inform the application about the successful transmission? - # Notify subscribers here to be used for the memory access server to know when to send operation complete - self.__notify_subscribers(mid.priority, pgn, mid.source_address, dest_address, timestamp, data) - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.EOM_ACK_RECEIVED - self._snd_buffer[buffer_hash]['deadline'] = time.time() # wake up immediately - self.__job_thread_wakeup() + num_segments_all = self._snd_buffer[buffer_hash]['num_segments'] + self._snd_buffer[buffer_hash]['next_packet_to_send'] = segment_num - 1 + segments_to_be_sent = num_segments_all - self._snd_buffer[buffer_hash]['next_packet_to_send'] + if num_segments > num_segments_all: + logger.debug("CTS: Allowed more packets %d than complete transmission %d", num_segments, num_segments_all) + num_segments = num_segments_all + if num_segments > self._max_cmdt_packets: + logger.debug("CTS: Allowed more packets %d than transmitters max-cmdt-number %d", num_segments, self._max_cmdt_packets) + num_segments = self._max_cmdt_packets + if num_segments > segments_to_be_sent: + logger.debug("CTS: Allowed more packets %d than needed to complete transmission %d", num_segments, segments_to_be_sent) + num_segments = segments_to_be_sent + + self._snd_buffer[buffer_hash]['next_wait_on_cts'] = self._snd_buffer[buffer_hash]['next_packet_to_send'] + num_segments - 1 + + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.SENDING_RTS_CTS + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() # wake up immediately + self.__job_thread_wakeup() - # BAM FD.TP.CM received - elif control_byte == self.TpControlType.BAM: - buffer_hash = self._buffer_hash(session_num, src_address, dest_address) - if buffer_hash in self._rcv_buffer: - # buffer already in use - logger.info('bam receive buffer already in use 0x%x', buffer_hash ) + elif control_byte == self.TpControlType.EOM_STATUS: + buffer_hash = self._buffer_hash(session_num, src_address, dest_address) + if buffer_hash not in self._rcv_buffer: + self.__put_rts_cts_session(session_num) + return + pgn = self._rcv_buffer[buffer_hash]['pgn'] + if (self._rcv_buffer[buffer_hash]['message_size'] == message_size) and (self._rcv_buffer[buffer_hash]['num_segments'] == segment_num): + if pgn == ParameterGroupNumber.PGN.COMMANDED_ADDRESS: + # route Commanded Address (J1939-81) to the registered CAs + # and consume it (do not forward to generic subscribers, + # consistent with ADDRESSCLAIM/REQUEST handling in notify()) + for ca in self._cas: + ca._process_commanded_address(src_address, self._rcv_buffer[buffer_hash]['data'], timestamp) + else: + self.__notify_subscribers(mid.priority, pgn, src_address, dest_address, timestamp, self._rcv_buffer[buffer_hash]['data']) + if dest_address != ParameterGroupNumber.Address.GLOBAL: + self.__send_tp_eom_ack(dest_address, src_address, session_num, message_size, segment_num, pgn) + else: + self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) del self._rcv_buffer[buffer_hash] - self.__put_bam_session(self._rcv_buffer['session']) - return + self.__put_rts_cts_session(session_num) - # init new buffer for this connection - self._rcv_buffer[buffer_hash] = { - 'pgn': pgn, - 'session': session_num, - 'message_size': message_size, # Total message size, number of bytes - 'num_segments': segment_num, # Total number of segments - 'next_packet': 1, - 'data': [], - 'deadline': time.time() + self.Timeout.T1, - 'src_address' : src_address, - 'dest_address' : dest_address, - } - self.__job_thread_wakeup() + elif control_byte == self.TpControlType.EOM_ACK: + buffer_hash = self._buffer_hash(session_num, dest_address, src_address) + if buffer_hash not in self._snd_buffer: + self.__send_tp_abort(dest_address, src_address, session_num, self.ConnectionAbortReason.RESOURCES, pgn) + self.__put_rts_cts_session(session_num) + return + # TODO: should we inform the application about the successful transmission? + # Notify subscribers here to be used for the memory access server to know when to send operation complete + self.__notify_subscribers(mid.priority, pgn, mid.source_address, dest_address, timestamp, data) + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.EOM_ACK_RECEIVED + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() # wake up immediately + self.__job_thread_wakeup() - elif control_byte == self.TpControlType.ABORT: - # if abort received before transmission established -> cancel transmission - buffer_hash = self._buffer_hash(session_num, dest_address, src_address) - if buffer_hash in self._snd_buffer and self._snd_buffer[buffer_hash]['state'] == self.SendBufferState.WAITING_CTS: - # cancel transmission - self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED - self._snd_buffer[buffer_hash]['deadline'] = time.time() - # TODO: any more abort responses? - else: - raise RuntimeError('Received TP.CM with unknown control_byte %d', control_byte) + # BAM FD.TP.CM received + elif control_byte == self.TpControlType.BAM: + buffer_hash = self._buffer_hash(session_num, src_address, dest_address) + if buffer_hash in self._rcv_buffer: + # buffer already in use + logger.info('bam receive buffer already in use 0x%x', buffer_hash ) + del self._rcv_buffer[buffer_hash] + self.__put_bam_session(session_num) + return + + # init new buffer for this connection + self._rcv_buffer[buffer_hash] = { + 'pgn': pgn, + 'session': session_num, + 'message_size': message_size, # Total message size, number of bytes + 'num_segments': segment_num, # Total number of segments + 'next_packet': 1, + 'data': [], + 'deadline': time.monotonic() + self.Timeout.T1, + 'src_address' : src_address, + 'dest_address' : dest_address, + } + self.__job_thread_wakeup() + + elif control_byte == self.TpControlType.ABORT: + # if abort received before transmission established -> cancel transmission + buffer_hash = self._buffer_hash(session_num, dest_address, src_address) + if buffer_hash in self._snd_buffer and self._snd_buffer[buffer_hash]['state'] == self.SendBufferState.WAITING_CTS: + # cancel transmission + self._snd_buffer[buffer_hash]['state'] = self.SendBufferState.TRANSMISSION_FINISHED + self._snd_buffer[buffer_hash]['deadline'] = time.monotonic() + # TODO: any more abort responses? + else: + raise RuntimeError('Received TP.CM with unknown control_byte %d', control_byte) def _process_tp_dt(self, mid, dest_address, data, timestamp): @@ -640,7 +643,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) @@ -649,48 +651,49 @@ def _process_tp_dt(self, mid, dest_address, data, timestamp): return buffer_hash = self._buffer_hash(session_num, src_address, dest_address) - if buffer_hash not in self._rcv_buffer: - logger.critical('buffer error process dt 0x%x', buffer_hash) - return - if self._rcv_buffer[buffer_hash]['next_packet'] != segment_num: - logger.critical('packet error. required: '+ str(self._rcv_buffer[buffer_hash]['next_packet']) + ' received: ' + str(segment_num) ) - return + with self._buffer_lock: + if buffer_hash not in self._rcv_buffer: + logger.critical('buffer error process dt 0x%x', buffer_hash) + return - # get data - self._rcv_buffer[buffer_hash]['data'].extend(data[4:]) + if self._rcv_buffer[buffer_hash]['next_packet'] != segment_num: + logger.critical('packet error. required: '+ str(self._rcv_buffer[buffer_hash]['next_packet']) + ' received: ' + str(segment_num) ) + return - self._rcv_buffer[buffer_hash]['next_packet'] = segment_num + 1 + # get data + self._rcv_buffer[buffer_hash]['data'].extend(data[4:]) - # message is complete with sending an acknowledge - if len(self._rcv_buffer[buffer_hash]['data']) >= self._rcv_buffer[buffer_hash]['message_size']: - logger.info('finished RCV of PGN {} with size {}'.format(self._rcv_buffer[buffer_hash]['pgn'], self._rcv_buffer[buffer_hash]['message_size'])) - # shorten data to message_size - self._rcv_buffer[buffer_hash]['data'] = self._rcv_buffer[buffer_hash]['data'][:self._rcv_buffer[buffer_hash]['message_size']] - # finished reassembly - if dest_address != ParameterGroupNumber.Address.GLOBAL: - # set deadlin for waiting on eom status - self._rcv_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.T1 - self.__job_thread_wakeup() - return + self._rcv_buffer[buffer_hash]['next_packet'] = segment_num + 1 - # send clear to send - if (dest_address != ParameterGroupNumber.Address.GLOBAL) and (segment_num >= self._rcv_buffer[buffer_hash]['next_cts_border']): - # send cts - number_of_packets_that_can_be_sent = min( self._rcv_buffer[buffer_hash]['num_segments_max_rec'], self._rcv_buffer[buffer_hash]['num_segments'] - self._rcv_buffer[buffer_hash]['next_cts_border'] ) - next_packet_to_be_sent = self._rcv_buffer[buffer_hash]['next_cts_border'] + 1 - self.__send_tp_cts(dest_address, src_address, session_num, number_of_packets_that_can_be_sent, next_packet_to_be_sent, self._rcv_buffer[buffer_hash]['pgn']) + # message is complete with sending an acknowledge + if len(self._rcv_buffer[buffer_hash]['data']) >= self._rcv_buffer[buffer_hash]['message_size']: + logger.info('finished RCV of PGN {} with size {}'.format(self._rcv_buffer[buffer_hash]['pgn'], self._rcv_buffer[buffer_hash]['message_size'])) + # shorten data to message_size + self._rcv_buffer[buffer_hash]['data'] = self._rcv_buffer[buffer_hash]['data'][:self._rcv_buffer[buffer_hash]['message_size']] + # finished reassembly + if dest_address != ParameterGroupNumber.Address.GLOBAL: + # set deadline for waiting on eom status + self._rcv_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.T1 + self.__job_thread_wakeup() + return - # calculate next packet number at which a CTS is to be sent - self._rcv_buffer[buffer_hash]['next_cts_border'] = min(self._rcv_buffer[buffer_hash]['next_cts_border'] + self._rcv_buffer[buffer_hash]['num_segments_max_rec'], - self._rcv_buffer[buffer_hash]['num_segments']) + # send clear to send + if (dest_address != ParameterGroupNumber.Address.GLOBAL) and (segment_num >= self._rcv_buffer[buffer_hash]['next_cts_border']): + # send cts + number_of_packets_that_can_be_sent = min( self._rcv_buffer[buffer_hash]['num_segments_max_rec'], self._rcv_buffer[buffer_hash]['num_segments'] - self._rcv_buffer[buffer_hash]['next_cts_border'] ) + next_packet_to_be_sent = self._rcv_buffer[buffer_hash]['next_cts_border'] + 1 + self.__send_tp_cts(dest_address, src_address, session_num, number_of_packets_that_can_be_sent, next_packet_to_be_sent, self._rcv_buffer[buffer_hash]['pgn']) - self._rcv_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.T2 - self.__job_thread_wakeup() - return + # calculate next packet number at which a CTS is to be sent + self._rcv_buffer[buffer_hash]['next_cts_border'] = min(self._rcv_buffer[buffer_hash]['next_cts_border'] + self._rcv_buffer[buffer_hash]['num_segments_max_rec'], + self._rcv_buffer[buffer_hash]['num_segments']) - self._rcv_buffer[buffer_hash]['deadline'] = time.time() + self.Timeout.T1 - #self.__job_thread_wakeup() + self._rcv_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.T2 + self.__job_thread_wakeup() + return + + self._rcv_buffer[buffer_hash]['deadline'] = time.monotonic() + self.Timeout.T1 def _process_multi_pg(self, mid : MessageId, dest_address, data, timestamp): # currently "SAE J1939 with no assurance data" trailer format supported only @@ -709,7 +712,15 @@ def _process_multi_pg(self, mid : MessageId, dest_address, data, timestamp): payload_length = (data[3] & 0xFF) if (tos == 2) and (trailer_format == 0): # SAE J1939 with no assurance data - self.__notify_subscribers(mid.priority, cpgn, src_address, dest_address, timestamp, data[4:(4+payload_length)].copy()) + payload = data[4:(4+payload_length)].copy() + if cpgn == ParameterGroupNumber.PGN.COMMANDED_ADDRESS: + # route Commanded Address (J1939-81) to the registered CAs and + # consume it (do not forward to generic subscribers, consistent + # with ADDRESSCLAIM/REQUEST handling in notify()) + for ca in self._cas: + ca._process_commanded_address(src_address, payload, timestamp) + else: + self.__notify_subscribers(mid.priority, cpgn, src_address, dest_address, timestamp, payload) else: # TODO print('other tos/tf formats currently not supported') @@ -737,7 +748,7 @@ def __send_tp_bam(self, priority, src_address, session_num, pgn_value, message_s self.__send_tp_cm(src_address, ParameterGroupNumber.Address.GLOBAL, self.TpControlType.BAM, session_num, message_size, num_segments, 0xFF , 0, pgn_value, priority) def __send_tp_cm(self, src_address, dest_address, - TpControlType : TpControlType, session_num, message_size, + tp_control_type: TpControlType, session_num, message_size, num_segments, # total number of segments or next segment number to be sent byte_7, # maximum number of segments or num of segments that can be sent or assurance data Size byte_8, # assurance data type or request code or teason code: @@ -748,7 +759,7 @@ def __send_tp_cm(self, src_address, dest_address, mid = MessageId(priority=priority, parameter_group_number=pgn_tp_cm.value, source_address=src_address) data = [0] * 12 - data[0] = ( (TpControlType & 0xF) | ((session_num & 0xF) << 4)) + data[0] = ( (tp_control_type & 0xF) | ((session_num & 0xF) << 4)) data[1] = ( message_size & 0xFF ) data[2] = ( (message_size >> 8) & 0xFF ) data[3] = ( (message_size >> 16) & 0xFF ) @@ -778,7 +789,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) None: """ Makes an overarching Memory access class + Spawns a background servicer thread tied to the lifetime of this + instance. Call :meth:`stop` (or use the instance as a context + manager) when done. The instance is also registered as a dependent + of the parent ECU, so ``ecu.stop()`` will cascade and tear this + instance down automatically. + :param ca: Controller Application """ self._ca = ca self.query = j1939.Dm14Query(ca) self.server = j1939.DM14Server(ca) - self.proceed = False + self._proceed_event = threading.Event() self._ca.subscribe(self._listen_for_dm14) self.state = DMState.IDLE self.seed_security = False self._notify_query_received = None self._proceed_function = None + self._stopped = False + self._stop_lock = threading.Lock() self._job_thread_end = threading.Event() self._job_thread = threading.Thread(target=self._servicer, name='j1939.memory_access servicer_thread') # A thread can be flagged as a "daemon thread". The significance of @@ -36,21 +50,87 @@ def __init__(self, ca: j1939.ControllerApplication) -> None: self._job_thread.daemon = True self._job_thread.start() - def __del__(self): + # Register with the parent ECU so ecu.stop() cascades to this instance. + # Done after the thread has started so a failed registration during + # shutdown is still recoverable by the user calling stop() directly. + try: + self._ca.register_dependent(self) + except Exception: + # If registration fails (e.g. ECU already stopping) we still want + # the user to be able to stop us manually; just log and continue. + logger.exception("Failed to register MemoryAccess with ECU") + + def stop(self, timeout: float = 2.0) -> None: + """Stop the background servicer thread and release resources. + + Idempotent: subsequent calls are no-ops. Safe to call from any + thread, including from inside ``ecu.stop()``'s cascade. + + :param float timeout: + Maximum time in seconds to wait for the servicer thread to exit. + """ + with self._stop_lock: + if self._stopped: + return + self._stopped = True + + # Signal shutdown and wake the servicer immediately so it does not + # have to wait out its full poll interval. self._job_thread_end.set() + self._proceed_event.set() + if self._job_thread.is_alive(): - self._job_thread.join() + self._job_thread.join(timeout=timeout) + + # Best-effort cleanup of the CA-level subscription. If the CA/ECU + # is already torn down this may raise; that is fine. + try: + self._ca.unsubscribe(self._listen_for_dm14) + except Exception: + pass + + # Best-effort removal from the ECU's dependent registry. If we are + # being called from inside the cascade this is a no-op (the registry + # has already been cleared); if we are being called explicitly it + # prevents a stale reference. + try: + self._ca.unregister_dependent(self) + except Exception: + pass + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + self.stop() + return False + + def __del__(self): + # Defensive backstop only. The primary cleanup paths are explicit + # stop() / context-manager exit / ecu.stop() cascade. Guard against + # partial __init__ (where _job_thread may not exist) and swallow all + # exceptions per the __del__ contract. + try: + if getattr(self, '_job_thread', None) is None: + return + self.stop() + except Exception: + pass def _servicer(self): """ - Job thread to service memory access requests + Job thread to service memory access requests. + + Blocks on a threading.Event instead of busy-polling """ while not self._job_thread_end.is_set(): - if (self.state == DMState.WAIT_RESPONSE) and self.proceed: - self.proceed = False + triggered = self._proceed_event.wait(timeout=1.0) + if self._job_thread_end.is_set(): + return + if triggered and self.state == DMState.WAIT_RESPONSE: + self._proceed_event.clear() if self._notify_query_received is not None: self._notify_query_received() # notify incoming request - time.sleep(0.001) # Add a small delay to yield control to other threads def _handle_error(self, priority: int, pgn: int, sa: int, timestamp: int, data: bytearray, error_code: int) -> None: @@ -94,7 +174,9 @@ def _listen_for_dm14( self.state = DMState.WAIT_RESPONSE self._ca.unsubscribe(self._listen_for_dm14) if self._proceed_function is not None: - self.proceed = self._proceed_function( + if self.server.address is None: + raise RuntimeError("server address must be set before calling proceed function") + proceed = self._proceed_function( self.server.command, int.from_bytes( bytes=self.server.address, @@ -109,21 +191,29 @@ def _listen_for_dm14( self.server.access_level, 0x0, # placeholder for seed ) # call proceed function and pass in basic parameters - if not self.proceed: + if not proceed: self._handle_error(priority, pgn, sa, timestamp, data, 0x100) + else: + self._proceed_event.set() else: - self.proceed = True # no security, so always proceed + self._proceed_event.set() # no security, so always proceed case DMState.REQUEST_STARTED: self.server.parse_dm14(priority, pgn, sa, timestamp, data) if self.server.state == j1939.ResponseState.SEND_PROCEED: self.state = DMState.WAIT_RESPONSE if self.seed_security: + if self.server.seed is None: + raise RuntimeError("server seed must be set before verifying key") + if self.server.key is None: + raise RuntimeError("server key must be set before verifying key") if self.server.verify_key( self.server.seed, self.server.key ): if self._proceed_function is not None: - self.proceed = self._proceed_function( + if self.server.address is None: + raise RuntimeError("server address must be set before calling proceed function") + proceed = self._proceed_function( self.server.command, int.from_bytes( bytes=self.server.address, @@ -138,10 +228,12 @@ def _listen_for_dm14( self.server.access_level, self.server.seed, ) # call proceed function and pass in basic parameters - if not self.proceed: + if not proceed: self._handle_error(priority, pgn, sa, timestamp, data, 0x100) + else: + self._proceed_event.set() else: - self.proceed = True # no proceed function, so always proceed + self._proceed_event.set() # no proceed function, so always proceed else: self._handle_error(priority, pgn, sa, timestamp, data, 0x1003) @@ -158,11 +250,11 @@ def _listen_for_dm14( def respond( self, proceed: bool, - data: list = None, + data: list | None = None, error: int = 0xFFFFFF, edcp: int = 0xFF, max_timeout: int = 3, - ) -> list: + ) -> list | None: """ Responds with requested data and error code, if applicable, to a read request @@ -178,7 +270,7 @@ def respond( if self.state is not DMState.WAIT_RESPONSE: return data - self.proceed = False + self._proceed_event.clear() self._ca.unsubscribe(self._listen_for_dm14) return_data = self.server.respond(proceed, data, error, edcp, max_timeout) self.state = DMState.SERVER_CLEANUP if self.server.state.value != DMState.IDLE.value else DMState.IDLE @@ -254,44 +346,44 @@ def write( ) self.reset() - def set_seed_generator(self, seed_generator: callable) -> None: + def set_seed_generator(self, seed_generator: Callable[[], int]) -> None: """ Sets seed generator function to use :param seed_generator: seed generator function """ self.server.set_seed_generator(seed_generator) - def set_seed_key_algorithm(self, algorithm: callable) -> None: + def set_seed_key_algorithm(self, algorithm: Callable[[int], int]) -> None: """ Sets seed-key algorithm to be used for key generation - :param callable algorithm: seed-key algorithm + :param algorithm: seed-key algorithm """ self.seed_security = True self.query.set_seed_key_algorithm(algorithm) self.server.set_seed_key_algorithm(algorithm) - def set_verify_key(self, verify_key: callable) -> None: + def set_verify_key(self, verify_key: Callable[..., bool]) -> None: """ Sets verify key function to be used for verifying the key - :param callable verify_key: verify key function + :param verify_key: verify key function """ self.server.set_verify_key(verify_key) - def set_notify(self, notify: callable) -> None: + def set_notify(self, notify: Callable[[], None]) -> None: """ Sets notify function to be used for notifying the user of memory accesses - :param callable notify: notify function + :param notify: notify function """ self._notify_query_received = notify - def set_proceed(self, proceed: callable) -> None: + def set_proceed(self, proceed: Callable[..., bool]) -> None: """ Sets proceed function to determine if a memory query is valid or not - :param callable proceed: proceed function + :param proceed: proceed function """ self._proceed_function = proceed @@ -304,4 +396,4 @@ def reset(self) -> None: self._ca.subscribe(self._listen_for_dm14) self.server.reset_server() self.query.reset_query() - self.proceed = False + self._proceed_event.clear() 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..e302908 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). @@ -33,7 +34,7 @@ class PGN: ADDRESSCLAIM = 60928 # EE00 DATATRANSFER = 60160 # EB00 TP_CM = 60416 # EC00 - #COMMANDED_ADDRESS = 65240 + COMMANDED_ADDRESS = 65240 # FED8 #PROPRIETARY_A = 61184 #SOFTWARE_IDENT = 65242 # Diagnostic messages diff --git a/j1939/version.py b/j1939/version.py index 6463fd4..b3f4756 100644 --- a/j1939/version.py +++ b/j1939/version.py @@ -1 +1 @@ -__version__ = "2.0.12" \ No newline at end of file +__version__ = "0.1.2" diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..193c8f2 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,90 @@ +[build-system] +requires = ["setuptools>=77", "wheel"] +build-backend = "setuptools.build_meta" + +[project] +name = "python-can-j1939" +dynamic = ["version"] +description = "SAE J1939 stack implementation (fork of can-j1939 by Juergen Heilgemeir)" +readme = { file = "README.md", content-type = "text/markdown" } +license = "MIT" +license-files = ["LICENSE"] +authors = [ + { name = "Raul Sainz-Maza" }, + { name = "Drew Rife" }, + { name = "Grant Allan" }, + { name = "Koltan Hauersperger" }, + { name = "Mahesh Sharma" }, + { name = "Todd Snider" }, + { name = "Victor Klueber" }, +] +maintainers = [ + { name = "Raul Sainz-Maza" }, +] +keywords = ["CAN", "SAE", "J1939", "J1939-FD", "J1939-22"] +classifiers = [ + "Development Status :: 4 - Beta", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Intended Audience :: Developers", + "Topic :: Scientific/Engineering", +] +requires-python = ">=3.10" +dependencies = [ + "python-can >= 4.2.0", +] + +[project.optional-dependencies] +test = [ + "pytest >= 6.2.5", +] +lint = [ + "ruff", + "pyright >= 1.1", + "vermin", +] + +[project.urls] +Homepage = "https://github.com/RaulSMS/python-can-j1939" +"Bug Tracker" = "https://github.com/RaulSMS/python-can-j1939/issues" +Documentation = "https://python-can-j1939.readthedocs.io/en/latest/" + +[tool.setuptools.dynamic] +version = { attr = "j1939.version.__version__" } + +[tool.setuptools.packages.find] +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/pyrightconfig.json b/pyrightconfig.json new file mode 100644 index 0000000..b021440 --- /dev/null +++ b/pyrightconfig.json @@ -0,0 +1,8 @@ +{ + "include": ["j1939"], + "exclude": ["test", "examples", "docs"], + "pythonVersion": "3.10", + "typeCheckingMode": "basic", + "reportMissingImports": true, + "reportMissingTypeStubs": false +} diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index 7c2b287..0000000 --- a/setup.cfg +++ /dev/null @@ -1,2 +0,0 @@ -[bdist_wheel] -universal = 1 \ No newline at end of file diff --git a/setup.py b/setup.py deleted file mode 100644 index 8885ff7..0000000 --- a/setup.py +++ /dev/null @@ -1,38 +0,0 @@ -from setuptools import setup, find_packages, Extension - -exec(open('j1939/version.py').read()) - -description = open("README.rst").read() -# Change links to stable documentation -description = description.replace("/latest/", "/stable/") - -setup( - name="can-j1939", - url="https://github.com/juergenH87/python-can-j1939", - version=__version__, - packages=find_packages(exclude=['docs', 'examples']), - author="Juergen Heilgemeir", - description="SAE J1939 stack implementation", - keywords="CAN SAE J1939 J1939-FD J1939-22", - long_description=description, - long_description_content_type='text/x-rst', - license="MIT", - platforms=["any"], - classifiers=[ - "License :: OSI Approved :: MIT License", - "Operating System :: OS Independent", - "Programming Language :: Python :: 3", - "Intended Audience :: Developers", - "Topic :: Scientific/Engineering" - ], - install_requires=[ - "python-can >= 3.3.4", - "numpy >= 1.17.0", - "pytest >= 6.2.5", - ], - include_package_data=True, - - # Tests can be run using `python setup.py test` - test_suite="nose.collector", - tests_require=["nose"] -) diff --git a/test_helpers/__init__.py b/test/__init__.py similarity index 100% rename from test_helpers/__init__.py rename to test/__init__.py diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 0000000..e3f14bf --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,41 @@ +import threading + +import pytest + +from test.helpers.feeder import Feeder + + +@pytest.fixture() +def feeder(): + # setup + f = Feeder() + try: + yield f + finally: + # teardown — guarantee cleanup even if the test raises + try: + f.stop() + except Exception: + pass + + +@pytest.fixture(autouse=True) +def _assert_no_j1939_thread_leak(): + """Fail any test that leaves a j1939.* background thread alive.""" + before = {t.ident for t in threading.enumerate() + if t.name.startswith('j1939.')} + yield + # Give freshly-stopped threads a brief moment to actually exit. + import time + for _ in range(20): + leaked = [t for t in threading.enumerate() + if t.name.startswith('j1939.') + and t.ident not in before + and t.is_alive()] + if not leaked: + break + time.sleep(0.01) + assert not leaked, ( + "Test leaked j1939 background thread(s): " + + ", ".join(t.name for t in leaked) + ) diff --git a/test/helpers/__init__.py b/test/helpers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test_helpers/feeder.py b/test/helpers/feeder.py similarity index 76% rename from test_helpers/feeder.py rename to test/helpers/feeder.py index 6ce6be9..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 @@ -41,8 +41,13 @@ class MsgType(object): def __init__(self): self.STOP_THREAD = object() + self._stopped = False self.message_queue = queue.Queue() - self.message_thread = threading.Thread(target=self._async_can_feeder) + self.message_thread = threading.Thread( + target=self._async_can_feeder, name='j1939.test feeder_thread') + # Daemon so a stray feeder cannot prevent interpreter exit if a test + # forgets to call stop(). + self.message_thread.daemon = True self.message_thread.start() # redirect the send_message from the can bus to our simulation self.ecu = j1939.ElectronicControlUnit(send_message=self._send_message) @@ -56,7 +61,15 @@ def _async_can_feeder(self): recv_time = message[3] if recv_time == 0.0: recv_time = time.time() - self.ecu.notify(message[1], message[2], recv_time) + try: + self.ecu.notify(message[1], message[2], recv_time) + except Exception: + # An assertion failure inside a subscriber callback (e.g. + # Feeder._on_message) must not kill the feeder thread + # silently — that previously produced + # PytestUnhandledThreadExceptionWarning and left the feeder + # unable to process further messages. Log and continue. + logger.exception("Feeder _async_can_feeder: notify failed") def _inject_messages_into_ecu(self): while self.can_messages and self.can_messages[0][0] == Feeder.MsgType.CANRX: @@ -70,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 @@ -93,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 @@ -144,6 +157,17 @@ def process_messages(self): self.ecu.unsubscribe(self._on_message) def stop(self): - self.ecu.stop() + if self._stopped: + return + self._stopped = True + try: + self.ecu.stop() + except Exception: + logger.exception("Feeder.stop: ecu.stop() failed") self.message_queue.put(self.STOP_THREAD) - self.message_thread.join() + self.message_thread.join(timeout=2.0) + if self.message_thread.is_alive(): + raise RuntimeError( + "Feeder thread did not exit within timeout; " + "possible thread leak or blocked _async_can_feeder" + ) diff --git a/test/test_ca.py b/test/test_ca.py index 6f25a86..192298c 100644 --- a/test/test_ca.py +++ b/test/test_ca.py @@ -1,8 +1,7 @@ import time import j1939 -from test_helpers.feeder import Feeder -from test_helpers.conftest import feeder +from test.helpers.feeder import Feeder def address_claim( @@ -69,12 +68,12 @@ def test_addr_claim_fixed_reduced_time(feeder): ) new_ca = feeder.ecu.add_ca(name=name, device_address=128) new_ca.start(0.2) - - # wait until all messages are processed asynchronously - # rounded up to account for scheduling delays - time.sleep(0.3) - # assert that the expected message was sent + # wait until the address claim message is processed, with a 2s timeout + deadline = time.monotonic() + 2.0 + while len(feeder.can_messages) > 0 and time.monotonic() < deadline: + time.sleep(0.050) + assert len(feeder.can_messages) == 0 @@ -181,3 +180,197 @@ def test_stop_method(feeder): assert new_ca.started new_ca.stop() assert not new_ca.started + + +def test_bypass_address_claim_with_address_zero(feeder): + """bypass_address_claim=True with device_address_preferred=0x00 must reach State.NORMAL. + + Address 0x00 (engine controller / ECM) is valid but falsy in Python. + A truthiness check on the address silently skips the bypass, leaving the + CA in State.NONE and causing RuntimeError on the first send. Regression + test for issue #8. + """ + name = j1939.Name( + arbitrary_address_capable=0, + industry_group=j1939.Name.IndustryGroup.Global, + vehicle_system_instance=0, + vehicle_system=0, + function=0, + function_instance=0, + ecu_instance=0, + manufacturer_code=0, + identity_number=0, + ) + ca = j1939.ControllerApplication(name=name, device_address_preferred=0x00, bypass_address_claim=True) + assert ca.state == j1939.ControllerApplication.State.NORMAL + assert ca.device_address == 0x00 + + +def _commanded_address_name(arbitrary_address_capable=0): + """NAME used by the Commanded Address tests. + + With arbitrary_address_capable=0 the NAME serializes to + [135, 214, 82, 83, 130, 201, 254, 82]; with =1 only the most significant + byte changes (bit 63 set) to 210. + """ + return j1939.Name( + arbitrary_address_capable=arbitrary_address_capable, + industry_group=j1939.Name.IndustryGroup.Industrial, + vehicle_system_instance=2, + vehicle_system=127, + function=201, + function_instance=16, + ecu_instance=2, + manufacturer_code=666, + identity_number=1234567, + ) + + +def test_commanded_address_claims_new_address(feeder): + """A BAM Commanded Address (PGN 65240) whose NAME matches an + arbitrary-address-capable CA causes that CA to claim the new address. + The new address (100) is < 128 so it is claimed immediately. + """ + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240, 9 bytes, 2 packets + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 135, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 (NAME bytes 0..6) + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 210, 100, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 (NAME byte 7 + new SA 100) + (Feeder.MsgType.CANTX, 0x18EEFF64, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @100 + ] + + name = _commanded_address_name(arbitrary_address_capable=1) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + time.sleep(0.500) + + assert new_ca.state == j1939.ControllerApplication.State.NORMAL + assert new_ca.device_address == 100 + + +def test_commanded_address_ignored_for_other_name(feeder): + """A Commanded Address with a NAME that does not match the CA is ignored.""" + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 136, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 (NAME byte 0 differs) + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 210, 100, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 + ] + + name = _commanded_address_name(arbitrary_address_capable=1) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + time.sleep(0.500) + + assert new_ca.device_address == 128 + + +def test_commanded_address_ignored_when_not_arbitrary_capable(feeder): + """A CA that is not arbitrary-address-capable (default policy) does not + adopt a matching Commanded Address. + """ + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 82], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 135, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 82, 100, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 (matching NAME, new SA 100) + ] + + name = _commanded_address_name(arbitrary_address_capable=0) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + time.sleep(0.500) + + assert new_ca.device_address == 128 + + +def test_commanded_address_not_delivered_to_subscribers(feeder): + """Commanded Address is consumed by the CA and not forwarded to generic + subscribers. + """ + received_pgns = [] + + def on_message(priority, pgn, sa, timestamp, data): + received_pgns.append(pgn) + + feeder.ecu.subscribe(on_message) + + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 135, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 210, 100, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 + (Feeder.MsgType.CANTX, 0x18EEFF64, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @100 + ] + + name = _commanded_address_name(arbitrary_address_capable=1) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + time.sleep(0.500) + + feeder.ecu.unsubscribe(on_message) + + assert j1939.ParameterGroupNumber.PGN.COMMANDED_ADDRESS not in received_pgns + + +def test_commanded_address_invalid_sa_ignored(feeder): + """A Commanded Address that commands a non-claimable source address (NULL + 254 / GLOBAL 255) is ignored; the CA keeps its current address and does not + transmit an Address Claimed for the invalid SA. + """ + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 135, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 210, 254, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 (new SA = NULL 254) + ] + + name = _commanded_address_name(arbitrary_address_capable=1) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + time.sleep(0.500) + + assert new_ca.state == j1939.ControllerApplication.State.NORMAL + assert new_ca.device_address == 128 + + +def test_commanded_address_in_veto_range_claims_after_veto(feeder): + """A Commanded Address for an address in the 128..247 range enters WAIT_VETO + and resolves to NORMAL at the commanded address. The re-armed veto timeout + makes the transition happen within the veto window rather than at the next + periodic claim tick. + """ + feeder.can_messages = [ + (Feeder.MsgType.CANTX, 0x18EEFF80, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @128 + (Feeder.MsgType.CANRX, 0x1CECFF01, [32, 9, 0, 2, 255, 216, 254, 0], 0.0), # TP.CM BAM, PGN 65240 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [1, 135, 214, 82, 83, 130, 201, 254], 0.0), # TP.DT 1 + (Feeder.MsgType.CANRX, 0x1CEBFF01, [2, 210, 200, 255, 255, 255, 255, 255], 0.0), # TP.DT 2 (new SA = 200) + (Feeder.MsgType.CANTX, 0x18EEFFC8, [135, 214, 82, 83, 130, 201, 254, 210], 0.0), # Address Claimed @200 + ] + + name = _commanded_address_name(arbitrary_address_capable=1) + new_ca = feeder.ecu.add_ca(name=name, device_address=128) + new_ca.start() + + while len(feeder.can_messages) > 0: + time.sleep(0.500) + # allow the (re-armed) veto window to elapse + time.sleep(0.500) + + assert new_ca.state == j1939.ControllerApplication.State.NORMAL + assert new_ca.device_address == 200 diff --git a/test/test_dtc_conversion_methods.py b/test/test_dtc_conversion_methods.py new file mode 100644 index 0000000..f646c7a --- /dev/null +++ b/test/test_dtc_conversion_methods.py @@ -0,0 +1,85 @@ +"""Tests for the SAE J1939-73 SPN conversion methods (CM 1, 2, 3, 4).""" +import pytest + +from j1939.diagnostic_messages import DTC + + +@pytest.mark.parametrize("cm", [1, 2, 3, 4]) +@pytest.mark.parametrize("spn", [0, 123, 456, 0x12345, 0x3FFFF]) +@pytest.mark.parametrize("fmi", [0, 1, 5, 31]) +@pytest.mark.parametrize("oc", [0, 1, 42, 127]) +def test_dtc_round_trip(cm, spn, fmi, oc): + """Encoded DTC bytes must decode back to the same SPN/FMI/OC/CM.""" + encoded = DTC(spn=spn, fmi=fmi, oc=oc, cm=cm) + decoded = DTC(dtc=encoded.dtc, cm=cm) + assert decoded.spn == spn + assert decoded.fmi == fmi + assert decoded.oc == oc + assert decoded.cm == cm + + +@pytest.mark.parametrize("cm,expected_cm_bit", [(1, 1), (2, 1), (3, 1), (4, 0)]) +def test_cm_bit_on_wire(cm, expected_cm_bit): + """Only CM 4 has the CM bit cleared; CMs 1/2/3 set it.""" + d = DTC(spn=1000, fmi=5, oc=3, cm=cm) + assert ((d.dtc >> 31) & 0x01) == expected_cm_bit + + +def test_cm1_byte_layout_matches_reference(): + """CM 1 layout: b1=SPN[18:11], b2=SPN[10:3], b3=SPN[2:0]|FMI, b4=OC|CM.""" + spn, fmi, oc = 0x12345, 5, 3 + d = DTC(spn=spn, fmi=fmi, oc=oc, cm=1) + b1 = d.dtc & 0xFF + b2 = (d.dtc >> 8) & 0xFF + b3 = (d.dtc >> 16) & 0xFF + b4 = (d.dtc >> 24) & 0xFF + assert b1 == (spn >> 11) & 0xFF + assert b2 == (spn >> 3) & 0xFF + assert b3 == (((spn & 0x07) << 5) | (fmi & 0x1F)) + assert b4 == ((oc & 0x7F) | 0x80) + + +def test_cm2_byte_layout_matches_reference(): + """CM 2 layout: b1=SPN[10:3], b2=SPN[18:11], b3=SPN[2:0]|FMI, b4=OC|CM.""" + spn, fmi, oc = 0x12345, 5, 3 + d = DTC(spn=spn, fmi=fmi, oc=oc, cm=2) + b1 = d.dtc & 0xFF + b2 = (d.dtc >> 8) & 0xFF + b3 = (d.dtc >> 16) & 0xFF + b4 = (d.dtc >> 24) & 0xFF + assert b1 == (spn >> 3) & 0xFF + assert b2 == (spn >> 11) & 0xFF + assert b3 == (((spn & 0x07) << 5) | (fmi & 0x1F)) + assert b4 == ((oc & 0x7F) | 0x80) + + +@pytest.mark.parametrize("cm", [3, 4]) +def test_cm3_cm4_byte_layout(cm): + """CM 3/4 layout: SPN packed little-endian in b1/b2 with top 3 bits in b3.""" + spn, fmi, oc = 0x12345, 5, 3 + d = DTC(spn=spn, fmi=fmi, oc=oc, cm=cm) + b1 = d.dtc & 0xFF + b2 = (d.dtc >> 8) & 0xFF + b3 = (d.dtc >> 16) & 0xFF + b4 = (d.dtc >> 24) & 0xFF + assert b1 == spn & 0xFF + assert b2 == (spn >> 8) & 0xFF + assert b3 == ((((spn >> 16) & 0x07) << 5) | (fmi & 0x1F)) + expected_b4 = oc & 0x7F + if cm == 3: + expected_b4 |= 0x80 + assert b4 == expected_b4 + + +def test_invalid_cm_raises(): + with pytest.raises(ValueError): + DTC(spn=1, fmi=1, oc=0, cm=5) + with pytest.raises(ValueError): + DTC(dtc=0x12345678, cm=0) + + +def test_default_cm_is_4(): + """Backward compatibility: omitting `cm` produces the modern CM 4 layout.""" + d = DTC(spn=0x12345, fmi=5, oc=3) + assert d.cm == 4 + assert ((d.dtc >> 31) & 0x01) == 0 diff --git a/test/test_ecu.py b/test/test_ecu.py index 65f8eb9..fab0be8 100644 --- a/test/test_ecu.py +++ b/test/test_ecu.py @@ -1,39 +1,18 @@ import time import can +import pytest + import j1939 -from test_helpers.feeder import Feeder -from test_helpers.conftest 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): +from j1939.j1939_21 import J1939_21 +from j1939.message_id import MessageId +from test.helpers.feeder import Feeder + +# def test_connect(self): # self.feeder.ecu.connect(bustype="virtual", channel=1) # self.feeder.ecu.disconnect() + def test_broadcast_receive_short(feeder): """Test the receivement of a normal broadcast message @@ -50,6 +29,7 @@ def test_broadcast_receive_short(feeder): feeder.receive() + def test_broadcast_receive_long(feeder): """Test the receivement of a long broadcast message @@ -59,17 +39,120 @@ def test_broadcast_receive_long(feeder): feeder.accept_all_messages() feeder.can_messages = [ - (Feeder.MsgType.CANRX, 0x00ECFF01, [32, 20, 0, 3, 255, 0xB0, 0xFE, 0], 0.0), # TP.CM BAM (to global Address) - (Feeder.MsgType.CANRX, 0x00EBFF01, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 - (Feeder.MsgType.CANRX, 0x00EBFF01, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 - (Feeder.MsgType.CANRX, 0x00EBFF01, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 + ( + Feeder.MsgType.CANRX, + 0x00ECFF01, + [32, 20, 0, 3, 255, 0xB0, 0xFE, 0], + 0.0, + ), # TP.CM BAM (to global Address) + (Feeder.MsgType.CANRX, 0x00EBFF01, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 + (Feeder.MsgType.CANRX, 0x00EBFF01, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 + (Feeder.MsgType.CANRX, 0x00EBFF01, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 ] - feeder.pdus = [(Feeder.MsgType.PDU, 65200, [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6])] + feeder.pdus = [ + ( + Feeder.MsgType.PDU, + 65200, + [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6], + ) + ] feeder.receive() +def test_broadcast_receive_out_of_sequence_packet_raises(): + """Reject and terminate a BAM session with an invalid sequence number.""" + sent = [] + notified = [] + dll = J1939_21( + send_message=lambda *args: sent.append(args), + job_thread_wakeup=lambda: None, + notify_subscribers=lambda *args: notified.append(args), + max_cmdt_packets=1, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=None, + ecu_is_message_acceptable=lambda dest: True, + ) + bam_mid = MessageId(can_id=0x00ECFF01) + dll._process_tp_cm(bam_mid, 0xFF, [32, 20, 0, 3, 255, 0xB0, 0xFE, 0], 0.0) + buffer_hash = dll._buffer_hash(0x01, 0xFF) + mid = MessageId(can_id=0x00EBFF01) + + with pytest.raises(ValueError, match='out of sequence'): + dll._process_tp_dt(mid, 0xFF, [2, 8, 9, 10, 11, 12, 13, 14], 0.0) + + assert sent == [] + assert notified == [] + assert buffer_hash not in dll._rcv_buffer + + +def test_peer_to_peer_receive_out_of_sequence_packet_aborts(feeder): + """Reject an out-of-sequence CMDT packet and abort the session.""" + sent = [] + dll = J1939_21( + send_message=lambda *args: sent.append(args), + job_thread_wakeup=lambda: None, + notify_subscribers=lambda *args: None, + max_cmdt_packets=1, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=None, + ecu_is_message_acceptable=lambda dest: True, + ) + rts_mid = MessageId(can_id=0x00EC0201) + dll._process_tp_cm(rts_mid, 0x02, [16, 20, 0, 3, 1, 0, 223, 0], 0.0) + sent.clear() + + dt_mid = MessageId(can_id=0x00EB0201) + with pytest.raises(ValueError, match='out of sequence'): + dll._process_tp_dt(dt_mid, 0x02, [2, 1, 2, 3, 4, 5, 6, 7], 0.0) + + assert sent == [ + ( + 0x1CEC0102, + True, + [255, 7, 255, 255, 255, 0, 223, 0], + ) + ] + assert not dll._rcv_buffer + + dll._process_tp_cm(rts_mid, 0x02, [16, 20, 0, 3, 1, 0, 223, 0], 0.0) + sent.clear() + + with pytest.raises(ValueError, match='out of sequence'): + dll._process_tp_dt(dt_mid, 0x02, [0, 1, 2, 3, 4, 5, 6, 7], 0.0) + + assert sent[0][2][1] == 7 + + +def test_peer_to_peer_sequence_gap_after_valid_packet_aborts(feeder): + """Reject a sequence gap without delivering a partial RTS/CTS payload.""" + sent = [] + notified = [] + dll = J1939_21( + send_message=lambda *args: sent.append(args), + job_thread_wakeup=lambda: None, + notify_subscribers=lambda *args: notified.append(args), + max_cmdt_packets=2, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=None, + ecu_is_message_acceptable=lambda dest: True, + ) + rts_mid = MessageId(can_id=0x00EC0201) + dll._process_tp_cm(rts_mid, 0x02, [16, 20, 0, 3, 2, 0, 223, 0], 0.0) + sent.clear() + + dt_mid = MessageId(can_id=0x00EB0201) + dll._process_tp_dt(dt_mid, 0x02, [1, 1, 2, 3, 4, 5, 6, 7], 0.0) + + with pytest.raises(ValueError, match='out of sequence'): + dll._process_tp_dt(dt_mid, 0x02, [3, 8, 9, 10, 11, 12, 13, 14], 0.0) + + assert sent[0][2][1] == 7 + assert notified == [] + assert not dll._rcv_buffer + + def test_peer_to_peer_receive_short(feeder): """Test the receivement of a normal peer-to-peer message @@ -79,13 +162,14 @@ def test_peer_to_peer_receive_short(feeder): feeder.accept_all_messages() feeder.can_messages = [ - (Feeder.MsgType.CANRX, 0x00DC0201, [1, 2, 3, 4, 5, 6, 7, 8], 0.0), # TP.CM RTS + (Feeder.MsgType.CANRX, 0x00DC0201, [1, 2, 3, 4, 5, 6, 7, 8], 0.0), # TP.CM RTS ] feeder.pdus = [(Feeder.MsgType.PDU, 56320, [1, 2, 3, 4, 5, 6, 7, 8], 0)] feeder.receive() + def test_peer_to_peer_receive_long(feeder): """Test the receivement of a long peer-to-peer message @@ -95,20 +179,52 @@ def test_peer_to_peer_receive_long(feeder): feeder.accept_all_messages() # TODO: we have to select another PGN here! This one is for broadcasting only! feeder.can_messages = [ - (Feeder.MsgType.CANRX, 0x00EC0201, [16, 20, 0, 3, 1, 176, 254, 0], 0.0), # TP.CM RTS - (Feeder.MsgType.CANTX, 0x1CEC0102, [17, 1, 1, 255, 255, 176, 254, 0], 0.0), # TP.CM CTS 1 - (Feeder.MsgType.CANRX, 0x00EB0201, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 - (Feeder.MsgType.CANTX, 0x1CEC0102, [17, 1, 2, 255, 255, 176, 254, 0], 0.0), # TP.CM CTS 2 - (Feeder.MsgType.CANRX, 0x00EB0201, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 - (Feeder.MsgType.CANTX, 0x1CEC0102, [17, 1, 3, 255, 255, 176, 254, 0], 0.0), # TP.CM CTS 3 - (Feeder.MsgType.CANRX, 0x00EB0201, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 - (Feeder.MsgType.CANTX, 0x1CEC0102, [19, 20, 0, 3, 255, 176, 254, 0], 0.0), # TP.CM EOMACK + ( + Feeder.MsgType.CANRX, + 0x00EC0201, + [16, 20, 0, 3, 1, 176, 254, 0], + 0.0, + ), # TP.CM RTS + ( + Feeder.MsgType.CANTX, + 0x1CEC0102, + [17, 1, 1, 255, 255, 176, 254, 0], + 0.0, + ), # TP.CM CTS 1 + (Feeder.MsgType.CANRX, 0x00EB0201, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 + ( + Feeder.MsgType.CANTX, + 0x1CEC0102, + [17, 1, 2, 255, 255, 176, 254, 0], + 0.0, + ), # TP.CM CTS 2 + (Feeder.MsgType.CANRX, 0x00EB0201, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 + ( + Feeder.MsgType.CANTX, + 0x1CEC0102, + [17, 1, 3, 255, 255, 176, 254, 0], + 0.0, + ), # TP.CM CTS 3 + (Feeder.MsgType.CANRX, 0x00EB0201, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 + ( + Feeder.MsgType.CANTX, + 0x1CEC0102, + [19, 20, 0, 3, 255, 176, 254, 0], + 0.0, + ), # TP.CM EOMACK ] - feeder.pdus = [(Feeder.MsgType.PDU, 65200, [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6])] + feeder.pdus = [ + ( + Feeder.MsgType.PDU, + 65200, + [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6], + ) + ] feeder.receive() + def test_peer_to_peer_send_short(feeder): """Test sending of a short peer-to-peer message @@ -116,7 +232,7 @@ def test_peer_to_peer_send_short(feeder): Its length is 8 Bytes. The contained values are bogous of cause. """ feeder.can_messages = [ - (Feeder.MsgType.CANTX, 0x18F09B90, [1, 2, 3, 4, 5, 6, 7, 8], 0.0), # PGN 61440 + (Feeder.MsgType.CANTX, 0x18F09B90, [1, 2, 3, 4, 5, 6, 7, 8], 0.0), # PGN 61440 ] pdu = (Feeder.MsgType.PDU, 61440, [1, 2, 3, 4, 5, 6, 7, 8]) @@ -133,22 +249,52 @@ def test_peer_to_peer_send_long(feeder): feeder.accept_all_messages() feeder.can_messages = [ - (Feeder.MsgType.CANTX, 0x18EC9B90, [16, 20, 0, 3, 1, 0, 223, 0], 0.0), # TP.CM RTS 1 - (Feeder.MsgType.CANRX, 0x1CEC909B, [17, 1, 1, 255, 255, 0, 223, 0], 0.0), # TP.CM CTS 1 - (Feeder.MsgType.CANTX, 0x1CEB9B90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 - (Feeder.MsgType.CANRX, 0x1CEC909B, [17, 1, 2, 255, 255, 0, 223, 0], 0.0), # TP.CM CTS 2 - (Feeder.MsgType.CANTX, 0x1CEB9B90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 - (Feeder.MsgType.CANRX, 0x1CEC909B, [17, 1, 3, 255, 255, 0, 223, 0], 0.0), # TP.CM CTS 3 - (Feeder.MsgType.CANTX, 0x1CEB9B90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 - (Feeder.MsgType.CANRX, 0x1CEC909B, [19, 20, 0, 3, 255, 0, 223, 0], 0.0), # TP.CM EOMACK + ( + Feeder.MsgType.CANTX, + 0x18EC9B90, + [16, 20, 0, 3, 1, 0, 223, 0], + 0.0, + ), # TP.CM RTS 1 + ( + Feeder.MsgType.CANRX, + 0x1CEC909B, + [17, 1, 1, 255, 255, 0, 223, 0], + 0.0, + ), # TP.CM CTS 1 + (Feeder.MsgType.CANTX, 0x1CEB9B90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 + ( + Feeder.MsgType.CANRX, + 0x1CEC909B, + [17, 1, 2, 255, 255, 0, 223, 0], + 0.0, + ), # TP.CM CTS 2 + (Feeder.MsgType.CANTX, 0x1CEB9B90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 + ( + Feeder.MsgType.CANRX, + 0x1CEC909B, + [17, 1, 3, 255, 255, 0, 223, 0], + 0.0, + ), # TP.CM CTS 3 + (Feeder.MsgType.CANTX, 0x1CEB9B90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 + ( + Feeder.MsgType.CANRX, + 0x1CEC909B, + [19, 20, 0, 3, 255, 0, 223, 0], + 0.0, + ), # TP.CM EOMACK ] feeder.pdus = [(Feeder.MsgType.PDU, 57088, None)] - pdu = (Feeder.MsgType.PDU, 57088, [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6]) + pdu = ( + Feeder.MsgType.PDU, + 57088, + [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6], + ) feeder.send(pdu, 144, 155) + def test_broadcast_send_long(feeder): """Test sending of a long broadcast message (with BAM) @@ -156,16 +302,26 @@ def test_broadcast_send_long(feeder): Its length is 20 Bytes. The contained values are bogous of cause. """ feeder.can_messages = [ - (Feeder.MsgType.CANTX, 0x18ECFF90, [32, 20, 0, 3, 255, 176, 254, 0], 0.0), # TP.BAM - (Feeder.MsgType.CANTX, 0x1CEBFF90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 - (Feeder.MsgType.CANTX, 0x1CEBFF90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 - (Feeder.MsgType.CANTX, 0x1CEBFF90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 + ( + Feeder.MsgType.CANTX, + 0x18ECFF90, + [32, 20, 0, 3, 255, 176, 254, 0], + 0.0, + ), # TP.BAM + (Feeder.MsgType.CANTX, 0x1CEBFF90, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 1 + (Feeder.MsgType.CANTX, 0x1CEBFF90, [2, 1, 2, 3, 4, 5, 6, 7], 0.0), # TP.DT 2 + (Feeder.MsgType.CANTX, 0x1CEBFF90, [3, 1, 2, 3, 4, 5, 6, 255], 0.0), # TP.DT 3 ] - pdu = (Feeder.MsgType.PDU, 65200, [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6]) + pdu = ( + Feeder.MsgType.PDU, + 65200, + [1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6, 7, 1, 2, 3, 4, 5, 6], + ) feeder.send(pdu, 144, pdu[1]) + def test_add_bus(feeder): """ Test adding and removing a bus to the ECU @@ -174,7 +330,8 @@ 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): """ @@ -186,7 +343,63 @@ 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_notifier_after_notifier_stop_still_delivers(feeder): + """A listener stopped via notifier.stop() must work when re-added later. + + Regression test for a bug where ElectronicControlUnit.add_notifier() + re-added the ECU's own MessageListener (created once, in __init__, and + reused for the ECU's whole lifetime) to a new notifier without + resetting listener.stopped -- can.Notifier.stop() sets that flag + permanently on every listener it holds, so once *any* notifier this + ECU had been added to was stopped, every future notifier it was added + to (even a brand new one) silently dropped every frame forever, + despite the notifier itself being alive and the listener being + registered on it. + """ + bus = can.interface.Bus(interface="virtual", channel="notifier-reset-test") + try: + feeder.ecu.add_bus(bus) + + notifier1 = can.Notifier(bus=bus, listeners=[]) + feeder.ecu.add_notifier(notifier1) + notifier1.stop() + feeder.ecu.remove_notifier() + + notifier2 = can.Notifier(bus=bus, listeners=[]) + feeder.ecu.add_notifier(notifier2) + + received = [] + feeder.ecu.subscribe( + lambda priority, pgn, sa, timestamp, data: received.append(data) + ) + + sender = can.interface.Bus(interface="virtual", channel="notifier-reset-test") + try: + msg = can.Message( + arbitration_id=0x18FEB201, + data=[1, 2, 3, 4, 5, 6, 7, 8], + is_extended_id=True, + ) + sender.send(msg) + + for _ in range(50): + if received: + break + time.sleep(0.01) + + assert received, ( + "Frame was not delivered after re-adding to a new notifier -- " + "listener.stopped was not reset" + ) + finally: + notifier2.stop() + sender.shutdown() + finally: + bus.shutdown() + def test_add_bus_filters(feeder): """ @@ -195,12 +408,13 @@ def test_add_bus_filters(feeder): bus = can.interface.Bus(interface="virtual", channel=1) feeder.ecu.add_bus(bus) filters = [ - {'can_id': 0x123, 'can_mask': 0x7FF, 'extended': True}, - {'can_id': 0x456, 'can_mask': 0x7FF} + {"can_id": 0x123, "can_mask": 0x7FF, "extended": True}, + {"can_id": 0x456, "can_mask": 0x7FF}, ] feeder.ecu.add_bus_filters(filters) assert feeder.ecu._bus.filters == filters + def test_subscribe(feeder): """ Test subscribing to callback @@ -212,7 +426,7 @@ def callback(priority: int, pgn: int, sa: int, timestamp: int, data: bytearray): call_count += 1 feeder.ecu.subscribe(callback) - + feeder.can_messages = [ (Feeder.MsgType.CANRX, 0x00FEB201, [1, 2, 3, 4, 5, 6, 7, 8], 0.0), ] @@ -222,3 +436,128 @@ def callback(priority: int, pgn: int, sa: int, timestamp: int, data: bytearray): feeder.receive() assert call_count == 1 + + +def test_constructor_accepts_bus_instance(): + """Passing a bus instance to the constructor stores it without calling connect().""" + bus = can.interface.Bus(interface="virtual", channel="test_ctor_bus") + ecu = None + try: + ecu = j1939.ElectronicControlUnit(bus=bus) + assert ecu._bus is bus + assert ecu._notifier is None # connect() not yet called + assert ecu._bus_created is False # bus was not created by this ECU + finally: + if ecu is not None: + ecu.stop() + bus.shutdown() + + +def test_constructor_bus_none_by_default(): + """Without a bus= argument, _bus starts as None.""" + ecu = j1939.ElectronicControlUnit(send_message=lambda *a, **kw: None) + try: + assert ecu._bus is None + assert ecu._bus_created is False + finally: + ecu.stop() + + +def test_constructor_invalid_data_link_layer_raises(): + """An unsupported data_link_layer string raises ValueError immediately.""" + import pytest + + with pytest.raises(ValueError, match="j1939-21.*j1939-22"): + j1939.ElectronicControlUnit(data_link_layer="j1939-99") + + +def test_connect_with_preexisting_bus_sets_notifier(): + """When a bus is passed to __init__, connect() sets up the notifier + without creating a new bus and without emitting a DeprecationWarning. + """ + import warnings + + bus = can.interface.Bus(interface="virtual", channel="test_connect_prebus") + ecu = None + try: + ecu = j1939.ElectronicControlUnit(bus=bus) + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + returned_bus = ecu.connect() + + # No DeprecationWarning: bus was already provided + deprecations = [x for x in w if issubclass(x.category, DeprecationWarning)] + assert deprecations == [], ( + "connect() must not warn when bus was provided in constructor" + ) + + assert returned_bus is bus + assert ecu._notifier is not None + assert ecu._bus is bus + finally: + if ecu is not None: + ecu.disconnect() + ecu.stop() + bus.shutdown() + + +def test_connect_without_preexisting_bus_emits_deprecation_warning(): + """When connect() creates the bus itself (legacy path), it emits a + DeprecationWarning advising the caller to pass bus= to the constructor. + """ + import warnings + + ecu = j1939.ElectronicControlUnit(send_message=lambda *a, **kw: None) + try: + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + ecu.connect(interface="virtual", channel="test_connect_legacy") + + deprecations = [x for x in w if issubclass(x.category, DeprecationWarning)] + assert len(deprecations) == 1 + assert "deprecated" in str(deprecations[0].message).lower() + assert ecu._bus_created is True + finally: + ecu.disconnect() + ecu.stop() + + +def test_disconnect_before_connect_raises_runtime_error(): + """Calling disconnect() before connect() raises RuntimeError (previously + would crash with AttributeError/NoneType errors). + """ + import pytest + + ecu = j1939.ElectronicControlUnit(send_message=lambda *a, **kw: None) + try: + with pytest.raises(RuntimeError): + ecu.disconnect() + finally: + ecu.stop() + + +def test_disconnect_does_not_shutdown_external_bus(): + """When a bus was passed to __init__ (not created by connect()), disconnect() + must NOT call bus.shutdown() — the caller owns the bus lifecycle. + """ + shutdown_called = [] + + class TrackingBus(can.interfaces.virtual.VirtualBus): + def shutdown(self): + shutdown_called.append(True) + super().shutdown() + + bus = TrackingBus(channel="test_disconnect_external") + ecu = None + try: + ecu = j1939.ElectronicControlUnit(bus=bus) + ecu.connect() + ecu.disconnect() + + assert shutdown_called == [], ( + "disconnect() must not shutdown a bus that was provided externally" + ) + finally: + if ecu is not None: + ecu.stop() + bus.shutdown() diff --git a/test/test_j1939_22.py b/test/test_j1939_22.py new file mode 100644 index 0000000..852ee7a --- /dev/null +++ b/test/test_j1939_22.py @@ -0,0 +1,348 @@ +""" +Tests for J1939-22 transport protocol chunking logic. + +This module tests the data chunking algorithm used in J1939-22 for splitting +large messages into transport protocol segments of 60 bytes each. +""" +import pytest + +import j1939 +from j1939.j1939_22 import J1939_22 +from j1939.message_id import FrameFormat, MessageId +from j1939.parameter_group_number import ParameterGroupNumber + + +class TestChunkingAlgorithm: + """Isolated tests for the data chunking algorithm.""" + + @staticmethod + def chunk_data(data, chunk_size): + """Pure-Python chunking implementation matching j1939_22.py:send_pgn()""" + data_length = len(data) + return [list(data[i:i + chunk_size]) for i in range(0, data_length, chunk_size)] + + @pytest.mark.parametrize("data_length,expected_chunks,expected_last_chunk_size", [ + (60, 1, 60), + (61, 2, 1), + (119, 2, 59), + (120, 2, 60), + (121, 3, 1), + (180, 3, 60), + (181, 4, 1), + ]) + def test_chunk_sizes(self, data_length, expected_chunks, expected_last_chunk_size): + """Verify correct chunk count and sizes for various data lengths.""" + data = list(range(data_length)) + result = self.chunk_data(data, J1939_22.DataLength.TP) + + assert len(result) == expected_chunks + assert len(result[-1]) == expected_last_chunk_size + + def test_data_integrity(self): + """All original bytes are present after chunking, in correct order.""" + data = list(range(2560)) # Large data set: 43 chunks + result = self.chunk_data(data, J1939_22.DataLength.TP) + + # Verify chunk count + expected_chunks = 2560 // J1939_22.DataLength.TP + (1 if 2560 % J1939_22.DataLength.TP else 0) + assert len(result) == expected_chunks + + # Verify all data preserved in order + reconstructed = [] + for chunk in result: + reconstructed.extend(chunk) + assert reconstructed == data + + def test_chunk_count_matches_num_segments_formula(self): + """Verify chunking matches the num_segments formula used in j1939_22.py.""" + for data_length in [60, 61, 119, 120, 121, 180, 500, 1000]: + data = [i % 256 for i in range(data_length)] + + result = self.chunk_data(data, J1939_22.DataLength.TP) + + # Formula from j1939_22.py + expected = int(data_length / J1939_22.DataLength.TP) + ((data_length % J1939_22.DataLength.TP) != 0) + + assert len(result) == expected, f"data_length={data_length}" + + +class TestJ1939_22Integration: + """Integration tests for J1939-22 chunking through send_pgn.""" + + @staticmethod + def create_j1939_22(): + """Create a J1939_22 instance with mock callbacks.""" + return J1939_22( + send_message=lambda *args, **kwargs: None, + job_thread_wakeup=lambda: None, + notify_subscribers=lambda *args: None, + max_cmdt_packets=16, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=0.010, + ecu_is_message_acceptable=lambda dest: True + ) + + def test_short_message_not_chunked(self, feeder): + """Data <= J1939_22.DataLength.TP bytes uses multi-pg path, not TP chunking.""" + feeder.accept_all_messages() + j1939_22 = self.create_j1939_22() + + result = j1939_22.send_pgn( + data_page=0, pdu_format=0xFE, pdu_specific=0xFF, + priority=7, src_address=0x01, data=list(range(J1939_22.DataLength.TP)), + time_limit=0, frame_format=FrameFormat.CEFF + ) + + assert result is True + assert len(j1939_22._snd_buffer) == 0 + + def test_bam_broadcast_chunking(self, feeder): + """BAM broadcast correctly chunks data and verifies integrity.""" + feeder.accept_all_messages() + j1939_22 = self.create_j1939_22() + + test_data = list(range(121)) # 3 chunks: 60 + 60 + 1 + + result = j1939_22.send_pgn( + data_page=0, pdu_format=0xFE, pdu_specific=0xFF, + priority=7, src_address=0x01, data=test_data, + time_limit=0, frame_format=FrameFormat.CEFF + ) + + assert result is True + buffer = list(j1939_22._snd_buffer.values())[0] + + assert buffer['num_segments'] == 3 + assert len(buffer['data']) == 3 + assert [len(chunk) for chunk in buffer['data']] == [60, 60, 1] + + # Verify data integrity + reconstructed = [] + for chunk in buffer['data']: + reconstructed.extend(chunk) + assert reconstructed == test_data + + def test_rts_cts_peer_to_peer_chunking(self, feeder): + """RTS/CTS peer-to-peer uses different code path but chunks correctly.""" + feeder.accept_all_messages() + j1939_22 = self.create_j1939_22() + + test_data = list(range(180)) # 3 chunks of 60 each + + result = j1939_22.send_pgn( + data_page=0, pdu_format=0xDF, pdu_specific=0x04, # PDU1 = peer-to-peer + priority=7, src_address=0x01, data=test_data, + time_limit=0, frame_format=1 + ) + + assert result is True + buffer = list(j1939_22._snd_buffer.values())[0] + + assert buffer['num_segments'] == 3 + assert all(len(chunk) == 60 for chunk in buffer['data']) + + reconstructed = [] + for chunk in buffer['data']: + reconstructed.extend(chunk) + assert reconstructed == test_data + + @pytest.mark.parametrize("data_length,expected_segments", [ + (61, 2), + (120, 2), + (121, 3), + (180, 3), + (240, 4), + (500, 9), + ]) + def test_various_data_sizes(self, feeder, data_length, expected_segments): + """Parametrized test for segment count across various data sizes.""" + feeder.accept_all_messages() + j1939_22 = self.create_j1939_22() + + test_data = [i % 256 for i in range(data_length)] + + result = j1939_22.send_pgn( + data_page=0, pdu_format=0xFE, pdu_specific=0xFF, + priority=7, src_address=0x01, data=test_data, + time_limit=0, frame_format=FrameFormat.CEFF + ) + + assert result is True + buffer = list(j1939_22._snd_buffer.values())[0] + + assert buffer['num_segments'] == expected_segments + assert len(buffer['data']) == expected_segments + + +class _RecordingCA: + """Minimal CA stub that records Commanded Address routing calls.""" + + def __init__(self): + self.commanded = [] + + def _process_commanded_address(self, src_address, data, timestamp): + self.commanded.append((src_address, list(data), timestamp)) + + def message_acceptable(self, dest_address): + return True + + +class TestCommandedAddressRouting: + """J1939-22 routing of Commanded Address (PGN 65240) to the CAs. + + A 9-byte Commanded Address may arrive either inside a Multi-PG frame or via + FD-TP reassembly; both completion points must route to the CAs and consume + the message (no delivery to generic subscribers). + """ + + COMMANDER_SA = 0xF9 + + @staticmethod + def _make_dll(notify_record): + return J1939_22( + send_message=lambda *a, **k: None, + job_thread_wakeup=lambda: None, + notify_subscribers=lambda *a: notify_record.append(a), + max_cmdt_packets=16, + minimum_tp_rts_cts_dt_interval=None, + minimum_tp_bam_dt_interval=0.010, + ecu_is_message_acceptable=lambda dest: True, + ) + + @staticmethod + def _name_bytes(): + name = j1939.Name( + arbitrary_address_capable=1, + industry_group=j1939.Name.IndustryGroup.Industrial, + vehicle_system_instance=2, + vehicle_system=127, + function=201, + function_instance=16, + ecu_instance=2, + manufacturer_code=666, + identity_number=1234567, + ) + return name.bytes + + def test_commanded_address_routed_from_multi_pg(self): + """A Commanded Address carried in a Multi-PG frame is routed to the CAs + and not delivered to generic subscribers. + """ + notify = [] + dll = self._make_dll(notify) + ca = _RecordingCA() + dll.add_ca(ca) + + payload = self._name_bytes() + [100] # NAME + new SA 100 + cpgn = ParameterGroupNumber.PGN.COMMANDED_ADDRESS # 65240 / 0xFED8 + # C-PG header: tos=2, tf=0 + frame = [ + (2 << 5) | (0 << 2) | ((cpgn >> 16) & 0x3), + (cpgn >> 8) & 0xFF, + cpgn & 0xFF, + len(payload), + ] + payload + + mid = MessageId( + priority=7, + parameter_group_number=ParameterGroupNumber.PGN.FEFF_MULTI_PG | 0xFF, + source_address=self.COMMANDER_SA, + ) + dll._process_multi_pg(mid, ParameterGroupNumber.Address.GLOBAL, frame, 1.0) + + assert len(ca.commanded) == 1 + assert ca.commanded[0][0] == self.COMMANDER_SA + assert ca.commanded[0][1] == payload + # consumed: not forwarded to generic subscribers + assert notify == [] + + def test_other_pgn_in_multi_pg_still_delivered_to_subscribers(self): + """A non-Commanded-Address PGN in a Multi-PG frame is still delivered to + subscribers (routing change must not swallow other PGNs). + """ + notify = [] + dll = self._make_dll(notify) + ca = _RecordingCA() + dll.add_ca(ca) + + payload = [1, 2, 3, 4] + cpgn = 0xFEEE # some broadcast PGN, not Commanded Address + frame = [ + (2 << 5) | (0 << 2) | ((cpgn >> 16) & 0x3), + (cpgn >> 8) & 0xFF, + cpgn & 0xFF, + len(payload), + ] + payload + + mid = MessageId( + priority=7, + parameter_group_number=ParameterGroupNumber.PGN.FEFF_MULTI_PG | 0xFF, + source_address=self.COMMANDER_SA, + ) + dll._process_multi_pg(mid, ParameterGroupNumber.Address.GLOBAL, frame, 1.0) + + assert ca.commanded == [] + assert len(notify) == 1 + # notify args: (priority, pgn, src, dest, timestamp, data) + assert notify[0][1] == cpgn + assert notify[0][5] == payload + + def test_commanded_address_routed_from_fd_tp_bam(self): + """A Commanded Address reassembled via FD-TP (BAM) is routed to the CAs + on EOM_STATUS and not delivered to generic subscribers. + """ + notify = [] + dll = self._make_dll(notify) + ca = _RecordingCA() + dll.add_ca(ca) + + payload = self._name_bytes() + [100] # NAME + new SA 100 + pgn = ParameterGroupNumber.PGN.COMMANDED_ADDRESS + session = 0 + message_size = len(payload) # 9 bytes -> 1 segment + num_segments = 1 + dest = ParameterGroupNumber.Address.GLOBAL + + cm_mid = MessageId( + priority=7, + parameter_group_number=(ParameterGroupNumber.PGN.FD_TP_CM & 0x1FF00) | dest, + source_address=self.COMMANDER_SA, + ) + dt_mid = MessageId( + priority=7, + parameter_group_number=(ParameterGroupNumber.PGN.FD_TP_DT & 0x1FF00) | dest, + source_address=self.COMMANDER_SA, + ) + + # FD.TP.CM BAM + cm_bam = [ + (J1939_22.TpControlType.BAM & 0xF) | ((session & 0xF) << 4), + message_size & 0xFF, (message_size >> 8) & 0xFF, (message_size >> 16) & 0xFF, + num_segments & 0xFF, (num_segments >> 8) & 0xFF, (num_segments >> 16) & 0xFF, + 0xFF, 0x00, + pgn & 0xFF, (pgn >> 8) & 0xFF, (pgn >> 16) & 0xFF, + ] + dll._process_tp_cm(cm_mid, dest, cm_bam, 1.0) + + # FD.TP.DT segment 1 + dt = [ + (0 & 0xF) | ((session & 0xF) << 4), + 1, 0, 0, + ] + payload + dll._process_tp_dt(dt_mid, dest, dt, 1.0) + + # FD.TP.CM EOM_STATUS triggers delivery + cm_eom = [ + (J1939_22.TpControlType.EOM_STATUS & 0xF) | ((session & 0xF) << 4), + message_size & 0xFF, (message_size >> 8) & 0xFF, (message_size >> 16) & 0xFF, + num_segments & 0xFF, (num_segments >> 8) & 0xFF, (num_segments >> 16) & 0xFF, + 0x00, 0x00, + pgn & 0xFF, (pgn >> 8) & 0xFF, (pgn >> 16) & 0xFF, + ] + dll._process_tp_cm(cm_mid, dest, cm_eom, 1.0) + + assert len(ca.commanded) == 1 + assert ca.commanded[0][0] == self.COMMANDER_SA + assert ca.commanded[0][1] == payload + # consumed: not forwarded to generic subscribers + assert notify == [] diff --git a/test/test_memory_access.py b/test/test_memory_access.py index ffa7977..fe908fc 100644 --- a/test/test_memory_access.py +++ b/test/test_memory_access.py @@ -1,8 +1,9 @@ +import time + import pytest -from test_helpers.feeder import Feeder -from test_helpers.conftest import feeder + import j1939 -import time +from test.helpers.feeder import Feeder # fmt: off read_with_seed_key = [ @@ -523,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() @@ -629,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, @@ -677,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_passive_observation.py b/test/test_passive_observation.py new file mode 100644 index 0000000..6daee6e --- /dev/null +++ b/test/test_passive_observation.py @@ -0,0 +1,113 @@ +"""Passive observation vs. active participation for directed (PDU1) traffic. + +A subscriber registered without an exact owned address (a wildcard +``device_address=None`` monitor) must be able to *observe* directed peer-to-peer +frames addressed to another node, but the stack must not *participate* in the +connection-mode transport protocol (it must not answer RTS with CTS/EOM-ACK) for +destinations it does not own. +""" + +import time + +import j1939 + + +def _make_ecu(): + """Create an ECU whose transmissions are recorded instead of sent.""" + sent = [] + + def record_send(can_id, extended_id, data, fd_format=False): + sent.append((can_id, list(data))) + + ecu = j1939.ElectronicControlUnit(send_message=record_send) + return ecu, sent + + +def test_wildcard_observes_directed_single_frame(): + """A wildcard subscriber receives a directed single-frame PDU1 addressed to + a third node, and the stack transmits nothing in response.""" + ecu, sent = _make_ecu() + received = [] + ecu.subscribe( + lambda priority, pgn, sa, timestamp, data: received.append( + (pgn, sa, list(data)) + ) + ) + try: + # Directed single-frame PDU1: PGN 0xEF00 (Proprietary A), + # destination 0x20, source 0xF9 -> arbitration id 0x18EF20F9. + ecu.notify(0x18EF20F9, [1, 2, 3, 4, 5, 6, 7, 8], time.time()) + time.sleep(0.1) + finally: + ecu.stop() + + assert received == [(0xEF00, 0xF9, [1, 2, 3, 4, 5, 6, 7, 8])] + assert sent == [], "passive observation must not transmit on the bus" + + +def test_no_cts_for_rts_directed_at_third_node(): + """A wildcard monitor must not make the stack answer an RTS that is directed + at a third node: no CTS is sent and nothing is reassembled/delivered.""" + ecu, sent = _make_ecu() + received = [] + ecu.subscribe(lambda *args: received.append(args)) + try: + # TP.CM RTS for a 20-byte / 3-packet transfer, destination 0x20, + # source 0xF9 -> arbitration id 0x18EC20F9. + ecu.notify( + 0x18EC20F9, [16, 20, 0, 3, 1, 0xB0, 0xFE, 0], time.time() + ) + time.sleep(0.1) + finally: + ecu.stop() + + assert sent == [], "must not answer CTS for an RTS addressed to another node" + assert received == [], "must not reassemble/deliver a transfer we do not own" + + +def test_wildcard_observes_address_claim(): + """A wildcard subscriber observes address-claim broadcasts (e.g. to read a + node's NAME), and the stack transmits nothing in response.""" + ecu, sent = _make_ecu() + received = [] + ecu.subscribe( + lambda priority, pgn, sa, timestamp, data: received.append( + (pgn, sa, list(data)) + ) + ) + try: + name = [1, 2, 3, 4, 5, 6, 7, 8] + # Address Claimed: PGN 0xEE00, global destination, source 0xB0 + # -> arbitration id 0x18EEFFB0. + ecu.notify(0x18EEFFB0, name, time.time()) + time.sleep(0.1) + finally: + ecu.stop() + + assert received == [(0xEE00, 0xB0, name)] + assert sent == [], "observing an address claim must not transmit on the bus" + + +def test_owned_destination_still_participates(): + """Regression: when a CA owns the destination, the stack still answers the + RTS/CTS handshake (active participation is preserved).""" + ecu, sent = _make_ecu() + + class OwnAllCa(j1939.ControllerApplication): + def message_acceptable(self, dest_address): + return True + + ca = OwnAllCa(None, None, False) + ecu.add_ca(controller_application=ca) + try: + # Same RTS as above; now the destination is owned, so a CTS must be sent. + ecu.notify( + 0x18EC20F9, [16, 20, 0, 3, 1, 0xB0, 0xFE, 0], time.time() + ) + time.sleep(0.1) + finally: + ecu.stop() + + assert sent, "an owned destination must still answer the RTS with a CTS" + # First transmitted frame should be a TP.CM CTS (control byte 17). + assert sent[0][1][0] == 17, "expected a CTS (control byte 17) response" diff --git a/test/test_threading.py b/test/test_threading.py new file mode 100644 index 0000000..d8a292f --- /dev/null +++ b/test/test_threading.py @@ -0,0 +1,762 @@ +""" +Threading safety and lifecycle tests. + +This module tests: +- Timer accuracy and drift prevention (heapq-based scheduling) +- Protocol/timer thread separation (slow callbacks don't block protocol) +- Dispatch queue: Notifier thread unblocked, ordering, drain on stop +- Thread-safe subscriber list operations +- MemoryAccess servicer thread lifecycle +- Dependent registry and cascaded shutdown from ECU +""" + +import threading +import time + +import pytest + +import j1939 +from j1939.parameter_group_number import ParameterGroupNumber +from test.helpers.feeder import Feeder + + +def _make_ecu(): + """Create a mock ECU with no CAN bus.""" + return j1939.ElectronicControlUnit(send_message=lambda *a, **kw: None) + + +def _wait_thread_exit(thread, timeout=0.5): + """Wait for a thread to exit, polling every 10ms. + + :param thread: The thread to wait for. + :param timeout: Maximum time to wait in seconds. + :return: True if thread exited, False if timeout reached. + """ + deadline = time.monotonic() + timeout + while thread.is_alive() and time.monotonic() < deadline: + time.sleep(0.01) + return not thread.is_alive() + + +def _wait_no_threads_named(name, timeout=0.5): + """Wait until no alive threads have the given name. + + :param name: Thread name to check for. + :param timeout: Maximum time to wait in seconds. + :return: True if no matching threads remain, False if timeout reached. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not any(t.name == name and t.is_alive() for t in threading.enumerate()): + return True + time.sleep(0.01) + return False + + +def test_timer_no_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() + + def callback(cookie): + timestamps.append(time.monotonic()) + if len(timestamps) >= 10: + done.set() + return False # stop rescheduling + return True # reschedule + + ecu.add_timer(0.050, callback) + fired = done.wait(timeout=10.0) # generous timeout for very slow CI + ecu.stop() + + 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)] + + # 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): + """A slow application timer callback must not delay BAM reassembly.""" + + slow_fired = threading.Event() + + def slow_callback(cookie): + slow_fired.set() + time.sleep(0.150) # simulate heavy work + return True + + feeder.ecu.add_timer(0.020, slow_callback) + # Wait until the slow callback has fired at least once so it is + # definitely holding the (old single) job thread during the BAM. + slow_fired.wait(timeout=1.0) + + # 20-byte BAM: BAM announce + 3 DT frames + pgn_value = 0xFEC8 # arbitrary broadcast PGN + # Build raw CAN message sequence (same pattern as test_ecu.py) + can_id_bam = 0x1CECFF01 # TP.CM BAM from 0x01 to global + can_id_dt = 0x1CEBFF01 # TP.DT from 0x01 to global + + feeder.can_messages = [ + ( + Feeder.MsgType.CANRX, + can_id_bam, + [32, 20, 0, 3, 255, pgn_value & 0xFF, (pgn_value >> 8) & 0xFF, 0], + 0.0, + ), + (Feeder.MsgType.CANRX, can_id_dt, [1, 1, 2, 3, 4, 5, 6, 7], 0.0), + (Feeder.MsgType.CANRX, can_id_dt, [2, 8, 9, 10, 11, 12, 13, 14], 0.0), + (Feeder.MsgType.CANRX, can_id_dt, [3, 15, 16, 17, 18, 19, 20, 255], 0.0), + ] + + received = threading.Event() + + def on_message(priority, pgn, sa, timestamp, data): + if pgn == pgn_value: + received.set() + + feeder.ecu.subscribe(on_message) + feeder.ecu.accept_all_messages = lambda: None # already set by Feeder init + + feeder.accept_all_messages() + start = time.monotonic() + feeder._inject_messages_into_ecu() + + # BAM with 3 DT frames at 50ms inter-frame gap = ~150ms minimum. + # Allow 400ms — still well under the 150ms slow callback sleeping + # indefinitely on the old single thread. + delivered = received.wait(timeout=0.4) + elapsed = time.monotonic() - start + + feeder.ecu.unsubscribe(on_message) + feeder.ecu.remove_timer(slow_callback) + + assert delivered, ( + f"BAM message was not reassembled within 400ms (elapsed {elapsed * 1000:.0f}ms). " + "Slow callback may be blocking the protocol thread." + ) + + +def test_concurrent_add_remove_no_crash(): + """Concurrent add/remove of timers from multiple threads must not crash or deadlock.""" + ecu = _make_ecu() + errors = [] + + def noop(cookie): + return True + + def hammer(): + try: + deadline = time.monotonic() + 0.3 + while time.monotonic() < deadline: + ecu.add_timer(0.01, noop) + ecu.remove_timer(noop) + except Exception as exc: + errors.append(exc) + + threads = [threading.Thread(target=hammer) for _ in range(5)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=2.0) + assert not t.is_alive(), "Hammer thread deadlocked" + + ecu.stop() + + assert not errors, f"Exceptions during concurrent timer ops: {errors}" + + +def test_memory_access_event_latency(): + """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( + arbitrary_address_capable=0, + industry_group=j1939.Name.IndustryGroup.Industrial, + vehicle_system_instance=1, + vehicle_system=1, + function=1, + function_instance=1, + ecu_instance=1, + manufacturer_code=1, + identity_number=1, + ), + device_address=0x80, + ) + + ma = MemoryAccess(ca) + + callback_times = [] + set_time = [] + + def notify(): + callback_times.append(time.monotonic()) + + ma.set_notify(notify) + ma.state = DMState.WAIT_RESPONSE + + set_time.append(time.monotonic()) + ma._proceed_event.set() + + # 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) + + ecu.stop() + + assert callback_times, "notify callback was never called after _proceed_event.set()" + latency = callback_times[0] - set_time[0] + assert latency < 0.500, ( + f"MemoryAccess notify latency was {latency * 1000:.2f}ms, " + f"expected < 500ms (thread should not be blocked/deadlocked)" + ) + + +def test_subscribe_unsubscribe_race(feeder): + """Concurrent subscribe/unsubscribe while messages arrive must not crash.""" + errors = [] + received_count = [0] + + def counting_cb(priority, pgn, sa, timestamp, data): + received_count[0] += 1 + + def subscribe_loop(): + try: + deadline = time.monotonic() + 0.5 + while time.monotonic() < deadline: + feeder.ecu.subscribe(counting_cb) + time.sleep(0.001) + feeder.ecu.unsubscribe(counting_cb) + except Exception as exc: + errors.append(exc) + + # Keep at least one stable subscriber so messages are delivered + feeder.ecu.subscribe(counting_cb) + + sub_thread = threading.Thread(target=subscribe_loop) + sub_thread.start() + + # Inject broadcast messages repeatedly + can_id = 0x18FEC801 # broadcast from 0x01, PGN 0xFEC8 + inject_deadline = time.monotonic() + 0.5 + while time.monotonic() < inject_deadline: + feeder.message_queue.put( + (Feeder.MsgType.CANRX, can_id, bytearray([1, 2, 3, 4, 5, 6, 7, 8]), 0.0) + ) + time.sleep(0.01) + + sub_thread.join(timeout=2.0) + feeder.ecu.unsubscribe(counting_cb) + + assert not errors, f"Exceptions during subscribe/unsubscribe race: {errors}" + assert received_count[0] > 0, "No messages were received during the race" + + +def _make_ca(ecu, device_address=0x80): + """Create a ControllerApplication with minimal valid Name.""" + return ecu.add_ca( + name=j1939.Name( + arbitrary_address_capable=0, + industry_group=j1939.Name.IndustryGroup.Industrial, + vehicle_system_instance=1, + vehicle_system=1, + function=1, + function_instance=1, + ecu_instance=1, + manufacturer_code=1, + identity_number=1, + ), + device_address=device_address, + ) + + +def _j1939_threads(): + """Return list of alive threads with names starting with 'j1939.'.""" + return [ + t for t in threading.enumerate() if t.name.startswith("j1939.") and t.is_alive() + ] + + +class _FakeDependent: + """Test helper that logs stop() calls and optionally raises.""" + + def __init__(self, log, name, raise_on_stop=False): + self.log = log + self.name = name + self.raise_on_stop = raise_on_stop + self.stop_count = 0 + + def stop(self): + self.stop_count += 1 + self.log.append(self.name) + if self.raise_on_stop: + raise RuntimeError(f"{self.name} blew up") + + +def test_ecu_stop_cascades_to_memory_access(): + """ecu.stop() alone must tear down a MemoryAccess servicer thread.""" + from j1939.memory_access import MemoryAccess + + ecu = _make_ecu() + ca = _make_ca(ecu) + MemoryAccess(ca) + + # Sanity: servicer thread is running + assert any( + t.name == "j1939.memory_access servicer_thread" for t in _j1939_threads() + ) + + ecu.stop() + + assert _wait_no_threads_named("j1939.memory_access servicer_thread", timeout=1.0), ( + "MemoryAccess servicer thread still running after ecu.stop()" + ) + + +def test_ecu_stop_cascades_lifo(): + """Dependents must be stopped in reverse registration order.""" + ecu = _make_ecu() + log = [] + a = _FakeDependent(log, "A") + b = _FakeDependent(log, "B") + c = _FakeDependent(log, "C") + ecu.register_dependent(a) + ecu.register_dependent(b) + ecu.register_dependent(c) + + ecu.stop() + + assert log == ["C", "B", "A"], log + + +def test_ecu_stop_continues_on_dependent_failure(): + """A failing dependent.stop() must not prevent others from running.""" + ecu = _make_ecu() + log = [] + a = _FakeDependent(log, "A") + b = _FakeDependent(log, "B", raise_on_stop=True) + c = _FakeDependent(log, "C") + ecu.register_dependent(a) + ecu.register_dependent(b) + ecu.register_dependent(c) + + ecu.stop() # must not raise + + # All three should have had stop() called despite B raising. + assert log == ["C", "B", "A"] + # And ECU's own threads are stopped. + assert not ecu._protocol_thread.is_alive() + assert not ecu._timer_thread.is_alive() + + +def test_memory_access_explicit_stop_no_leak(): + """Explicit ma.stop() cleans up servicer thread quickly (< 50ms) before ecu.stop().""" + from j1939.memory_access import MemoryAccess + + ecu = _make_ecu() + ca = _make_ca(ecu) + ma = MemoryAccess(ca) + + t0 = time.monotonic() + ma.stop() + elapsed = time.monotonic() - t0 + + assert elapsed < 0.050, ( + f"MemoryAccess.stop() took {elapsed * 1000:.1f}ms, expected < 50ms" + ) + assert _wait_no_threads_named("j1939.memory_access servicer_thread"), ( + "Servicer thread still running after ma.stop()" + ) + + ecu.stop() + + +def test_memory_access_context_manager(): + """MemoryAccess context manager stops servicer thread on __exit__.""" + from j1939.memory_access import MemoryAccess + + ecu = _make_ecu() + ca = _make_ca(ecu) + with MemoryAccess(ca) as ma: + assert ma._job_thread.is_alive() + + assert _wait_thread_exit(ma._job_thread), ( + "Servicer thread did not stop after context exit" + ) + ecu.stop() + + +def test_memory_access_stop_idempotent(): + """Multiple calls to ma.stop() must not raise or block.""" + from j1939.memory_access import MemoryAccess + + ecu = _make_ecu() + ca = _make_ca(ecu) + ma = MemoryAccess(ca) + ma.stop() + ma.stop() + ma.stop() + ecu.stop() + + +def test_register_unregister_dependent_idempotent(): + """Duplicate register/unregister calls are silently handled.""" + ecu = _make_ecu() + log = [] + a = _FakeDependent(log, "A") + + ecu.register_dependent(a) + ecu.register_dependent(a) # duplicate - silently deduped + ecu.unregister_dependent(a) + ecu.unregister_dependent(a) # second unregister - no error + + ecu.stop() + assert log == [], "Unregistered dependent should not be stopped" + + +def test_register_dependent_requires_stop_method(): + """Registering object without stop() method raises TypeError.""" + ecu = _make_ecu() + with pytest.raises(TypeError): + ecu.register_dependent(object()) + ecu.stop() + + +def test_register_dependent_rejected_during_shutdown(): + """Registering new dependent during shutdown raises RuntimeError.""" + ecu = _make_ecu() + log = [] + blocker = _FakeDependent(log, "blocker") + late = _FakeDependent(log, "late") + captured = [] + + def blocker_stop(): + log.append("blocker") + try: + ecu.register_dependent(late) + except RuntimeError as e: + captured.append(e) + + blocker.stop = blocker_stop + ecu.register_dependent(blocker) + + ecu.stop() + + assert captured, "Expected RuntimeError when registering during shutdown" + assert log == ["blocker"] + + +def test_send_pgn_concurrent_no_crash(): + """Concurrent send_pgn calls while the protocol thread is running must not + raise RuntimeError (dictionary changed size during iteration) or corrupt + _snd_buffer. Regression test for the missing _buffer_lock in j1939_21 + send_pgn.""" + sent = [] + errors = [] + + def capture_send(can_id, extended, data, fd_format=False): + sent.append(can_id) + + ecu = j1939.ElectronicControlUnit(send_message=capture_send) + + def spam_send_pgn(): + try: + deadline = time.monotonic() + 0.5 + src = 0x01 + dst = ParameterGroupNumber.Address.GLOBAL + payload = list(range(20)) # >8 bytes → TP path + while time.monotonic() < deadline: + ecu.send_pgn(0, 0xFE, dst, 6, src, payload) + time.sleep(0.001) + except Exception as exc: + errors.append(exc) + + threads = [threading.Thread(target=spam_send_pgn) for _ in range(4)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=2.0) + assert not t.is_alive(), "send_pgn stress thread deadlocked" + + ecu.stop() + + assert not errors, f"Exceptions during concurrent send_pgn: {errors}" + + +def test_send_pgn_j1939_21_buffer_lock_no_race(): + """send_pgn check-then-write on _snd_buffer must be atomic: two threads + sending to the same src/dst pair must not both succeed and overwrite each + other's buffer entry.""" + results = [] + errors = [] + + def capture_send(can_id, extended, data, fd_format=False): + pass + + ecu = j1939.ElectronicControlUnit(send_message=capture_send) + + barrier = threading.Barrier(2) + + def send_once(): + try: + barrier.wait() # start both threads simultaneously + result = ecu.send_pgn( + 0, 0xFE, ParameterGroupNumber.Address.GLOBAL, 6, 0x01, list(range(20)) + ) + results.append(result) + except Exception as exc: + errors.append(exc) + + threads = [threading.Thread(target=send_once) for _ in range(2)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=2.0) + + ecu.stop() + + assert not errors, f"Exceptions: {errors}" + # Exactly one should succeed (True) and one should be rejected (False) + # because both target the same src/dst hash. + assert sorted(results) == [False, True], ( + f"Expected one success and one rejection, got: {results}" + ) + + +def test_dependent_registration_stress_no_leak(): + """Create/stop many MemoryAccess instances; no servicer thread may leak.""" + from j1939.memory_access import MemoryAccess + + ecu = _make_ecu() + ca = _make_ca(ecu) + + for _ in range(50): + ma = MemoryAccess(ca) + ma.stop() + + assert _wait_no_threads_named("j1939.memory_access servicer_thread", timeout=1.0), ( + "Leaked servicer thread(s) after stress test" + ) + + ecu.stop() + + +def test_dispatch_thread_exists_and_named(): + """ECU must have a live, correctly named dispatch thread after construction.""" + ecu = _make_ecu() + try: + assert hasattr(ecu, "_dispatch_thread"), "ECU has no _dispatch_thread attribute" + assert ecu._dispatch_thread.is_alive(), "dispatch thread is not alive" + assert ecu._dispatch_thread.name == "j1939.ecu dispatch_thread" + finally: + ecu.stop() + + +def test_notify_returns_immediately_while_dispatch_is_busy(): + """notify() must return well under 100 ms even when a slow subscriber + callback is running in the dispatch thread. + """ + ecu = _make_ecu() + try: + callback_entered = threading.Event() + callback_release = threading.Event() + + def slow_subscriber(priority, pgn, sa, timestamp, data): + callback_entered.set() + callback_release.wait() # block until the test releases it + + ecu.subscribe(slow_subscriber) + # PGN 0xFF00 — PDU2 broadcast, SA 0x01 + can_id = 0x18FF0001 + + # Fire the first notify so the slow callback is now running + ecu.notify(can_id, bytearray(8), 0.0) + assert callback_entered.wait(timeout=2.0), "Slow callback never started" + + # The dispatch thread is now stuck in slow_subscriber. + # notify() must still return instantly from the Notifier thread's POV. + t0 = time.perf_counter() + ecu.notify(can_id, bytearray(8), 0.0) + elapsed = time.perf_counter() - t0 + + assert elapsed < 0.1, ( + f"notify() took {elapsed * 1000:.2f} ms while dispatch was busy — " + "Notifier thread is being blocked by subscriber callbacks." + ) + finally: + callback_release.set() # unblock callback so threads can exit cleanly + ecu.stop() + + +def test_dispatch_preserves_frame_order(): + """Frames must be delivered to subscribers in the order notify() was called.""" + ecu = _make_ecu() + try: + received_sas = [] + done = threading.Event() + expected_count = 10 + + def record(priority, pgn, sa, timestamp, data): + received_sas.append(sa) + if len(received_sas) >= expected_count: + done.set() + + ecu.subscribe(record) + + # Inject frames with SA = 0..9 in order; PDU2 broadcast PGN + for sa in range(expected_count): + can_id = 0x18FF0000 | sa + ecu.notify(can_id, bytearray(8), 0.0) + + assert done.wait(timeout=2.0), "Not all frames were delivered" + assert received_sas == list(range(expected_count)), ( + f"Frame delivery order incorrect: {received_sas}" + ) + finally: + ecu.stop() + + +def test_dispatch_thread_stops_cleanly_on_ecu_stop(): + """The dispatch thread must exit within 500 ms of ecu.stop().""" + ecu = _make_ecu() + dispatch_thread = ecu._dispatch_thread + assert dispatch_thread.is_alive() + + ecu.stop() + + assert _wait_thread_exit(dispatch_thread, timeout=0.5), ( + "dispatch_thread did not exit within 500 ms of ecu.stop()" + ) + + +def test_dispatch_drains_queued_frames_before_stop(): + """Frames enqueued just before stop() must still be delivered. + + The dispatch thread drains the queue after _job_thread_end is set, so + in-flight frames are not silently dropped on shutdown. + """ + ecu = _make_ecu() + try: + received = [] + done = threading.Event() + n = 5 + + def record(priority, pgn, sa, timestamp, data): + received.append(sa) + if len(received) >= n: + done.set() + + ecu.subscribe(record) + + # Enqueue several frames then stop immediately + for sa in range(n): + ecu.notify(0x18FF0000 | sa, bytearray(8), 0.0) + finally: + ecu.stop() + + # After stop(), drain should have processed everything already enqueued + assert len(received) == n, ( + f"Expected {n} frames after stop-drain, got {len(received)}: {received}" + ) + + +def test_dispatch_queue_drop_on_full(): + """When the dispatch queue is full, notify() drops frames without blocking + or raising. The drop counter increments and the dropped flag is set. + Once the queue drains the state resets. + """ + # Use a tiny queue so it is easy to fill. + ecu = j1939.ElectronicControlUnit( + send_message=lambda *a, **kw: None, + dispatch_queue_size=5, + ) + try: + callback_entered = threading.Event() + callback_release = threading.Event() + + def blocking_subscriber(priority, pgn, sa, timestamp, data): + callback_entered.set() + callback_release.wait() + + ecu.subscribe(blocking_subscriber) + + # Trigger the first frame so the dispatch thread is blocked inside the + # slow callback, preventing the queue from draining. + can_id = 0x18FF0001 + ecu.notify(can_id, bytearray(8), 0.0) + assert callback_entered.wait(timeout=2.0), "Blocking subscriber never entered" + + # Queue can hold 5 items; one is already consumed (dispatch thread is + # inside the callback). Fill and then overflow it. + for _ in range(10): + ecu.notify(can_id, bytearray(8), 0.0) + + # Some frames must have been dropped (queue size 5, we sent 10 extra). + assert ecu._dispatch_queue_drop_count > 0, ( + "Expected dropped frames but drop_count is 0" + ) + assert ecu._dispatch_queue_dropped is True, ( + "Expected _dispatch_queue_dropped to be True while queue is full" + ) + + # Release the blocking callback so the queue drains. + callback_release.set() + + # Wait for the queue to drain fully, then send one more frame to + # trigger the "success path" of put_nowait which resets the drop state. + deadline = time.monotonic() + 2.0 + while ecu._dispatch_queue.qsize() > 0 and time.monotonic() < deadline: + time.sleep(0.01) + + # One more notify() to trigger the drain-reset path in notify(). + ecu.notify(can_id, bytearray(8), 0.0) + + # Give a moment for the reset to propagate (it happens synchronously in + # the put_nowait success branch, so it should be immediate). + deadline = time.monotonic() + 2.0 + while ecu._dispatch_queue_dropped and time.monotonic() < deadline: + time.sleep(0.01) + + assert not ecu._dispatch_queue_dropped, ( + "dispatch_queue_dropped flag was not cleared after queue drained" + ) + assert ecu._dispatch_queue_drop_count == 0, ( + "dispatch_queue_drop_count was not reset after queue drained" + ) + finally: + callback_release.set() # ensure unblocked even if test fails early + ecu.stop() diff --git a/test_helpers/conftest.py b/test_helpers/conftest.py deleted file mode 100644 index 47f9831..0000000 --- a/test_helpers/conftest.py +++ /dev/null @@ -1,11 +0,0 @@ -import pytest - -from test_helpers.feeder import Feeder - -@pytest.fixture() -def feeder(): - #setup - feeder = Feeder() - yield feeder - #teardown - feeder.stop() \ No newline at end of file