CANopenNode
Revision number
This addresses the following issues:
- Making RevisionNumber (and OrderCode) EDS fields editable in the GUI - DONE
- Storing all numeric id fields (eg. Vendor ID, Revision Number) in numeric variables and correctly handle decimal / Hex conversion for the GUI textboxes - DONE
The following topics will be implemented in a separate MR:
- Linking EDS DeviceInfo / XDD DeviceIdentity fileds to the respective object values (eg. 0x1018, 0x1008-A) - Deferred to a separate MR
- Adding some CLI option to EDSSharp way to update the identity fields so that we can use a script to update the XDD/EDS files programmatically. (this might require its own branch) - Deferred to a separate MR
Initial work done by @temi54c1l8 in #113 Resolves #154 and #155 (partially)
I've stumbled on an issue related to EDS<->XDD conversion. CANOpenEditor seems to use an internal representation of the device profile based on the EDS format (EDSsharp class). When trying to add support for the FileRevision field of the EDS file, I noticed that this is not specified in the XDD format. Instead, if my understanding is correct, the XDD format includes just a "fileVersion" string field with "Vendor specific version of the file", i.e. no fixed format/pattern. So oconverting between that and the FilerVersion/FileRevision Unsigned8 fields of the EDS specification is not trivial and may not even be possible. We could try to use a FileVersion.FileRevision format for the XDD fileVersion. However, my first concern is how to store internal the fileVersion read from a XDD file WITHOUT CONVERTING IT so that an arbitary vendor string can be preserved when saving the XDD again. Is it OK to add / remove fields in the EDSsharp class in a way that breaks 1:1 relation with EDS fields? My preference would be to have a fileVersion string in EDSsharp class and only attempts to convert to/from numeric FileVersion / FileRevision fileds when reading or writing a EDS file. Is that OK with the project's style? @CANopenNode
I've stumbled on an issue related to EDS<->XDD conversion. CANOpenEditor seems to use an internal representation of the device profile based on the EDS format (EDSsharp class). When trying to add support for the FileRevision field of the EDS file, I noticed that this is not specified in the XDD format. Instead, if my understanding is correct, the XDD format includes just a "fileVersion" string field with "Vendor specific version of the file", i.e. no fixed format/pattern. So oconverting between that and the FilerVersion/FileRevision Unsigned8 fields of the EDS specification is not trivial and may not even be possible. We could try to use a FileVersion.FileRevision format for the XDD fileVersion. However, my first concern is how to store internal the fileVersion read from a XDD file WITHOUT CONVERTING IT so that an arbitary vendor string can be preserved when saving the XDD again. Is it OK to add / remove fields in the EDSsharp class in a way that breaks 1:1 relation with EDS fields? My preference would be to have a fileVersion string in EDSsharp class and only attempts to convert to/from numeric FileVersion / FileRevision fileds when reading or writing a EDS file. Is that OK with the project's style? @CANopenNode
I have implemented my suggestion for now, i.e. the file version is read from XDD and stored internally as string and only attempted to be converted to FileVersion.FileRevision when writing an EDS file
The first 2 goals of the MR are done - pending testing / review from someone else too. For the other 2 goals I have decided to implement them separately, after aligning with the project's core members. MR is ready for review. @CANopenNode do I need to do anything to get the review started?
The first 2 goals of the MR are done - pending testing / review from someone else too. For the other 2 goals I have decided to implement them separately, after aligning with the project's core members. MR is ready for review. @CANopenNode do I need to do anything to get the review started?
Review will start as soon as a reviewer is available without needing to do anything no worries (give it a week).
There is quite a lot of changes. I didn't go into the details. However, changing CanOpen.proto is always very delicate. Vendor ID and others are already in object 1018, which is mandatory anyway. Core database should not become inconsistent.
In the current state of the program it is hard to add "simple things". My current goal is more ambitious, as described here: https://github.com/CANopenNode/CANopenEditor/issues/190#issuecomment-3551548838
There is quite a lot of changes. I didn't go into the details. However, changing CanOpen.proto is always very delicate. Vendor ID and others are already in object 1018, which is mandatory anyway. Core database should not become inconsistent.
Indeed I needed to update quite a few different parts of the code for this to work, but I believe it's in good shape now. We have already been using the updated version in our project. I had missed filling in the mapping for the added fields in CanOpen.proto, I've fixed that. I believe the fields I've added are specified for EDS and XDD files, and although they are replicated in 0x1018, some existing fields are already in the DeviceInfo / DeviceIdentity sections so I don't see a big problem. Automatically linking the DeviceInfo fileds with 0x1018 is a good next step but not required for this update to work.
In the current state of the program it is hard to add "simple things". My current goal is more ambitious, as described here: #190 (comment)
The code may not be perfect but it works and this updates also seems to work, i.e. the new feature is operational and nothing seems broken.
I had missed filling in the mapping for the added fields in CanOpen.proto, I've fixed that. I believe the fields I've added are specified for EDS and XDD files, and although they are replicated in 0x1018, some existing fields are already in the DeviceInfo / DeviceIdentity sections so I don't see a big problem.
I don't agree changing CanOpen.proto file at this time. Methods can be used to extract information from 1018 object. Standards should be complied by the exporters, but core database must not be messy. Information must not duplicate.
I didn't check other parts of this PR.
I had missed filling in the mapping for the added fields in CanOpen.proto, I've fixed that. I believe the fields I've added are specified for EDS and XDD files, and although they are replicated in 0x1018, some existing fields are already in the DeviceInfo / DeviceIdentity sections so I don't see a big problem.
I don't agree changing CanOpen.proto file at this time. Methods can be used to extract information from 1018 object. Standards should be complied by the exporters, but core database must not be messy. Information must not duplicate.
I didn't check other parts of this PR.
Although I don't think it's really messy, I can revert the CanOpen.proto changes, as they are not really needed for the feature to work - I had not mapped the new fields until my latest update a few minutes before. Still, It makes more sense to me to use the DeviceInfo / DeviceIdentity structure as the ground truth for the 0x1018 fields and not vice versa. Keep in mind, it's not only the exporters but the importers that we need to think about. The identity fields are duplicated in EDS / XDD files anyway and the importers need to handle that, including the case where 0x1018 does not agree with the DeviceIdentity section. I've reverted CanOpen.proto without affecting the feature
The identity fields are duplicated in EDS / XDD files anyway and the importers need to handle that, including the case where 0x1018 does not agree with the DeviceIdentity section.
Better to handle imported messy eds file than to export it by default.