-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A uniform way do interact with importer/exporter #122
Comments
For the strings, it sounds good to me. |
Yes, It sounds good to me. exporter selection in preferences is used also in "DeviceODView.cs", which was introduced by me with version v4. But I don't like it - it slightly changes user interface according to canopennode version selection. For example, if v4 exporter is selected, exotic datatypes like UNSIGNED24 are not used, pdo and sdo access are handled differently. Or maybe we can keep selection in preferences, bit rename it from "Selected exporter" to something else and use that preference only in "DeviceODView.cs" |
I don't like the exporter selection in preferences either. It would be nice if there was a more elegant solution. |
Okay, then it will be something like the image AND into export
ok, I understand |
Hi @CANopenNode & @trojanobelix Starting to figure out why CanOpenNode version is a config thing This function is called almost every time something in the OD changes. CANopenEditor/libEDSsharp/PDOHelper.cs Line 375 in 40c02b3
It looks like its only affecting the pdo communication index(s) and removing/adding subindexes based on what is supported by different CanOpenNode versions...does that make sense? CANopenEditor/libEDSsharp/PDOHelper.cs Lines 427 to 434 in 40c02b3
CANopenEditor/libEDSsharp/PDOHelper.cs Lines 451 to 455 in 40c02b3
CANopenEditor/libEDSsharp/PDOHelper.cs Lines 474 to 481 in 40c02b3
So i suggest to change/fix it in the different canopennode exporters and generate a warning if something is changed. Not sure if when the changes are done, should it be fixed in the "EDS" structure (permanent changes is done to the "eds" file) or just fix in on export? |
Correct, in buildmappingsfromliststhe all the RX /TX communication objects and the mapping are created from scratch in the OD (based on the PDO list) The differences between the CANopeNode V1.3/V4 versions are taken into account. I agree with you that it should be a matter for the exporter to take the differences into account. |
I think I made that in 32ef9eb CANopenNode v1.3 (legacy) requires exact structure shape for PDO communication parameter, both RPDO and TPDO (and also for PDO mapping parameters). So that exporter should be fixed. It should add "compatibility entry" automatically, adapt "Highest sub-index supported" for RPDO, exclude "Event timer" for RPDO. CANopenNode V4 don't have such requirements, it is not strict about PDO structure shape, it should already add warnings when necessary. I suggest removing "isCANopenNode_V4" condition and make PDOhelper work as for CANopenNode V4 and add adaptions into legacy exporter. Here are OD requirements for V1.3: CO_OD.h and CO_PDO.h. Structures must match. |
So what i want to do is to expose a"array" of importers and exporters from libEDS that the GUI & CLI can use to list the exporters and importers
the array will be composed of classes that looks a little like this:
So this is currently the import/export functions
Importers looks ready to use right now (ignoring experimental protobuffer)
The problem with the exporters is the there are a lot of different parameters
Exporters
So can we reduce the number of different parameters?
suggestion: Use only filepath, folderpath can we get from filepath (like now)
suggestion: move gitversion from gui to libEDS so it does not have to be sent with parameter.
suggestion: Remove it, it is just Path.GetFileNameWithoutExtension(FileName) so it can be moved into the function
suggestion: expose multiple exporter with it false/true (almost like now)
suggestion: expose multiple exporter with it set to eds / dcf (like now)
Sounds good ?
@CANopenNode & @trojanobelix
The text was updated successfully, but these errors were encountered: