Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## [Unreleased]
### Added
- Parsing of Dictionary tables

## [7.5.2] - 2025-11-24
### Added
- Fix PDO item creation for 0 bits registers.
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Use an environment with a certain Python version::

Install all dependencies::

poetry install --all-groups
poetry install --all-groups --extras virtual_drive


Project Tasks - Poe The Poet plugin
Expand Down
106 changes: 106 additions & 0 deletions ingenialink/dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,23 @@ def __iter__(self) -> Iterator[Union[str, None]]:
return iter((id_hex_string, self.affected_module, self.error_type, self.description))


@dataclass
class DictionaryTable:
"""Class to store a dictionary table."""

id: str
"""Table ID."""

axis: Optional[int]
"""Axis the table belongs to. None if belongs to non-multiaxis register."""

id_index: str
"""The uid of the register used to access the table index."""

id_value: str
"""The uid of the register used to read/write the table value"""


@dataclass
class DictionaryDescriptor:
"""Class to store a dictionary error."""
Expand Down Expand Up @@ -386,6 +403,7 @@ def __init__(self, dictionary_path: str, interface: Interface) -> None:
self.safety_modules = {}
self._registers = {}
self.subnodes = {}
self._tables = dict[int, dict[str, DictionaryTable]]()
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type hint syntax dict[int, dict[str, DictionaryTable]]() is incorrect. When initializing an empty dictionary, you should use {} without the type annotation, or use dict() without the type parameter. The correct syntax would be:

self._tables: dict[int, dict[str, DictionaryTable]] = {}

or simply:

self._tables = {}

The current syntax will raise a TypeError at runtime.

Suggested change
self._tables = dict[int, dict[str, DictionaryTable]]()
self._tables = {}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot according to what rule, isn't dict a generic typevar?

self.path = dictionary_path
"""Path of the dictionary."""
self.interface = interface
Expand Down Expand Up @@ -478,6 +496,7 @@ def __add__(self, other_dict: "Dictionary") -> "Dictionary":
other_dict_copy = copy.deepcopy(other_dict)
self_dict_copy._merge_registers(other_dict_copy)
self_dict_copy._merge_errors(other_dict_copy)
self_dict_copy._merge_tables(other_dict_copy)
self_dict_copy._merge_attributes(other_dict_copy)
self_dict_copy._set_image(other_dict_copy)
return self_dict_copy
Expand Down Expand Up @@ -550,6 +569,41 @@ def get_register(self, uid: str, axis: Optional[int] = None) -> Register:

return matching_registers[0]

@weak_lru()
def get_table(self, uid: str, axis: Optional[int] = None) -> DictionaryTable:
"""Gets the targeted table.

Args:
uid: table uid.
axis: axis. Should be specified if multiaxis, None otherwise.

Raises:
KeyError: if the specified axis does not exist.
KeyError: if the table is not present in the specified axis.
ValueError: if the table is not found in any axis, if axis is not provided.
ValueError: if the table is found in multiple axis, if axis is provided.

Returns:
table.
"""
if axis is not None:
if axis not in self._tables:
raise KeyError(f"{axis=} does not exist.")
if uid not in self._tables[axis]:
raise KeyError(f"Table {uid} not present in {axis=}")
return self._tables[axis][uid]

matching_tables: list[DictionaryTable] = [
self._tables[axis][uid] for axis in self._tables if uid in self._tables[axis]
]

if len(matching_tables) == 0:
raise ValueError(f"Table {uid} not found.")
if len(matching_tables) > 1:
raise ValueError(f"Table {uid} found in multiple axis. Axis should be specified.")

return matching_tables[0]

@abstractmethod
def read_dictionary(self) -> None:
"""Reads the dictionary file and initializes all its components."""
Expand Down Expand Up @@ -654,6 +708,18 @@ def _merge_errors(self, other_dict: "Dictionary") -> None:
"""
self.errors.update(other_dict.errors)

def _merge_tables(self, other_dict: "Dictionary") -> None:
"""Add the tables from another dictionary to the dictionary instance.

Args:
other_dict: The other dictionary instance.

"""
for axis, tables in other_dict._tables.items():
if axis not in self._tables:
self._tables[axis] = {}
self._tables[axis].update(tables)

def _set_image(self, other_dict: "Dictionary") -> None:
"""Set the image attribute.

Expand Down Expand Up @@ -795,6 +861,13 @@ class DictionaryV3(Dictionary):
__ERRORS_ELEMENT = "Errors"
__ERROR_ELEMENT = "Error"

