Cleanup f-strings, logging and formats (#443)

* Change old-style % formatting to f-strings
* Change str.format() to f-strings
* Harmonize logging function to use % formatting and arguments.
* Consistently print OD index and subindex in 0x01AB:0C notation.
* Introduce helper function pretty_index() for the latter usage.

Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: André Colomb <[email protected]>

Author: 3 people

canopen/emcy.py

@@ -130,7 +130,7 @@ def get_desc(self) -> str:
         return ""
 
     def __str__(self):
-        text = "Code 0x{:04X}".format(self.code)
+        text = f"Code 0x{self.code:04X}"
         description = self.get_desc()
         if description:
             text = text + ", " + description

canopen/lss.py

@@ -353,7 +353,7 @@ def __send_configure(self, req_cs, value1=0, value2=0):
             raise LssError("Response message is not for the request")
 
         if error_code != ERROR_NONE:
-            error_msg = "LSS Error: %d" % error_code
+            error_msg = f"LSS Error: {error_code}"
             raise LssError(error_msg)
 
     def __send_command(self, message):
@@ -368,9 +368,7 @@ def __send_command(self, message):
         :rtype: bytes
         """
 
-        message_str = " ".join(["{:02x}".format(x) for x in message])
-        logger.info(
-            "Sending LSS message {}".format(message_str))
+        logger.info("Sending LSS message %s", message.hex(" ").upper())
 
         response = None
         if not self.responses.empty():

canopen/node/local.py

@@ -86,7 +86,7 @@ def get_data(
                 return obj.encode_raw(obj.default)
 
         # Resource not available
-        logger.info("Resource unavailable for 0x%X:%d", index, subindex)
+        logger.info("Resource unavailable for 0x%04X:%02X", index, subindex)
         raise SdoAbortedError(0x060A0023)
 
     def set_data(

canopen/node/remote.py

@@ -122,18 +122,13 @@ def __load_configuration_helper(self, index, subindex, name, value):
         """
         try:
             if subindex is not None:
-                logger.info(str('SDO [{index:#06x}][{subindex:#06x}]: {name}: {value:#06x}'.format(
-                    index=index,
-                    subindex=subindex,
-                    name=name,
-                    value=value)))
+                logger.info('SDO [0x%04X][0x%02X]: %s: %#06x',
+                            index, subindex, name, value)
                 self.sdo[index][subindex].raw = value
             else:
                 self.sdo[index].raw = value
-                logger.info(str('SDO [{index:#06x}]: {name}: {value:#06x}'.format(
-                    index=index,
-                    name=name,
-                    value=value)))
+                logger.info('SDO [0x%04X]: %s: %#06x',
+                            index, name, value)
         except SdoCommunicationError as e:
             logger.warning(str(e))
         except SdoAbortedError as e:
@@ -142,7 +137,8 @@ def __load_configuration_helper(self, index, subindex, name, value):
             if e.code != 0x06010002:
                 # Abort codes other than "Attempt to write a read-only object"
                 # should still be reported.
-                logger.warning('[ERROR SETTING object {0:#06x}:{1:#06x}]  {2}'.format(index, subindex, str(e)))
+                logger.warning('[ERROR SETTING object 0x%04X:%02X] %s',
+                               index, subindex, e)
                 raise
 
     def load_configuration(self):

canopen/objectdictionary/__init__.py

@@ -8,6 +8,7 @@
 
 from canopen.objectdictionary.datatypes import *
 from canopen.objectdictionary.datatypes_24bit import Integer24, Unsigned24
+from canopen.utils import pretty_index
 
 logger = logging.getLogger(__name__)
 
@@ -108,8 +109,7 @@ def __getitem__(
             if isinstance(index, str) and '.' in index:
                 idx, sub = index.split('.', maxsplit=1)
                 return self[idx][sub]
-            name = "0x%X" % index if isinstance(index, int) else index
-            raise KeyError("%s was not found in Object Dictionary" % name)
+            raise KeyError(f"{pretty_index(index)} was not found in Object Dictionary")
         return item
 
     def __setitem__(
@@ -180,12 +180,12 @@ 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 {pretty_index(self.index)}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
         item = self.names.get(subindex) or self.subindices.get(subindex)
         if item is None:
-            raise KeyError("Subindex %s was not found" % subindex)
+            raise KeyError(f"Subindex {pretty_index(None, subindex)} was not found")
         return item
 
     def __setitem__(self, subindex: Union[int, str], var: "ODVariable"):
@@ -239,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 {pretty_index(self.index)}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
         var = self.names.get(subindex) or self.subindices.get(subindex)
@@ -249,7 +249,7 @@ def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
         elif isinstance(subindex, int) and 0 < subindex < 256:
             # Create a new variable based on first array item
             template = self.subindices[1]
-            name = "%s_%x" % (template.name, subindex)
+            name = f"{template.name}_{subindex:x}"
             var = ODVariable(name, self.index, subindex)
             var.parent = self
             for attr in ("data_type", "unit", "factor", "min", "max", "default",
@@ -258,7 +258,7 @@ def __getitem__(self, subindex: Union[int, str]) -> "ODVariable":
                 if attr in template.__dict__:
                     var.__dict__[attr] = template.__dict__[attr]
         else:
-            raise KeyError("Could not find subindex %r" % subindex)
+            raise KeyError(f"Could not find subindex {pretty_index(None, subindex)}")
         return var
 
     def __len__(self) -> int:
@@ -337,8 +337,8 @@ 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}>"
+        subindex = self.subindex if isinstance(self.parent, (ODRecord, ODArray)) else None
+        return f"<{type(self).__qualname__} {self.qualname!r} at {pretty_index(self.index, subindex)}>"
 
     @property
     def qualname(self) -> str:
@@ -417,8 +417,7 @@ def encode_raw(self, value: Union[int, float, str, bytes, bytearray]) -> bytes:
                 if self.max is not None and value > self.max:
                     logger.warning(
                         "Value %d is greater than max value %d",
-                        value,
-                        self.max)
+                        value, self.max)
             try:
                 return self.STRUCT_TYPES[self.data_type].pack(value)
             except struct.error:
@@ -427,8 +426,7 @@ def encode_raw(self, value: Union[int, float, str, bytes, bytearray]) -> bytes:
             raise ObjectDictionaryError("Data type has not been specified")
         else:
             raise TypeError(
-                "Do not know how to encode %r to data type %Xh" % (
-                    value, self.data_type))
+                f"Do not know how to encode {value!r} to data type 0x{self.data_type:X}")
 
     def decode_phys(self, value: int) -> Union[int, bool, float, str, bytes]:
         if self.data_type in INTEGER_TYPES:
@@ -446,7 +444,7 @@ def decode_desc(self, value: int) -> str:
             raise ObjectDictionaryError("No value descriptions exist")
         elif value not in self.value_descriptions:
             raise ObjectDictionaryError(
-                "No value description exists for %d" % value)
+                f"No value description exists for {value}")
         else:
             return self.value_descriptions[value]
 
