Skip to content

Custom Options for EDS file#615

Open
FedericoSpada wants to merge 1 commit intocanopen-python:masterfrom
FedericoSpada:manage_unknown_entries
Open

Custom Options for EDS file#615
FedericoSpada wants to merge 1 commit intocanopen-python:masterfrom
FedericoSpada:manage_unknown_entries

Conversation

@FedericoSpada
Copy link
Copy Markdown

@FedericoSpada FedericoSpada commented Sep 21, 2025

Vector CANeds tool groups all unknown options contained in an EDS File in a dedicated section called "Unknown Entries".
We use that section to save additional information that is not considered by the CANopen specifications (for example: unit, factor, offset, category, etc.), and so I had to modify this library to read and write those options.

If you open a file that contains unknown option names, they will be grouped in the "custom_options" dictionary.
If you desire to save a value with a specific option name, you can update the "custom_options" dictionary accordingly.

P.S.: In the meantime, I've improved some of the bad code.

Allowed to Read/Write options not defined by the EDS Standard.
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 22, 2025

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
canopen/objectdictionary/eds.py 89.47% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@bizfsc
Copy link
Copy Markdown

bizfsc commented Apr 28, 2026

Thanks for adding custom options support! I reviewed the changes and have a few observations:

1. Duplicate _get_custom_options() call for VAR types

For VAR / DOMAIN objects, _get_custom_options() is called twice:

  • Once in import_eds() (line 134): var.custom_options = _get_custom_options(eds, section)
  • Once at the end of build_variable() (line 352): var.custom_options = _get_custom_options(eds, section)

The second call overwrites the first. One of them should be removed. Since build_variable() is also called for subindexes, keeping it only there seems cleanest – then the calls in import_eds() for ARR/RECORD containers still make sense (they parse the parent section's custom options).

2. _STANDARD_OPTIONS is incomplete

The list is missing at least:

  • "NrOfEntries" (used in [index]Name sections and SubNumber calculation)
  • "SubNumber" (used in export_record)
  • "ObjFlags" and "Denotation" (valid per CiA 306)
  • Numeric keys like "0", "1", etc. (used as subindex names in CompactSubObj sections)

Any standard option not in this list will incorrectly end up in custom_options. Consider also making the comparison case-insensitive (e.g. using a lowercase set) as a safety measure, even though optionxform = str is set.

3. No tests

It would be great to add a test that:

  • Imports an EDS with a non-standard option (e.g. Category=MyCategory) on a variable
  • Verifies it appears in var.custom_options and standard options do not
  • Exports back to EDS and verifies the custom option is preserved

4. Typo fix should be a separate commit

The manufacturer_idicesmanufacturer_indices rename is a good fix but is unrelated to the custom options feature. Splitting it into its own commit would keep the history clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants