Minor housekeeping updates

* Manual merge in improments from master
* Remove opinionated set_*() and get_*() for non-async calls
* Comment updates

Author: sveinse

canopen/emcy.py

@@ -33,7 +33,7 @@ def on_emcy(self, can_id, data, timestamp):
         code, register, data = EMCY_STRUCT.unpack(data)
         entry = EmcyError(code, register, data, timestamp)
 
-        # NOTE: Blocking call
+        # NOTE: Blocking lock
         with self.emcy_received:
             if code & 0xFF00 == 0:
                 # Error reset
@@ -95,7 +95,7 @@ def wait(
         """
         end_time = time.time() + timeout
         while True:
-            # NOTE: Blocking call
+            # NOTE: Blocking lock
             with self.emcy_received:
                 prev_log_size = len(self.log)
                 # NOTE: Blocking call

canopen/lss.py

@@ -3,10 +3,7 @@
 import time
 import struct
 import asyncio
-try:
-    import queue
-except ImportError:
-    import Queue as queue
+import queue
 
 from canopen.async_guard import ensure_not_async
 
@@ -376,6 +373,7 @@ def __send_configure(self, req_cs, value1=0, value2=0):
             raise LssError(error_msg)
 
     # FIXME: Make async implementation "__asend_command"
+
     @ensure_not_async  # NOTE: Safeguard for accidental async use
     def __send_command(self, message):
         """Send a LSS operation code to the network

canopen/network.py

@@ -1,8 +1,5 @@
 from __future__ import annotations
-try:
-    from collections.abc import MutableMapping
-except ImportError:
-    from collections import MutableMapping
+from collections.abc import MutableMapping
 import logging
 import threading
 from typing import Callable, Dict, Iterable, List, Optional, Union, TYPE_CHECKING
@@ -42,8 +39,8 @@ class Network(MutableMapping):
 
     def __init__(
         self,
-        bus: can.BusABC | None = None,
-        loop: AbstractEventLoop | None = None
+        bus: Optional[BusABC] = None,
+        loop: Optional[AbstractEventLoop] = None
     ):
         """
         :param can.BusABC bus:

canopen/nmt.py

@@ -101,10 +101,6 @@ def state(self) -> str:
 
     @state.setter
     def state(self, new_state: str):
-        logger.warning("Accessing NmtBase.state setter is deprecated, use set_state()")
-        self.set_state(new_state)
-
-    def set_state(self, new_state: str):
         if new_state in NMT_COMMANDS:
             code = NMT_COMMANDS[new_state]
         else:
@@ -129,7 +125,7 @@ def __init__(self, node_id: int):
     @ensure_not_async  # NOTE: Safeguard for accidental async use
     def on_heartbeat(self, can_id, data, timestamp):
         # NOTE: Callback. Called from another thread unless async
-        # NOTE: Blocking call
+        # NOTE: Blocking lock
         with self.state_update:
             self.timestamp = timestamp
             new_state, = struct.unpack_from("B", data)
@@ -180,7 +176,7 @@ def send_command(self, code: int):
     @ensure_not_async  # NOTE: Safeguard for accidental async use
     def wait_for_heartbeat(self, timeout: float = 10):
         """Wait until a heartbeat message is received."""
-        # NOTE: Blocking call
+        # NOTE: Blocking lock
         with self.state_update:
             self._state_received = None
             # NOTE: Blocking call
@@ -205,7 +201,7 @@ def wait_for_bootup(self, timeout: float = 10) -> None:
         end_time = time.time() + timeout
         while True:
             now = time.time()
-            # NOTE: Blocking call
+            # NOTE: Blocking lock
             with self.state_update:
                 self._state_received = None
                 # NOTE: Blocking call
@@ -286,7 +282,7 @@ def send_command(self, code: int) -> None:
         # between INITIALIZING and PRE-OPERATIONAL state
         if old_state == 0 and self._state == 127:
             # NOTE: Blocking - OK. Protected in SdoClient
-            heartbeat_time_ms = self._local_node.sdo[0x1017].get_raw()
+            heartbeat_time_ms = self._local_node.sdo[0x1017].raw
             self.start_heartbeat(heartbeat_time_ms)
         else:
             self.update_heartbeat()

canopen/node/base.py

@@ -1,4 +1,4 @@
-from typing import TextIO, Union, TYPE_CHECKING
+from typing import TextIO, Union, Optional, TYPE_CHECKING
 
 from canopen.objectdictionary import ObjectDictionary, import_od
 
@@ -21,7 +21,7 @@ def __init__(
         node_id: int,
         object_dictionary: Union[ObjectDictionary, str, TextIO],
     ):
-        self.network: Network | None = None
+        self.network: Optional[Network] = None
 
         if not isinstance(object_dictionary, ObjectDictionary):
             object_dictionary = import_od(object_dictionary, node_id)

canopen/node/remote.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 import logging
-from typing import Union, TextIO, TYPE_CHECKING
+from typing import Union, TextIO, List, TYPE_CHECKING
 
 from canopen.sdo import SdoClient, SdoCommunicationError, SdoAbortedError
 from canopen.nmt import NmtMaster
@@ -39,7 +39,7 @@ def __init__(
         #: Enable WORKAROUND for reversed PDO mapping entries
         self.curtis_hack = False
 
-        self.sdo_channels: list[SdoClient] = []
+        self.sdo_channels: List[SdoClient] = []
         self.sdo = self.add_sdo(0x600 + self.id, 0x580 + self.id)
         self.tpdo = TPDO(self)
         self.rpdo = RPDO(self)
@@ -147,11 +147,11 @@ def __load_configuration_helper(self, index, subindex, name, value):
                     subindex=subindex,
                     name=name,
                     value=value)))
-                # NOTE: Blocking - OK. Protected in SdoClient
-                self.sdo[index][subindex].set_raw(value)
+                # NOTE: Blocking call - OK. Protected in SdoClient
+                self.sdo[index][subindex].raw = value
             else:
-                # FIXME: Blocking - OK. Protected in SdoClient
-                self.sdo[index].set_raw(value)
+                # NOTE: Blocking call - OK. Protected in SdoClient
+                self.sdo[index].raw = value
                 logger.info(str('SDO [{index:#06x}]: {name}: {value:#06x}'.format(
                     index=index,
                     name=name,

canopen/objectdictionary/__init__.py

@@ -3,10 +3,7 @@
 """
 import struct
 from typing import Dict, Iterable, List, Optional, TextIO, Union
-try:
-    from collections.abc import MutableMapping, Mapping
-except ImportError:
-    from collections import MutableMapping, Mapping
+from collections.abc import MutableMapping, Mapping
 import logging
 
 from canopen.objectdictionary.datatypes import *
@@ -183,7 +180,7 @@ def __init__(self, name: str, index: int):
         self.names = {}
 
     def __repr__(self) -> str:
-        return f"<{type(self).__qualname__} {self.name!r} at 0x{self.index:04x}>"
+        return f"<{type(self).__qualname__} {self.name!r} at 0x{self.index:04X}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
         item = self.names.get(subindex) or self.subindices.get(subindex)
@@ -242,7 +239,7 @@ def __init__(self, name: str, index: int):
         self.names = {}
 
     def __repr__(self) -> str:
-        return f"<{type(self).__qualname__} {self.name!r} at 0x{self.index:04x}>"
+        return f"<{type(self).__qualname__} {self.name!r} at 0x{self.index:04X}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
         var = self.names.get(subindex) or self.subindices.get(subindex)
@@ -340,11 +337,13 @@ def __init__(self, name: str, index: int, subindex: int = 0):
         self.pdo_mappable = False
 
     def __repr__(self) -> str:
-        suffix = f":{self.subindex:02x}" if isinstance(self.parent, (ODRecord, ODArray)) else ""
-        return f"<{type(self).__qualname__} {self.qualname!r} at 0x{self.index:04x}{suffix}>"
+        suffix = f":{self.subindex:02X}" if isinstance(self.parent, (ODRecord, ODArray)) else ""
+        return f"<{type(self).__qualname__} {self.qualname!r} at 0x{self.index:04X}{suffix}>"
 
     @property
     def qualname(self) -> str:
+        """Fully qualified name of the variable. If the variable is a subindex
+        of a record or array, the name will be prefixed with the parent's name."""
         if isinstance(self.parent, (ODRecord, ODArray)):
             return f"{self.parent.name}.{self.name}"
         return self.name

canopen/objectdictionary/eds.py

@@ -1,11 +1,7 @@
 import copy
 import logging
 import re
-
-try:
-    from configparser import RawConfigParser, NoOptionError, NoSectionError
-except ImportError:
-    from ConfigParser import RawConfigParser, NoOptionError, NoSectionError
+from configparser import RawConfigParser, NoOptionError, NoSectionError
 
 from canopen import objectdictionary
 from canopen.objectdictionary import ObjectDictionary, datatypes

canopen/objectdictionary/epf.py

@@ -1,7 +1,4 @@
-try:
-    import xml.etree.cElementTree as etree
-except ImportError:
-    import xml.etree.ElementTree as etree
+import xml.etree.ElementTree as etree
 import logging
 
 from canopen import objectdictionary

canopen/pdo/base.py

@@ -2,10 +2,7 @@
 import threading
 import math
 from typing import Callable, Dict, Iterable, List, Optional, Union, TYPE_CHECKING
-try:
-    from collections.abc import Mapping
-except ImportError:
-    from collections import Mapping
+from collections.abc import Mapping
 import logging
 import binascii
 import asyncio
@@ -140,7 +137,7 @@ def stop(self):
             pdo_map.stop()
 
 
-class PdoMaps(Mapping[int, "PdoMap"]):
+class PdoMaps(Mapping):
     """A collection of transmit or receive maps."""
 
     def __init__(self, com_offset, map_offset, pdo_node: PdoBase, cob_base=None):
@@ -212,7 +209,7 @@ def __init__(self, pdo_node: PdoBase, com_record, map_array):
         self._task = None
 
     def __repr__(self) -> str:
-        return f"<{type(self).__qualname__} {self.name!r} at COB-ID 0x{self.cob_id}>"
+        return f"<{type(self).__qualname__} {self.name!r} at COB-ID 0x{self.cob_id:X}>"
 
     def __getitem_by_index(self, value):
         valid_values = []
@@ -316,7 +313,7 @@ def on_message(self, can_id, data, timestamp):
         # NOTE: Callback. Called from another thread unless async
         is_transmitting = self._task is not None
         if can_id == self.cob_id and not is_transmitting:
-            # NOTE: Blocking call
+            # NOTE: Blocking lock
             with self.receive_condition:
                 self.is_received = True
                 self.data = data
@@ -413,8 +410,8 @@ def read(self, from_od=False) -> None:
                 value = var.od.default
             else:
                 # Get value from SDO
-                # NOTE: Blocking
-                value = var.get_raw()
+                # NOTE: Blocking call
+                value = var.raw
             try:
                 # Deliver value into read_generator and wait for next object
                 var = gen.send(value)
@@ -507,10 +504,10 @@ def save(self) -> None:
         for sdo, value in self.save_generator():
             if value == '@@get':
                 # NOTE: Sync implementation of the WORKAROUND in save_generator()
-                # NOTE: Blocking
-                self._fill_map(sdo.get_raw())
+                # NOTE: Blocking call
+                self._fill_map(sdo.raw)
             else:
-                # NOTE: Blocking
+                # NOTE: Blocking call
                 sdo.set_raw(value)
 
     async def asave(self) -> None:
@@ -630,7 +627,7 @@ def wait_for_reception(self, timeout: float = 10) -> float:
         :param float timeout: Max time to wait in seconds.
         :return: Timestamp of message received or None if timeout.
         """
-        # NOTE: Blocking call
+        # NOTE: Blocking lock
         with self.receive_condition:
             self.is_received = False
             # NOTE: Blocking call
@@ -692,7 +689,7 @@ def get_data(self) -> bytes:
         return data
 
     async def aget_data(self) -> bytes:
-        # As long as get_data() is not making any IO, it can be called
+        # Since get_data() is not making any IO, it can be called
         # directly with no special async variant
         return self.get_data()
 
@@ -730,7 +727,7 @@ def set_data(self, data: bytes):
         self.pdo_parent.update()
 
     async def aset_data(self, data: bytes):
-        # As long as get_data() is not making any IO, it can be called
+        # Since get_data() is not making any IO, it can be called
         # directly with no special async variant
         return self.set_data(data)