@@ -458,8 +456,8 @@ def encode_desc(self, desc: str) -> int:
                 if description == desc:
                     return value
         valid_values = ", ".join(self.value_descriptions.values())
-        error_text = "No value corresponds to '%s'. Valid values are: %s"
-        raise ValueError(error_text % (desc, valid_values))
+        raise ValueError(
+            f"No value corresponds to '{desc}'. Valid values are: {valid_values}")
 
     def decode_bits(self, value: int, bits: List[int]) -> int:
         try:

canopen/objectdictionary/eds.py

@@ -43,7 +43,7 @@ def import_eds(source, node_id):
     if eds.has_section("Comments"):
         linecount = int(eds.get("Comments", "Lines"), 0)
         od.comments = '\n'.join([
-            eds.get("Comments", "Line%i" % line)
+            eds.get("Comments", f"Line{line}")
             for line in range(1, linecount+1)
         ])
 
@@ -52,7 +52,7 @@ def import_eds(source, node_id):
     else:
         for rate in [10, 20, 50, 125, 250, 500, 800, 1000]:
             baudPossible = int(
-                eds.get("DeviceInfo", "BaudRate_%i" % rate, fallback='0'), 0)
+                eds.get("DeviceInfo", f"BaudRate_{rate}", fallback='0'), 0)
             if baudPossible != 0:
                 od.device_information.allowed_baudrates.add(rate*1000)
 
