CANopenNode / CANopenEditor

CANopen Object Dictionary Editor
GNU General Public License v3.0
115 stars 57 forks source link

protobuffer poc #115

Open nimrof opened 1 month ago

nimrof commented 1 month ago

protobuffer test code for #112 I have translate it mostly 1:1 but divided it a little. Feel free to ignore that.

Testcode

CanOpenDevice dev = new CanOpenDevice();
dev.DeviceComissioning = new CanOpen_DeviceComissioning();
dev.DeviceInfo = new CanOpen_DeviceInfo();
dev.FileInfo = new CanOpen_FileInfo();

dev.DeviceComissioning.Baudrate = 125000;
var obj2000 = new OdEntry()
{
    Disabled = true,
    ParameterName = "Custom object",
    Denotation = "Another name",
    Description = "Testing",
    ObjectType = LibEDSsharp.ObjectType.Var,
    CountLabel = "LABEL_1",
    StorageGroup = "XY",
    FlagsPDO = true
};
var obj2000_00 = new OdSubEntry
{
    DataType = LibEDSsharp.DataType.Unsigned32,
    AccessSDO = LibEDSsharp.AccessSDO.Ro,
    AccessPDO = LibEDSsharp.AccessPDO.No,
    AccessSRDO = LibEDSsharp.AccessSRDO.Tx,
    DefaultValue = "123",
    ParameterValue = "0",
    LowLimit = "0",
    HighLimit = "0",
    StringLengthMin = 1
};
obj2000.SubObjects.Add("00", obj2000_00);
dev.Objects.Add("2000", obj2000);

// write the protobuffer message in json format (alternative is binary wireformat)
var formatterConfig = new JsonFormatter.Settings(true).WithIndentation().WithFormatDefaultValues(true);
var formatter = new JsonFormatter(formatterConfig);
var rawJsonString = formatter.Format(dev);

//write & read rawJsonString to file
File.WriteAllText("test.json", rawJsonString);

//read parse the raw json back into a object
var parserConfig = new JsonParser.Settings(100);
var parser = new JsonParser(parserConfig);
var dev2 = parser.Parse<CanOpenDevice>(rawJsonString);

result

{
  "FileInfo": {
    "FileVersion": "",
    "Description": "",
    "CreatedBy": "",
    "ModifiedBy": ""
  },
  "DeviceInfo": {
    "VendorName": "",
    "VendorNumber": 0,
    "ProductName": "",
    "ProductNumber": 0,
    "BaudRate10": false,
    "BaudRate20": false,
    "BaudRate50": false,
    "BaudRate125": false,
    "BaudRate250": false,
    "BaudRate500": false,
    "BaudRate800": false,
    "BaudRate1000": false,
    "Granularity": 0,
    "NrOfRxPDO": 0,
    "NrOfTxPDO": 0,
    "LssSlave": false,
    "LssMaster": false,
    "NodeGuardingSlave": false,
    "NodeGuardingMaster": false,
    "NrOfNodeGuardingMonitoredNodes": 0
  },
  "DeviceComissioning": {
    "NodeID": 0,
    "NodeName": "",
    "Baudrate": 125000,
    "NetNumber": 0,
    "NetworkName": "",
    "CANopenManager": false,
    "LSSSerialNumber": 0
  },
  "Objects": {
    "2000": {
      "Disabled": true,
      "ParameterName": "Custom object",
      "Denotation": "Another name",
      "Description": "Testing",
      "ObjectType": "ObjectType_VAR",
      "ComplexDataType": 0,
      "SubObjects": {
        "00": {
          "SubParameterName": "",
          "Denotation": "",
          "DataType": "UNSIGNED32",
          "AccessSDO": "AccessSDO_ro",
          "AccessPDO": "no",
          "AccessSRDO": "AccessSRDO_tx",
          "DefaultValue": "123",
          "ParameterValue": "0",
          "LowLimit": "0",
          "HighLimit": "0",
          "StringLengthMin": 1
        }
      },
      "CountLabel": "LABEL_1",
      "StorageGroup": "XY",
      "FlagsPDO": true
    }
  }
}
CANopenNode commented 1 month ago

Protobuffer seems OK to me. proto files are very clear and cs files are generated automatically for them. I think, we should continue with it.

