Merge branch 'mypy' into typing/test

Author: acolomb

.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

@@ -5,9 +5,19 @@ name: Python package
 
 on:
   push:
-    branches: [ "master", "mypy" ]
+    branches:
+    - 'master'
+    - 'mypy'
+    paths-ignore:
+    - 'README.rst'
+    - 'LICENSE.txt'
   pull_request:
-    branches: [ "master", "mypy" ]
+    branches:
+    - 'master'
+    - 'mypy'
+    paths-ignore:
+    - 'README.rst'
+    - 'LICENSE.txt'
 
 jobs:
   build:
@@ -20,21 +30,21 @@ jobs:
         features: ['', '[db_export]']
 
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
-      uses: actions/setup-python@v3
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python-version }}
+        cache: 'pip'
+        cache-dependency-path: |
+          'pyproject.toml'
+          'requirements-dev.txt'
     - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        pip install pytest pytest-cov
-        pip install -e '.${{ matrix.features }}'
+      run: python3 -m pip install -e '.${{ matrix.features }}' -r requirements-dev.txt
     - name: Test with pytest
-      run: |
-        pytest -v --cov=canopen --cov-report=xml --cov-branch
+      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 }}
 
@@ -53,18 +63,3 @@ jobs:
       run: python3 -m pip install -r doc/requirements.txt -e .
     - name: Build docs
       run: make -C doc html
-
-  mypy:
-    continue-on-error: true
-    runs-on: ubuntu-latest
-    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 .
-    - run: mypy --config-file pyproject.toml .

README.rst

@@ -42,18 +42,18 @@ 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 .
 
 Unit tests can be run using the pytest_ framework::
 
-    $ pip install pytest
+    $ pip install -r requirements-dev.txt
     $ pytest -v
 
 You can also use :mod:`unittest` standard library module::

canopen/__init__.py

@@ -1,8 +1,14 @@
 from canopen.network import Network, NodeScanner
-from canopen.node import RemoteNode, LocalNode
-from canopen.sdo import SdoCommunicationError, SdoAbortedError
-from canopen.objectdictionary import import_od, export_od, ObjectDictionary, ObjectDictionaryError
+from canopen.node import LocalNode, RemoteNode
+from canopen.objectdictionary import (
+    ObjectDictionary,
+    ObjectDictionaryError,
+    export_od,
+    import_od,
+)
 from canopen.profiles.p402 import BaseNode402
+from canopen.sdo import SdoAbortedError, SdoCommunicationError
+
 try:
     from canopen._version import version as __version__
 except ImportError:

canopen/emcy.py

@@ -1,9 +1,12 @@
-import struct
 import logging
+import struct
 import threading
 import time
 from typing import Callable, List, Optional
 
+import canopen.network
+
+
 # Error code, error register, vendor specific data
 EMCY_STRUCT = struct.Struct("<HB5s")
 