@@ -93,7 +93,7 @@ def import_eds(source, node_id):
         match = re.match(r"^[Dd]ummy[Uu]sage$", section)
         if match is not None:
             for i in range(1, 8):
-                key = "Dummy%04d" % i
+                key = f"Dummy{i:04d}"
                 if eds.getint(section, key) == 1:
                     var = objectdictionary.ODVariable(key, i, 0)
                     var.data_type = i
@@ -239,7 +239,7 @@ def _revert_variable(var_type, value):
     elif var_type in datatypes.FLOAT_TYPES:
         return value
     else:
-        return "0x%02X" % value
+        return f"0x{value:02X}"
 
 
 def build_variable(eds, section, node_id, index, subindex=0):
@@ -264,9 +264,9 @@ def build_variable(eds, section, node_id, index, subindex=0):
         # The eds.get function gives us 0x00A0 now convert to String without hex representation and upper case
         # The sub2 part is then the section where the type parameter stands
         try:
-            var.data_type = int(eds.get("%Xsub1" % var.data_type, "DefaultValue"), 0)
+            var.data_type = int(eds.get(f"{var.data_type:X}sub1", "DefaultValue"), 0)
         except NoSectionError:
-            logger.warning("%s has an unknown or unsupported data type (%X)", name, var.data_type)
+            logger.warning("%s has an unknown or unsupported data type (0x%X)", name, var.data_type)
             # Assume DOMAIN to force application to interpret the byte data
             var.data_type = datatypes.DOMAIN
 
@@ -354,15 +354,15 @@ def export_common(var, eds, section):
     def export_variable(var, eds):
         if isinstance(var.parent, ObjectDictionary):
             # top level variable
-            section = "%04X" % var.index
+            section = f"{var.index:04X}"
         else:
             # nested variable
-            section = "%04Xsub%X" % (var.index, var.subindex)
+            section = f"{var.index:04X}sub{var.subindex:X}"
 
         export_common(var, eds, section)
-        eds.set(section, "ObjectType", "0x%X" % VAR)
+        eds.set(section, "ObjectType", f"0x{VAR:X}")
         if var.data_type:
-            eds.set(section, "DataType", "0x%04X" % var.data_type)
+            eds.set(section, "DataType", f"0x{var.data_type:04X}")
         if var.access_type:
             eds.set(section, "AccessType", var.access_type)
 
@@ -379,7 +379,7 @@ def export_variable(var, eds):
                 eds.set(section, "ParameterValue",
                         _revert_variable(var.data_type, var.value))
 
-        eds.set(section, "DataType", "0x%04X" % var.data_type)
+        eds.set(section, "DataType", f"0x{var.data_type:04X}")
         eds.set(section, "PDOMapping", hex(var.pdo_mappable))
 
         if getattr(var, 'min', None) is not None:
@@ -395,11 +395,11 @@ def export_variable(var, eds):
             eds.set(section, "Unit", var.unit)
 
     def export_record(var, eds):
-        section = "%04X" % var.index
+        section = f"{var.index:04X}"
         export_common(var, eds, section)
-        eds.set(section, "SubNumber", "0x%X" % len(var.subindices))
+        eds.set(section, "SubNumber", f"0x{len(var.subindices):X}")
         ot = RECORD if isinstance(var, objectdictionary.ODRecord) else ARR
-        eds.set(section, "ObjectType", "0x%X" % ot)
+        eds.set(section, "ObjectType", f"0x{ot:X}")
         for i in var:
             export_variable(var[i], eds)
 
@@ -461,7 +461,7 @@ def export_record(var, eds):
     for rate in od.device_information.allowed_baudrates.union(
             {10e3, 20e3, 50e3, 125e3, 250e3, 500e3, 800e3, 1000e3}):
         eds.set(
-            "DeviceInfo", "BaudRate_%i" % (rate/1000),
+            "DeviceInfo", f"BaudRate_{rate//1000}",
             int(rate in od.device_information.allowed_baudrates))
 
     if device_commisioning and (od.bitrate or od.node_id):
@@ -475,12 +475,12 @@ def export_record(var, eds):
     i = 0
     for line in od.comments.splitlines():
         i += 1
-        eds.set("Comments", "Line%i" % i, line)
+        eds.set("Comments", f"Line{i}", line)
     eds.set("Comments", "Lines", i)
 
     eds.add_section("DummyUsage")
     for i in range(1, 8):