I'm very inexperienced with configuring Visual studio, but once project is set-up correctly I can do the job with adjustment of importers, exporters, etc. Maybe you can do some more integration, It took me quite a lot of time for simple testing.

Maybe we could avoid json format and use binary wireformat directly, if it is more reliable.

nimrof commented 1 month ago

Protobuffer seems OK to me. proto files are very clear and cs files are generated automatically for them. I think, we should continue with it.

ok 👍

I'm very inexperienced with configuring Visual studio, but once project is set-up correctly I can do the job with adjustment of importers, exporters, etc. Maybe you can do some more integration, It took me quite a lot of time for simple testing.

Sure, so i should make this?

Maybe we could avoid json format and use binary wireformat directly, if it is more reliable.

Its not more reliable, but it uses way less space and way faster to read/write. I think json is the right default choice as it is human readable and looks good in diff, but if you want to make canopennode runtime configurable then the binary wireformat is the way to go, but then you could export for that use only. Alternatively we could support two formats, its just a few lines extra.

CANopenNode commented 1 month ago

Maybe we can continue with this branch an make a complete transition to protobuffer. For the start, you may do as you suggest, we also have a TODO from #102: A uniform way do interact with importer/exporter

My suggestion for open/save, import/export:

  1. Lets make our protobuffer file the only format for project open/save. File extension may be, for example, .codev (canopen device) for binary file or .codev.json for json. (On file save dialog one can choose binary or json format).
  2. CANopenEditor already supports multiple devices open at once (tabs on the left). We can make a protobuffer containing several canopen devices and save it in file with extension .conet (canopen network). That is just an idea, maybe we can do that later.
  3. Import is separate for:
    • xdd, xpd, xdc, with automatic version dectection
    • eds, dcf
  4. Export for:
    • xdd, xpd, xdc v1.0 and v1.1
    • eds, dcf
    • CANopenNode v1.3 or v4 (version should be select-able from export dialog, not be part of settings. To remove settings also GUI will have to be updated)
    • documentation, etc.
    • Exporters won't contain non-standard properties from protobuffer.

Next is precisely specifying protobuffer properties and integrate it into existing importers/exporters and GUI. I don't expect, anything will be broken in CANopenEditor. I can do that part of the job, but of course, cooperation is welcome.

EDS import/export will have to be rewritten. My approach for exporter would be to simply generate it as a text file, just write each property separately. If there is better approach in c#, please let me know. Similar for importer: read tokens and sort them into properties one by one.

Our protobuffer should have some extra validation control. For example, OD object of type VAR should have only one subindex, etc. Maybe it is good enough, if validator just writes warnings somewhere. How could we add our validators to protobuffer, can we extend classes with our methods?

nimrof commented 1 month ago

Maybe we can continue with this branch an make a complete transition to protobuffer. For the start, you may do as you suggest, 👍

we also have a TODO from #102: A uniform way do interact with importer/exporter

Yes, in my head that was limited to some way to make information about filetypes and parameter passed to it

My suggestion for open/save, import/export:

  1. Lets make our protobuffer file the only format for project open/save. File extension may be, for example, .codev (canopen device) for binary file or .codev.json for json. (On file save dialog one can choose binary or json format).

Sounds good as long as we can import/export other format via import/export

2. CANopenEditor already supports multiple devices open at once (tabs on the left). We can make a protobuffer containing several canopen devices and save it in file with extension `.conet` (canopen network). That is just an idea, maybe we can do that later.

Suggestion, can we only support multiple and if there is only one then its only one

3. Import is separate for:

   * xdd, xpd, xdc, with automatic version dectection
   * eds, dcf

4. Export for:

   * xdd, xpd, xdc v1.0 and v1.1
   * eds, dcf
   * CANopenNode v1.3 or v4 (version should be select-able from export dialog, not be part of settings. To remove settings also GUI will have to be updated)
   * documentation, etc.
   * Exporters won't contain non-standard properties from protobuffer.

That sound good, i am still on 1.3 and very much agree moving it away from settings.

Next is precisely specifying protobuffer properties and integrate it into existing importers/exporters and GUI. I don't expect, anything will be broken in CANopenEditor. I can do that part of the job, but of course, cooperation is welcome.

EDS import/export will have to be rewritten. My approach for exporter would be to simply generate it as a text file, just write each property separately. If there is better approach in c#, please let me know. Similar for importer: read tokens and sort them into properties one by one.

