Merge remote-tracking branch 'erlend-aasland/typing/ci-and-config' into mypy

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,19 +30,19 @@ 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
       with:
@@ -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

@@ -53,7 +53,7 @@ it in `develop mode`_::
 
 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/network.py

@@ -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()
@@ -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
@@ -308,7 +311,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):

canopen/nmt.py

@@ -2,7 +2,11 @@
 import logging
 import struct
 import time
-from typing import Callable, Optional
+from typing import Callable, Optional, TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from canopen.network import PeriodicMessageTask
+
 
 logger = logging.getLogger(__name__)
 
@@ -107,7 +111,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()
@@ -197,7 +201,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/sdo/base.py

@@ -105,10 +105,12 @@ def __getitem__(self, subindex: Union[int, str]) -> SdoVariable:
         return SdoVariable(self.sdo_node, self.od[subindex])
 
     def __iter__(self) -> Iterator[int]:
-        return iter(self.od)
+        # Skip the "highest subindex" entry, which is not part of the data
+        return filter(None, iter(self.od))
 
     def __len__(self) -> int:
-        return len(self.od)
+        # Skip the "highest subindex" entry, which is not part of the data
+        return len(self.od) - int(0 in self.od)
 
     def __contains__(self, subindex: Union[int, str]) -> bool:
         return subindex in self.od
@@ -127,6 +129,7 @@ def __getitem__(self, subindex: Union[int, str]) -> SdoVariable:
         return SdoVariable(self.sdo_node, self.od[subindex])
 
     def __iter__(self) -> Iterator[int]:
+        # Skip the "highest subindex" entry, which is not part of the data
         return iter(range(1, len(self) + 1))
 
     def __len__(self) -> int:

doc/conf.py

@@ -44,7 +44,6 @@
 # https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output
 
 html_theme = 'furo'
-html_static_path = ['_static']
 
 # -- Options for HTML help output --------------------------------------------
 # https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-help-output

doc/sdo.rst

@@ -189,7 +189,10 @@ API
 
     .. describe:: iter(record)
 
-       Return an iterator over the subindexes from the record.
+       Return an iterator over the subindexes from the record.  Only those with
+       a matching object dictionary entry are considered.  The "highest
+       subindex" entry is officially not part of the data and thus skipped in
+       the yielded values.
 
     .. describe:: subindex in record
 
@@ -198,7 +201,9 @@ API
 
     .. describe:: len(record)
 
-       Return the number of subindexes in the record.
+       Return the number of subindexes in the record, not counting the "highest
+       subindex" entry itself.  Only those with a matching object dictionary
+       entry are considered.
 
     .. method:: values()
 
@@ -220,25 +225,27 @@ API
     .. describe:: iter(array)
 
        Return an iterator over the subindexes from the array.
-       This will make a SDO read operation on subindex 0 in order to get the
-       actual length of the array.
+       This will make an SDO read operation on subindex 0 in order to get the
+       actual length of the array.  This "highest subindex" entry is officially
+       not part of the data and thus skipped in the yielded values.
 
     .. describe:: subindex in array
 
        Return ``True`` if the subindex (as int) or name (as string) exists in
        the array.
-       This will make a SDO read operation on subindex 0 in order to get the
+       This will make an SDO read operation on subindex 0 in order to get the
        actual length of the array.
 
     .. describe:: len(array)
 
-       Return the length of the array.
-       This will make a SDO read operation on subindex 0.
+       Return the length of the array, not counting the "highest subindex" entry
+       itself.
+       This will make an SDO read operation on subindex 0.
 
     .. method:: values()
 
        Return a list of :class:`canopen.sdo.SdoVariable` in the array.
-       This will make a SDO read operation on subindex 0 in order to get the
+       This will make an SDO read operation on subindex 0 in order to get the
        actual length of the array.
 
 

requirements-dev.txt

@@ -1 +1,3 @@
 mypy~=1.10
+pytest~=8.3
+pytest-cov~=5.0

test/sample.eds

@@ -100,12 +100,7 @@ DataType=0x0007
 AccessType=ro
 PDOMapping=0
 
-[1018sub3]
-ParameterName=Revision number
-ObjectType=0x7
-DataType=0x0007
-AccessType=ro
-PDOMapping=0
+; [1018sub3] left out for testing
 
 [1018sub4]
 ParameterName=Serial number
@@ -123,11 +118,62 @@ SupportedObjects=3
 [1003]
 ParameterName=Pre-defined error field
 ObjectType=0x8
-CompactSubObj=255
+SubNumber=9
+
+[1003sub0]
+ParameterName=Number of errors
+ObjectType=0x7
+DataType=0x0005
+AccessType=rw
+DefaultValue=3
+PDOMapping=0
+
+[1003sub1]
+ParameterName=Pre-defined error field_1
+ObjectType=0x7
+DataType=0x0007
+AccessType=ro
+DefaultValue=0
+PDOMapping=0
+
+; [1003sub2] left out for testing
+
+[1003sub3]
+ParameterName=Pre-defined error field_3
+ObjectType=0x7
+DataType=0x0007
+AccessType=ro
+DefaultValue=0
+PDOMapping=0
+
+[1003sub4]
+ParameterName=Pre-defined error field_4
+ObjectType=0x7
+DataType=0x0007
+AccessType=ro
+DefaultValue=0
+PDOMapping=0
+
+[1003sub5]
+ParameterName=Pre-defined error field_5
+ObjectType=0x7
+DataType=0x0007
+AccessType=ro
+DefaultValue=0
+PDOMapping=0
+
+; [1003sub6] left out for testing
+
+[1003sub7]
+ParameterName=Pre-defined error field_7
+ObjectType=0x7
 DataType=0x0007
 AccessType=ro
+DefaultValue=0
 PDOMapping=0
 
+; [1003sub8] left out for testing
+
 [1008]
 ParameterName=Manufacturer device name
 ObjectType=0x7