-        key = "Dummy%04d" % i
+        key = f"Dummy{i:04d}"
         eds.set("DummyUsage", key, 1 if (key in od) else 0)
 
     def mandatory_indices(x):
@@ -504,7 +504,7 @@ def add_list(section, list):
         eds.add_section(section)
         eds.set(section, "SupportedObjects", len(list))
         for i in range(0, len(list)):
-            eds.set(section, (i + 1), "0x%04X" % list[i])
+            eds.set(section, (i + 1), f"0x{list[i]:04X}")
         for index in list:
             export_object(od[index], eds)
 

canopen/pdo/__init__.py

@@ -39,7 +39,7 @@ class RPDO(PdoBase):
     def __init__(self, node):
         super(RPDO, self).__init__(node)
         self.map = PdoMaps(0x1400, 0x1600, self, 0x200)
-        logger.debug('RPDO Map as {0}'.format(len(self.map)))
+        logger.debug('RPDO Map as %d', len(self.map))
 
     def stop(self):
         """Stop transmission of all RPDOs.
@@ -64,7 +64,7 @@ class TPDO(PdoBase):
     def __init__(self, node):
         super(TPDO, self).__init__(node)
         self.map = PdoMaps(0x1800, 0x1A00, self, 0x180)
-        logger.debug('TPDO Map as {0}'.format(len(self.map)))
+        logger.debug('TPDO Map as %d', len(self.map))
 
     def stop(self):
         """Stop transmission of all TPDOs.

canopen/pdo/base.py

@@ -42,7 +42,7 @@ def __getitem__(self, key):
                 except KeyError:
                     # ignore if one specific PDO does not have the key and try the next one
                     continue
-        raise KeyError("PDO: {0} was not found in any map".format(key))
+        raise KeyError(f"PDO: {key} was not found in any map")
 
     def __len__(self):
         return len(self.map)
@@ -201,8 +201,8 @@ def __getitem_by_index(self, value):
                 valid_values.append(var.index)
                 if var.index == value:
                     return var
-        raise KeyError('{0} not found in map. Valid entries are {1}'.format(
-            value, ', '.join(str(v) for v in valid_values)))
+        raise KeyError(f"{value} not found in map. Valid entries are "
+                       f"{', '.join(str(v) for v in valid_values)}")
 
     def __getitem_by_name(self, value):
         valid_values = []
@@ -211,8 +211,8 @@ def __getitem_by_name(self, value):
                 valid_values.append(var.name)
                 if var.name == value:
                     return var
-        raise KeyError('{0} not found in map. Valid entries are {1}'.format(
-            value, ', '.join(valid_values)))
+        raise KeyError(f"{value} not found in map. Valid entries are "
+                       f"{', '.join(valid_values)}")
 
     def __getitem__(self, key: Union[int, str]) -> "PdoVariable":
         var = None
@@ -272,7 +272,7 @@ def name(self) -> str:
         if direction == "Rx":
             map_id -= 1
         node_id = self.cob_id & 0x7F
-        return "%sPDO%d_node%d" % (direction, map_id, node_id)
+        return f"{direction}PDO{map_id}_node{node_id}"
 
     @property
     def is_periodic(self) -> bool:
@@ -398,7 +398,7 @@ def save(self) -> None:
                 self._fill_map(self.map_array[0].raw)
             subindex = 1
             for var in self.map:
-                logger.info("Writing %s (0x%X:%d, %d bits) to PDO map",
+                logger.info("Writing %s (0x%04X:%02X, %d bits) to PDO map",
                             var.name, var.index, var.subindex, var.length)
                 if hasattr(self.pdo_node.node, "curtis_hack") and self.pdo_node.node.curtis_hack:  # Curtis HACK: mixed up field order
                     self.map_array[subindex].raw = (var.index |
@@ -467,7 +467,7 @@ def add_variable(
             # We want to see the bit fields within the PDO
             start_bit = var.offset
             end_bit = start_bit + var.length - 1
-            logger.info("Adding %s (0x%X:%d) at bits %d - %d to PDO map",
+            logger.info("Adding %s (0x%04X:%02X) at bits %d - %d to PDO map",
                         var.name, var.index, var.subindex, start_bit, end_bit)
             self.map.append(var)
             self.length += var.length