Add pretty_index for printing index and subindex

* Change print statements to use pretty_index()
* Updates from code review

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 = f"0x{index:04X}" if isinstance(index, int) else f"{index!r}"
-            raise KeyError(f"{name} was not found in Object Dictionary")
+            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(f"Subindex {subindex!r} was not found")
+            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)
@@ -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(f"Could not find subindex {subindex!r}")
+            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:
@@ -426,7 +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(
-                f"Do not know how to encode {value!r} to data type {self.data_type:X}h")
+                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:

canopen/pdo/base.py

@@ -201,8 +201,8 @@ def __getitem_by_index(self, value):
                 valid_values.append(var.index)
                 if var.index == value:
                     return var
-        raise KeyError(f'{value} not found in map. Valid entries are ' +
-            ", ".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(f'{value} not found in map. Valid entries are ' +
-            ', '.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

canopen/sdo/base.py

@@ -5,6 +5,7 @@
 from canopen import objectdictionary
 from canopen.objectdictionary import ObjectDictionary
 from canopen import variable
+from canopen.utils import pretty_index
 
 
 class CrcXmodem:
@@ -97,7 +98,7 @@ def __init__(self, sdo_node: SdoBase, od: ObjectDictionary):
         self.od = od
 
     def __repr__(self) -> str:
-        return f"<{type(self).__qualname__} {self.od.name!r} at 0x{self.od.index:04X}>"
+        return f"<{type(self).__qualname__} {self.od.name!r} at {pretty_index(self.od.index)}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "SdoVariable":
         return SdoVariable(self.sdo_node, self.od[subindex])
@@ -119,7 +120,7 @@ def __init__(self, sdo_node: SdoBase, od: ObjectDictionary):
         self.od = od
 
     def __repr__(self) -> str:
-        return f"<{type(self).__qualname__} {self.od.name!r} at 0x{self.od.index:04X}>"
+        return f"<{type(self).__qualname__} {self.od.name!r} at {pretty_index(self.od.index)}>"
 
     def __getitem__(self, subindex: Union[int, str]) -> "SdoVariable":
         return SdoVariable(self.sdo_node, self.od[subindex])

canopen/sdo/client.py

@@ -7,6 +7,7 @@
 from canopen.network import CanError
 from canopen import objectdictionary
 from canopen.sdo.base import SdoBase
+from canopen.utils import pretty_index
 from canopen.sdo.constants import *
 from canopen.sdo.exceptions import *
 
@@ -258,7 +259,7 @@ def __init__(self, sdo_client, index, subindex=0):
         # Check that the message is for us
         if res_index != index or res_subindex != subindex:
             raise SdoCommunicationError(
-                f"Node returned a value for 0x{res_index:04X}:{res_subindex:02X} instead, "
+                f"Node returned a value for {pretty_index(res_index, res_subindex)} instead, "
                 "maybe there is another SDO client communicating "
                 "on the same SDO channel?")
 
@@ -492,7 +493,7 @@ def __init__(self, sdo_client, index, subindex=0, request_crc_support=True):
         if res_index != index or res_subindex != subindex:
             self._error = True
             raise SdoCommunicationError(
-                f"Node returned a value for 0x{res_index:04X}:{res_subindex:02X} instead, "
+                f"Node returned a value for {pretty_index(res_index, res_subindex)} instead, "
                 "maybe there is another SDO client communicating "
                 "on the same SDO channel?")
         if res_command & BLOCK_SIZE_SPECIFIED:
@@ -661,7 +662,7 @@ def __init__(self, sdo_client, index, subindex=0, size=None, request_crc_support
         if res_index != index or res_subindex != subindex:
             self.sdo_client.abort()
             raise SdoCommunicationError(
-                f"Node returned a value for 0x{res_index:04X}:{res_subindex:02X} instead, "
+                f"Node returned a value for {pretty_index(res_index, res_subindex)} instead, "
                 "maybe there is another SDO client communicating "
                 "on the same SDO channel?")
         self._blksize, = struct.unpack_from("B", response, 4)

canopen/utils.py

@@ -0,0 +1,22 @@
+"""Additional utility functions for canopen."""
+from typing import Optional, Union
+
+
+def pretty_index(index: Optional[Union[int, str]],
+                 sub: Optional[Union[int, str]] = None):
+    """Format an index and subindex as a string."""
+
+    index_str = ""
+    if isinstance(index, int):
+        index_str = f"0x{index:04X}"
+    elif index:
+        index_str = f"{index!r}"
+
+    sub_str = ""
+    if isinstance(sub, int):
+        # Need 0x prefix if index is not present
+        sub_str = f"{'0x' if not index_str else ''}{sub:02X}"
+    elif sub:
+        sub_str = f"{sub!r}"
+
+    return ":".join(s for s in (index_str, sub_str) if s)

canopen/variable.py

@@ -3,6 +3,7 @@
 from collections.abc import Mapping
 
 from canopen import objectdictionary
+from canopen.utils import pretty_index
 
 logger = logging.getLogger(__name__)
 
@@ -23,10 +24,10 @@ def __init__(self, od: objectdictionary.ODVariable):
         self.subindex = od.subindex
 
     def __repr__(self) -> str:
-        suffix = f":{self.subindex:02X}" if isinstance(self.od.parent,
+        subindex = self.subindex if isinstance(self.od.parent,
             (objectdictionary.ODRecord, objectdictionary.ODArray)
-        ) else ""
-        return f"<{type(self).__qualname__} {self.name!r} at 0x{self.index:04X}{suffix}>"
+        ) else None
+        return f"<{type(self).__qualname__} {self.name!r} at {pretty_index(self.index, subindex)}>"
 
     def get_data(self) -> bytes:
         raise NotImplementedError("Variable is not readable")
@@ -76,7 +77,7 @@ def raw(self) -> Union[int, bool, float, str, bytes]:
         written as :class:`bytes`.
         """
         value = self.od.decode_raw(self.data)
-        text = f"Value of {self.name!r} (0x{self.index:04X}:{self.subindex:02X}) is {value!r}"
+        text = f"Value of {self.name!r} ({pretty_index(self.index, self.subindex)}) is {value!r}"
         if value in self.od.value_descriptions:
             text += f" ({self.od.value_descriptions[value]})"
         logger.debug(text)

test/test_eds.py

@@ -2,6 +2,7 @@
 import unittest
 import canopen
 from canopen.objectdictionary.eds import _signed_int_from_hex
+from canopen.utils import pretty_index
 
 EDS_PATH = os.path.join(os.path.dirname(__file__), 'sample.eds')
 
@@ -236,15 +237,15 @@ def test_export_eds(self):
 
                     for evar, avar in zip(expected_vars, actual_vars):
                         self.assertEqual(getattr(avar, "data_type", None), getattr(evar, "data_type", None),
-                                         f" mismatch on 0x{evar.index:04X}:{evar.subindex:02X}")
+                                         f" mismatch on {pretty_index(evar.index, evar.subindex)}")
                         self.assertEqual(getattr(avar, "default_raw", None), getattr(evar, "default_raw", None),
-                                         f" mismatch on 0x{evar.index:04X}:{evar.subindex:02X}")
+                                         f" mismatch on {pretty_index(evar.index, evar.subindex)}")
                         self.assertEqual(getattr(avar, "min", None), getattr(evar, "min", None),
-                                         f" mismatch on 0x{evar.index:04X}:{evar.subindex:02X}")
+                                         f" mismatch on {pretty_index(evar.index, evar.subindex)}")
                         self.assertEqual(getattr(avar, "max", None), getattr(evar, "max", None),
-                                         f" mismatch on 0x{evar.index:04X}:{evar.subindex:02X}")
+                                         f" mismatch on {pretty_index(evar.index, evar.subindex)}")
                         if doctype == "dcf":
                             self.assertEqual(getattr(avar, "value", None), getattr(evar, "value", None),
-                                             f" mismatch on 0x{evar.index:04X}:{evar.subindex:02X}")
+                                             f" mismatch on {pretty_index(evar.index, evar.subindex)}")
 
                         self.assertEqual(self.od.comments, exported_od.comments)

test/test_utils.py

@@ -0,0 +1,20 @@
+import unittest
+from canopen.utils import pretty_index
+
+
+class TestUtils(unittest.TestCase):
+
+    def test_pretty_index(self):
+        self.assertEqual(pretty_index(0x12ab), "0x12AB")
+        self.assertEqual(pretty_index(0x12ab, 0xcd), "0x12AB:CD")
+        self.assertEqual(pretty_index(0x12ab, ""), "0x12AB")
+        self.assertEqual(pretty_index("test"), "'test'")
+        self.assertEqual(pretty_index("test", 0xcd), "'test':CD")
+        self.assertEqual(pretty_index(None), "")
+        self.assertEqual(pretty_index(""), "")
+        self.assertEqual(pretty_index("", ""), "")
+        self.assertEqual(pretty_index(None, 0xab), "0xAB")
+
+
+if __name__ == "__main__":
+    unittest.main()