I do think the EDS import/exporter is very good use of c# tbh We can add custom protobuffer options (similar to c# attributes) to messages/enums and fields, maybe that can help like it does in EDS export/importer.

Our protobuffer should have some extra validation control. For example, OD object of type VAR should have only one subindex, etc. Maybe it is good enough, if validator just writes warnings somewhere. How could we add our validators to protobuffer, can we extend classes with our methods?

To to that we must make individual messages for var, array and then put the limitation there, but there is no way to limit the size of the array to a value, if it is repeated it can be way longer than 255 so we do need extra code anyway

trojanobelix commented 1 month ago

Checking the validity of CANopen objects is very complex. I have done this once with the CiA conformance testing tool and found it horrible.

The CANopen CC electronic data-sheet (EDS) checker developed and supported by Vector has been updated and revised and is now called CANeds. Unfortunately, it is no longer available as a stand-alone tool. The EDS Checker tool is embedded in the Vector tool chain (e.g. CANalyzer.CANopen). Unfortunately, you now have to download and install a multi-gigabyte package.

But the tool is very helpful and you are impressed by all the things that actually need to be checked. You don't always want to do this yourself in your own importer.

If I have problems, I always ask for the EDS or XDD and see what the CANeds recognizes for problems. Sometimes, however, it is a non-standardised handling of the CANopen editor as with the module:

CiA 306: If several modules are gathered to form a new Sub-Index, then the number is 0, followed by semicolon and the number of bits that are created per module to build a new sub-index. In example 2 bit modules with 8 bit objects: The first Sub-Index is built upon modules 1-4, the next upon modules 5-8 etc.: Count=0;2. The objects are created, when a new byte begins: Module 1 creates the Sub-Index 1; modules 2-4 fill it up; module 5 creates Sub-Index 2 and so on

CANopenEditor: Exception if count has the format Count=0;x

CANopenNode commented 1 month ago
  1. CANopenEditor already supports multiple devices open at once (tabs on the left). We can make a protobuffer containing several canopen devices and save it in file with extension .conet (canopen network). That is just an idea, maybe we can do that later.

Suggestion, can we only support multiple and if there is only one then its only one

Maybe yes. We always open/save a project (network) file, which contains an array of zero or more devices and some own properties (baudrate for example). But importer/exporter should include an option for single device in protobuf format.

CANopenNode commented 1 month ago

I do think the EDS import/exporter is very good use of c# tbh We can add custom protobuffer options (similar to c# attributes) to messages/enums and fields, maybe that can help like it does in EDS export/importer.

I don't like attributes in current EDS importer/exporter very much. But each programmer has his own style, so this doesn't matter much.

For me it is the most important, that we have our protobuffer as simple as possible, should include all that is needed, well specified and also flexible. Standards have differences in details. I think, our protobuf must contain best from the standards and we should follow them as much as reasonable. But we don't have to stick with all their rules. Of course, exporters must comply them fully, but our protobuf is under our control. I wish it to be clear and logical and don't care much about differences in the standards. For example, accessType as specified by EDS standard is not very optimal according to my opinion.

I would like to think about EDS exporter only in EDS exporter code. I don't like to add additional "eds format" specific rules into protobuf, just to make code look more like c#. Some properties will be transferred to eds file just as they are, like "name=value", other properties (accessType) will need some extra code. Recently we had a discussion about a "revisionNumber" and there was just no simple solution for that minor issue. Besides that, eds is not the only exporter, there is also xdd. And new CANopenFD standard does not even have eds specification, only renewed xdd...

CANopenNode commented 1 month ago

Our protobuffer should have some extra validation control. For example, OD object of type VAR should have only one subindex, etc. Maybe it is good enough, if validator just writes warnings somewhere. How could we add our validators to protobuffer, can we extend classes with our methods?

To to that we must make individual messages for var, array and then put the limitation there, but there is no way to limit the size of the array to a value, if it is repeated it can be way longer than 255 so we do need extra code anyway

Checking the validity of CANopen objects is very complex. I have done this once with the CiA conformance testing tool and found it horrible.

I think, we will need some extra code around auto-generated protobuf files. This would be some helper functions, generic validators, etc.

I think, also importers or exporters may have their own validators, for example CANopenNode_V4 exporter does much checking. Validator may add a warning into the "Warnings" class. We should have well defined approach for validators. But we can make more or less complex validators for separate exporters any time.