__TABLES_ELEMENT = "Tables"
__TABLE_ELEMENT = "Table"
__TABLE_ID_ATTR = "id"
__TABLE_AXIS_ATTR = "axis"
__TABLE_ID_INDEX_ATTR = "id_index"
__TABLE_ID_VALUE_ATTR = "id_value"

__LABELS_ELEMENT = "Labels"
__LABEL_ELEMENT = "Label"
__LABEL_LANG_ATTR = "lang"
Expand Down Expand Up @@ -1062,6 +1135,7 @@ def __read_device_eth(self, root: ElementTree.Element) -> None:
self.__read_mcb_register(register_element)
errors_element = self._find_and_check(root, self.__ERRORS_ELEMENT)
self._read_errors(errors_element, self.__ERROR_ELEMENT)
self._read_tables(root)

def __read_device_ecat(self, root: ElementTree.Element) -> None:
"""Process ECATDevice element.
Expand All @@ -1078,6 +1152,7 @@ def __read_device_ecat(self, root: ElementTree.Element) -> None:
self.__read_canopen_object(register_element)
errors_element = self._find_and_check(root, self.__ERRORS_ELEMENT)
self._read_errors(errors_element, self.__ERROR_ELEMENT)
self._read_tables(root)
safety_pdos_element = root.find(self.__SAFETY_PDOS_ELEMENT)
if safety_pdos_element is not None:
self.__read_safety_pdos(safety_pdos_element)
Expand All @@ -1100,6 +1175,7 @@ def __read_device_can(self, root: ElementTree.Element) -> None:
self.__read_canopen_object(register_element)
errors_element = self._find_and_check(root, self.__ERRORS_ELEMENT)
self._read_errors(errors_element, self.__ERROR_ELEMENT)
self._read_tables(root)

def __read_subnodes(self, root: ElementTree.Element) -> None:
"""Process Subnodes element and fill subnodes.
Expand Down Expand Up @@ -1519,6 +1595,36 @@ def __read_safety_module(
application_parameters=application_parameters,
)

def _read_tables(self, root: ElementTree.Element) -> None:
"""Process Tables element.

Args:
root: Root parent element where the <Tables> element is located.

"""
tables_element = root.find(self.__TABLES_ELEMENT)
if tables_element is None:
# Element not mandatory
return

table_elements = tables_element.findall(self.__TABLE_ELEMENT)
for table_element in table_elements:
uid = str(table_element.attrib.get(self.__TABLE_ID_ATTR))
_axis = table_element.attrib.get(self.__TABLE_AXIS_ATTR)
axis = int(_axis) if _axis is not None else 0
id_index = str(table_element.attrib.get(self.__TABLE_ID_INDEX_ATTR))
id_value = str(table_element.attrib.get(self.__TABLE_ID_VALUE_ATTR))

table = DictionaryTable(
id=uid,
axis=axis if _axis is not None else None,
Comment on lines +1614 to +1620
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for determining the axis value has a potential issue. When _axis is None, you set axis = 0 on line 1614, but then on line 1620 you check if _axis is not None to determine what to store in the DictionaryTable.axis field. This creates an inconsistency:

  • For tables without an axis attribute in XML, axis will be 0 (integer) for dictionary storage
  • But table.axis will be None

This means a table without an axis attribute will be stored in self._tables[0] but its axis property will be None. This could cause confusion. Consider either:

  1. Storing tables without axis in a different key (e.g., -1 or None if the dict supports it)
  2. Ensuring consistency by setting table.axis = 0 when no axis is specified

Looking at the test expectations, it seems tables without axis should be stored at index 0 but have axis=None, which is the current behavior. However, this should be clearly documented.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@polfeliu polfeliu Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is a bit confusing, also for me. This just replicates the structure for registers, that for non-axis are also stored on axis 0. Also note that the index of _tables is something private to the dictionary.
table.axis is public

id_index=id_index,
id_value=id_value,
)
if axis not in self._tables:
self._tables[axis] = {}
self._tables[axis][uid] = table


class DictionaryV2(Dictionary):
"""Class to represent a Dictionary V2."""
Expand Down
2 changes: 1 addition & 1 deletion tests/resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
TEST_DICT_ECAT_EOE_SAFE_v3 = (resources_path / "test_dict_ecat_eoe_safe_v3.0.xdf").as_posix()
TEST_DICT_ECAT_EOE_v3 = (resources_path / "test_dict_ecat_eoe_v3.0.xdf").as_posix()
TEST_CONFIG_FILE = (resources_path / "test_config_file.xcf").as_posix()

TEST_DICTIONARY_WITH_TABLES = (resources_path / "dictionary_with_tables_minimal.xdf3").as_posix()

__all__ = [
"DEN_NET_E_2_8_0_xdf_v3",
Expand Down
Loading