Merge branch 'canopen-python:master' into sdo-block-upload-retransmit-fix

Author: samsamfire

.github/workflows/pr-linters.yaml

@@ -0,0 +1,28 @@
+name: Run PR linters
+
+on:
+  pull_request:
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  pull-requests: read
+
+jobs:
+
+  mypy:
+    name: Run mypy static type checker (optional)
+    runs-on: ubuntu-latest
+    continue-on-error: true
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-python@v5
+      with:
+        python-version: 3.12
+        cache: pip
+        cache-dependency-path: |
+          'pyproject.toml'
+          'requirements-dev.txt'
+    - run: pip install -r requirements-dev.txt -e .
+    - name: Run mypy and report
+      run: mypy --config-file pyproject.toml .

.github/workflows/pythonpackage.yml

@@ -42,7 +42,7 @@ jobs:
     - name: Test with pytest
       run: pytest -v --cov=canopen --cov-report=xml --cov-branch
     - name: Upload coverage reports to Codecov
-      uses: codecov/codecov-action@v4
+      uses: codecov/codecov-action@v5
       with:
         token: ${{ secrets.CODECOV_TOKEN }}
 

README.rst

@@ -42,12 +42,12 @@ Install from PyPI_ using :program:`pip`::
 
 Install from latest ``master`` on GitHub::
 
-    $ pip install https://github.com/christiansandberg/canopen/archive/master.zip
+    $ pip install https://github.com/canopen-python/canopen/archive/master.zip
 
 If you want to be able to change the code while using it, clone it then install
 it in `develop mode`_::
 
-    $ git clone https://github.com/christiansandberg/canopen.git
+    $ git clone https://github.com/canopen-python/canopen.git
     $ cd canopen
     $ pip install -e .
 

canopen/network.py

@@ -75,10 +75,10 @@ def unsubscribe(self, can_id, callback=None) -> None:
             If given, remove only this callback.  Otherwise all callbacks for
             the CAN ID.
         """
-        if callback is None:
-            del self.subscribers[can_id]
-        else:
+        if callback is not None:
             self.subscribers[can_id].remove(callback)
+        if not self.subscribers[can_id] or callback is None:
+            del self.subscribers[can_id]
 
     def connect(self, *args, **kwargs) -> Network:
         """Connect to CAN bus using python-can.
@@ -392,7 +392,9 @@ class NodeScanner:
     SERVICES = (0x700, 0x580, 0x180, 0x280, 0x380, 0x480, 0x80)
 
     def __init__(self, network: Optional[Network] = None):
-        self.network = network
+        if network is None:
+            network = _UNINITIALIZED_NETWORK
+        self.network: Network = network
         #: A :class:`list` of nodes discovered
         self.nodes: List[int] = []
 
@@ -408,8 +410,6 @@ def reset(self):
 
     def search(self, limit: int = 127) -> None:
         """Search for nodes by sending SDO requests to all node IDs."""
-        if self.network is None:
-            raise RuntimeError("A Network is required to do active scanning")
         sdo_req = b"\x40\x00\x10\x00\x00\x00\x00\x00"
         for node_id in range(1, limit + 1):
             self.network.send_message(0x600 + node_id, sdo_req)

canopen/nmt.py

@@ -2,7 +2,7 @@
 import struct
 import threading
 import time
-from typing import Callable, Optional, TYPE_CHECKING
+from typing import Callable, Dict, Final, List, Optional, TYPE_CHECKING
 
 import canopen.network
 
@@ -12,7 +12,7 @@
 
 logger = logging.getLogger(__name__)
 