CANopenNode commented 1 week ago

CiA 306-1:2019 contains interesting table explaining different access types (eds, old and new(preferred) table format). But PDO access in new table format is still not very clear (T as TPDO, TR as TPDO and RPDO??). accessType

trojanobelix commented 1 week ago

More confusing is the Access Type = ro with a TRPDO. Would expect a TPDO.

CANopenNode commented 1 week ago

More confusing is the Access Type = ro with a TRPDO. Would expect a TPDO.

Probably it is just an error there. That standard is not a final version anyway. In CAN FD this is more clear. The same approach is also used by our new protobuffer: accesstype2

CANopenNode commented 1 week ago

I pushed protobuffer importer/exporter. Both are accessible from GUI. It seems they works correctly.

Tested: Load xdd file -> export to protobuffer file -> close file -> load protobuffer file -> save as xdd -> both xdds are the same, except metadata.

Exporter and importer were actually added to CanOpenXDD_1_1.cs file, so translation from EDSsharp works indirectly.

Nothing else was changed.

Program builds normally from visual studio, runs normally in windows.

I made proto definition file very slim for the start. If we will continue with it, we may add additional properties.

Protobuffer file (as CANopen device description) may be very easy to use with many other languages, including python. With few lines there is a complete database with object dictionary, etc. Thank you @nimrof for the idea and the initial example.

CANopenEditor is now little wider. I would like to make it much more slim (by using protobuffer as a core), but there must be a wider interest for this. Also new GUI would be necessary. We should somehow agree for the next steps. But for now protobuffer im/exporter is just enough for me.

CANopenNode commented 4 days ago

Only thing i really dislike is that it is added to the xdd source file and not its own file :)

My point of view: protobuf is (will be) our core and my new code inside xdd is importer/exporter for xdd. Actually I wrote exporter for xdd and temporary use it as exporter from eds (to xdd) to protobuf.

One think i dislike is some of the changes in csproj (see own comments)

It didn't work to me until I add "Unsafe" module. I did some automatic upgrade and it did what id did. Please fix it, I'm a little short here.

Yes missing a lot and I agree with MIT for protobuffer.

Most important question for me: Should we make a protobuf as a core database and make totally new eds importer/exporter and keep all eds related code away from protobuf. That will require update of the most of the code, but I think, it will be worth.

nimrof commented 4 days ago

Only thing i really dislike is that it is added to the xdd source file and not its own file :)

My point of view: protobuf is (will be) our core and my new code inside xdd is importer/exporter for xdd. Actually I wrote exporter for xdd and temporary use it as exporter from eds (to xdd) to protobuf.

Okay that makes more sense, just got hung up in ReadProtobuf & WriteProtobuf in xdd, but its a quick way to get something experimental working.

One think i dislike is some of the changes in csproj (see own comments)

It didn't work to me until I add "Unsafe" module. I did some automatic upgrade and it did what id did. Please fix it, I'm a little short here.

Oh, i need to do more runtime testing then

Yes missing a lot and I agree with MIT for protobuffer.

👍

Most important question for me: Should we make a protobuf as a core database and make totally new eds importer/exporter and keep all eds related code away from protobuf. That will require update of the most of the code, but I think, it will be worth.

agree, but slowly pls :)

CANopenNode commented 2 days ago

Only thing i really dislike is that it is added to the xdd source file and not its own file :)

My point of view: protobuf is (will be) our core and my new code inside xdd is importer/exporter for xdd. Actually I wrote exporter for xdd and temporary use it as exporter from eds (to xdd) to protobuf.

Okay that makes more sense, just got hung up in ReadProtobuf & WriteProtobuf in xdd, but its a quick way to get something experimental working.

I made xdd exporter/importer first, because this file is most familiar to me as an author.

agree, but slowly pls :)

Yes, I agree. I only ask you to fix this PR and then we merge it into main. So we have experimental protobuffer exporter/importer. For me this is quite a lot, because there are many possibilities to play with it.

If we once decide to put CANopenEditor on the next level, my proposal is the following:

If we decide. However, current version is good enough, bud I don't feel adding more functionality in it. Anyway, I wish we have a modern and rich CANopen GUI tool once in the future. Or maybe set of simpler tools? Slowly.