@@ -82,7 +85,7 @@ def wait(
 class EmcyProducer:
 
     def __init__(self, cob_id: int):
-        self.network = None
+        self.network: canopen.network.Network = canopen.network._UNINITIALIZED_NETWORK
         self.cob_id = cob_id
 
     def send(self, code: int, register: int = 0, data: bytes = b""):

canopen/lss.py

@@ -1,7 +1,10 @@
 import logging
-import time
-import struct
 import queue
+import struct
+import time
+
+import canopen.network
+
 
 logger = logging.getLogger(__name__)
 
@@ -78,8 +81,8 @@ class LssMaster:
     #: Max time in seconds to wait for response from server
     RESPONSE_TIMEOUT = 0.5
 
-    def __init__(self):
-        self.network = None
+    def __init__(self) -> None:
+        self.network: canopen.network.Network = canopen.network._UNINITIALIZED_NETWORK
         self._node_id = 0
         self._data = None
         self.responses = queue.Queue()

canopen/network.py

@@ -1,21 +1,21 @@
 from __future__ import annotations
 
-from collections.abc import MutableMapping
 import logging
 import threading
-from typing import Callable, Dict, Iterator, List, Optional, Union
+from collections.abc import MutableMapping
+from typing import Callable, Dict, Final, Iterator, List, Optional, Union
 
 import can
 from can import Listener
-from can import CanError
 
-from canopen.node import RemoteNode, LocalNode
-from canopen.sync import SyncProducer
-from canopen.timestamp import TimeProducer
-from canopen.nmt import NmtMaster
 from canopen.lss import LssMaster
-from canopen.objectdictionary.eds import import_from_node
+from canopen.nmt import NmtMaster
+from canopen.node import LocalNode, RemoteNode
 from canopen.objectdictionary import ObjectDictionary
+from canopen.objectdictionary.eds import import_from_node
+from canopen.sync import SyncProducer
+from canopen.timestamp import TimeProducer
+
 
 logger = logging.getLogger(__name__)
 
@@ -25,6 +25,9 @@
 class Network(MutableMapping):
     """Representation of one CAN bus containing one or more nodes."""
 
+    NOTIFIER_CYCLE: float = 1.0  #: Maximum waiting time for one notifier iteration.
+    NOTIFIER_SHUTDOWN_TIMEOUT: float = 5.0  #: Maximum waiting time to stop notifiers.
+
     def __init__(self, bus: Optional[can.BusABC] = None):
         """
         :param can.BusABC bus:
@@ -38,7 +41,7 @@ def __init__(self, bus: Optional[can.BusABC] = None):
         #: List of :class:`can.Listener` objects.
         #: Includes at least MessageListener.
         self.listeners = [MessageListener(self)]
-        self.notifier = None
+        self.notifier: Optional[can.Notifier] = None
         self.nodes: Dict[int, Union[RemoteNode, LocalNode]] = {}
         self.subscribers: Dict[int, List[Callback]] = {}
         self.send_lock = threading.Lock()
@@ -72,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.
@@ -105,7 +108,7 @@ def connect(self, *args, **kwargs) -> Network:
         if self.bus is None:
             self.bus = can.Bus(*args, **kwargs)
         logger.info("Connected to '%s'", self.bus.channel_info)
-        self.notifier = can.Notifier(self.bus, self.listeners, 1)
+        self.notifier = can.Notifier(self.bus, self.listeners, self.NOTIFIER_CYCLE)
         return self
 
     def disconnect(self) -> None:
@@ -117,7 +120,7 @@ def disconnect(self) -> None:
             if hasattr(node, "pdo"):
                 node.pdo.stop()
         if self.notifier is not None:
-            self.notifier.stop()
+            self.notifier.stop(self.NOTIFIER_SHUTDOWN_TIMEOUT)
         if self.bus is not None:
             self.bus.shutdown()
         self.bus = None
@@ -279,6 +282,21 @@ def __len__(self) -> int:
         return len(self.nodes)
 
 
+class _UninitializedNetwork(Network):
+    """Empty network implementation as a placeholder before actual initialization."""
+
+    def __init__(self, bus: Optional[can.BusABC] = None):
+        """Do not initialize attributes, by skipping the parent constructor."""
+
+    def __getattribute__(self, name):
+        raise RuntimeError("No actual Network object was assigned, "
+                           "try associating to a real network first.")
+
+
+#: Singleton instance
+_UNINITIALIZED_NETWORK: Final[Network] = _UninitializedNetwork()
+
+
 class PeriodicMessageTask:
     """
     Task object to transmit a message periodically using python-can's
@@ -308,7 +326,6 @@ def __init__(
         self.msg = can.Message(is_extended_id=can_id > 0x7FF,
                                arbitration_id=can_id,
                                data=data, is_remote_frame=remote)
-        self._task = None
         self._start()
 
     def _start(self):
@@ -375,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] = []
 
@@ -391,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

@@ -1,13 +1,13 @@
-import threading
 import logging
 import struct
+import threading
 import time
-from collections.abc import Callable
-from typing import Dict, Final, List, Optional, TYPE_CHECKING
+from typing import Callable, Dict, Final, List, Optional, TYPE_CHECKING
 
+import canopen.network
 
 if TYPE_CHECKING:
-    from canopen import Network
+    from canopen.network import PeriodicMessageTask
 
 
 logger = logging.getLogger(__name__)
@@ -51,7 +51,7 @@ class NmtBase:
 
     def __init__(self, node_id: int):
         self.id = node_id
-        self.network: Optional[Network] = None
+        self.network: canopen.network.Network = canopen.network._UNINITIALIZED_NETWORK
         self._state = 0
 
     def on_command(self, can_id, data, timestamp):
@@ -113,7 +113,7 @@ class NmtMaster(NmtBase):
     def __init__(self, node_id: int):
         super(NmtMaster, self).__init__(node_id)
         self._state_received = None
-        self._node_guarding_producer = None
+        self._node_guarding_producer: Optional[PeriodicMessageTask] = None
         #: Timestamp of last heartbeat message
         self.timestamp: Optional[float] = None
         self.state_update = threading.Condition()
@@ -206,7 +206,7 @@ class NmtSlave(NmtBase):
 
     def __init__(self, node_id: int, local_node):
         super(NmtSlave, self).__init__(node_id)
-        self._send_task = None
+        self._send_task: Optional[PeriodicMessageTask] = None
         self._heartbeat_time_ms = 0
         self._local_node = local_node
 

canopen/node/__init__.py

@@ -1,2 +1,2 @@
-from canopen.node.remote import RemoteNode
 from canopen.node.local import LocalNode
+from canopen.node.remote import RemoteNode

canopen/node/base.py

@@ -1,4 +1,6 @@
 from typing import TextIO, Union
+
+import canopen.network
 from canopen.objectdictionary import ObjectDictionary, import_od
 
 
@@ -17,10 +19,14 @@ def __init__(
         node_id: int,
         object_dictionary: Union[ObjectDictionary, str, TextIO],
     ):
-        self.network = None
+        self.network: canopen.network.Network = canopen.network._UNINITIALIZED_NETWORK
 
         if not isinstance(object_dictionary, ObjectDictionary):
             object_dictionary = import_od(object_dictionary, node_id)
         self.object_dictionary = object_dictionary
 
         self.id = node_id or self.object_dictionary.node_id
+
+    def has_network(self) -> bool:
+        """Check whether the node has been associated to a network."""
+        return not isinstance(self.network, canopen.network._UninitializedNetwork)