-NMT_STATES = {
+NMT_STATES: Final[Dict[int, str]] = {
     0: 'INITIALISING',
     4: 'STOPPED',
     5: 'OPERATIONAL',
@@ -21,7 +21,7 @@
     127: 'PRE-OPERATIONAL'
 }
 
-NMT_COMMANDS = {
+NMT_COMMANDS: Final[Dict[str, int]] = {
     'OPERATIONAL': 1,
     'STOPPED': 2,
     'SLEEP': 80,
@@ -32,7 +32,7 @@
     'RESET COMMUNICATION': 130
 }
 
-COMMAND_TO_STATE = {
+COMMAND_TO_STATE: Final[Dict[int, int]] = {
     1: 5,
     2: 4,
     80: 80,
@@ -117,7 +117,7 @@ def __init__(self, node_id: int):
         #: Timestamp of last heartbeat message
         self.timestamp: Optional[float] = None
         self.state_update = threading.Condition()
-        self._callbacks = []
+        self._callbacks: List[Callable[[int], None]] = []
 
     def on_heartbeat(self, can_id, data, timestamp):
         with self.state_update:
@@ -186,7 +186,8 @@ def start_node_guarding(self, period: float):
         :param period:
             Period (in seconds) at which the node guarding should be advertised to the slave node.
         """
-        if self._node_guarding_producer : self.stop_node_guarding()
+        if self._node_guarding_producer:
+            self.stop_node_guarding()
         self._node_guarding_producer = self.network.send_periodic(0x700 + self.id, None, period, True)
 
     def stop_node_guarding(self):

canopen/node/local.py

@@ -39,6 +39,8 @@ def __init__(
         self.emcy = EmcyProducer(0x80 + self.id)
 
     def associate_network(self, network: canopen.network.Network):
+        if self.has_network():
+            raise RuntimeError("Node is already associated with a network")
         self.network = network
         self.sdo.network = network
         self.tpdo.network = network
@@ -49,6 +51,8 @@ def associate_network(self, network: canopen.network.Network):
         network.subscribe(0, self.nmt.on_command)
 
     def remove_network(self) -> None:
+        if not self.has_network():
+            return
         self.network.unsubscribe(self.sdo.rx_cobid, self.sdo.on_request)
         self.network.unsubscribe(0, self.nmt.on_command)
         self.network = canopen.network._UNINITIALIZED_NETWORK

canopen/node/remote.py

@@ -51,6 +51,8 @@ def __init__(
             self.load_configuration()
 
     def associate_network(self, network: canopen.network.Network):
+        if self.has_network():
+            raise RuntimeError("Node is already associated with a network")
         self.network = network
         self.sdo.network = network
         self.pdo.network = network
@@ -64,6 +66,8 @@ def associate_network(self, network: canopen.network.Network):
         network.subscribe(0, self.nmt.on_command)
 
     def remove_network(self) -> None:
+        if not self.has_network():
+            return
         for sdo in self.sdo_channels:
             self.network.unsubscribe(sdo.tx_cobid, sdo.on_response)
         self.network.unsubscribe(0x700 + self.id, self.nmt.on_heartbeat)

canopen/objectdictionary/eds.py

@@ -1,12 +1,18 @@
+from __future__ import annotations
+
 import copy
 import logging
 import re
 from configparser import NoOptionError, NoSectionError, RawConfigParser
+from typing import TYPE_CHECKING
 
 from canopen import objectdictionary
 from canopen.objectdictionary import ObjectDictionary, datatypes
 from canopen.sdo import SdoClient
 
+if TYPE_CHECKING:
+    import canopen.network
+
 
 logger = logging.getLogger(__name__)
 
@@ -173,7 +179,7 @@ def import_eds(source, node_id):
     return od
 
 
-def import_from_node(node_id, network):
+def import_from_node(node_id: int, network: canopen.network.Network):
     """ Download the configuration from the remote node
     :param int node_id: Identifier of the node
     :param network: network object

canopen/sdo/client.py

@@ -353,7 +353,7 @@ def __init__(self, sdo_client, index, subindex=0, size=None, force_segment=False
         self._exp_header = None
         self._done = False
 
-        if size is None or size > 4 or force_segment:
+        if size is None or size < 1 or size > 4 or force_segment:
             # Initiate segmented download
             request = bytearray(8)
             command = REQUEST_DOWNLOAD

canopen/sync.py

@@ -1,4 +1,9 @@
-from typing import Optional
+from __future__ import annotations
+
+from typing import Optional, TYPE_CHECKING
+
+if TYPE_CHECKING:
+    import canopen.network
 
 
 class SyncProducer:
@@ -7,7 +12,7 @@ class SyncProducer:
     #: COB-ID of the SYNC message
     cob_id = 0x80
 
-    def __init__(self, network):
+    def __init__(self, network: canopen.network.Network):
         self.network = network
         self.period: Optional[float] = None
         self._task = None