CANopenNode / CANopenEditor

CANopen Object Dictionary Editor
GNU General Public License v3.0
120 stars 60 forks source link

Change of 0x16xx change the SubIndex of 0x14xx #45

Closed trojanobelix closed 1 year ago

trojanobelix commented 1 year ago

If you change the objects from 0x1600 and save these changes, the number of subobjects in 0x16xx are also transferred to 0x14xx

Example: 6 Subs in 0x1602 are transfered to 0x1406 image

trojanobelix commented 1 year ago

Able to reproduce. If you add a new PDO in RX/TX Tab all PDO objects are deleted and rebuild with buildmappingsfromlists This sets the value of sub 0 (max supported sub-indexes) to "6", even if there are less defined (which is allowed if not used according DS301)

trojanobelix commented 1 year ago

There seems to be an error in the DS301

RPDO description says that sub 6 should not be implemented. But value range for Sub 0 is 2-6 TPDO description descriped in detail sub 6. But value range for Sub 0 is 2-5

RPDO

Sub-index 06h contains the SYNC start value.
This is not used by RPDOs. It shall not be implemented; in this case read or write access shall lead to the SDO abort transfer service (abort
code: 0609 0011
h).
OBJECT DESCRIPTION
Index | 1400h to 15FFh -- | -- Name | RPDO communication parameter Object code | RECORD Data type | PDO communication parameter record Category | Conditional;Mandatory for each supported RPDO
ENTRY DESCRIPTION
Sub-index | 00h -- | -- Description | highest sub-index supported Entry category | Mandatory Access | const PDO mapping | No Value range | 02h to 06h Default value | No

TPDO

.
Sub-index 06
h contains the SYNC start value. The SYNC start value of 0 shall indicate that the
counter of the SYNC message shall not be processed for this PDO. The SYNC start value 1 to
240 shall indicate that the counter of the SYNC message shall be processed for this PDO. In
case the counter of the SYNC message is not enabled (see 7.5.2.22) sub-index 06
h shall be
ignored. The SYNC message of which the counter value equals the SYNC start value shall be
regarded as the first received SYNC message. The value shall not be changed while the PDO
exists (bit 31 of sub-index 01
h is set to 0b).
NOTE if the CANopen device detects on switch into the NMT state operational that the SYNC
counter value received is higher than the SYNC start value, then the CANopen device has to wait
a full cycle until the correct SYNC counter is received.
OBJECT DESCRIPTION
Index | 1800h to 19FFh -- | -- Name | TPDO communication parameter Object code | RECORD Data type | PDO communication parameter record Category | Conditional;Mandatory for each supported TPDO
ENTRY DESCRIPTION
Sub-index | 00h -- | -- Description | highest sub-index supported Entry category | Mandatory Access | const PDO mapping | No Value range | 02h to 05h
trojanobelix commented 1 year ago

@CANopenNode Can you please give me your assessment of my suspicion of the error in the DS301. And how you think the Sub0 for TPDO/RPDO should be set in buildmappingsfromlists

trojanobelix commented 1 year ago

actual bugfix in 415b7ed689c3f43ac47e2ba44193863fc48cb061 is:

  1. Default value of sub 0 is 6 for TPDO and 5 for RPDO (instead of 6 for both). This still violates the Value range 02h to 05 for TPDOs if range is correct in DS301.

sub.defaultvalue = slot.isTXPDO()? "6" : "5";// max supported sub index TPDO=6, RPDO=5 according DS301

  1. Add missing Sub 5 for RPDO, so the default value 5 for sub 0 fits
                    sub = new ODentry("event timer", (ushort)slot.ConfigurationIndex, 5);
                    sub.datatype = DataType.UNSIGNED16;
                    sub.defaultvalue = slot.eventtimer.ToString();
                    sub.accesstype = EDSsharp.AccessType.rw;
                    config.addsubobject(0x05, sub);
trojanobelix commented 1 year ago

Delete/Add PDO in RX/TX seems to work, but if tab "Object Directory" gets focus, the view is not updated (Sub 5 not added if there were less). All Subs are updated after editing a value.

CANopenNode commented 1 year ago

Hi, I took a look into buildmappingsfromlists() function. As mentioned #47, there is a problem. I suggest to generate different objects for "LEGACY" and for "V4" configuration, for both CANopenNode versions to work correctly. There is not many difference, but anyway. And there must always be 1+8 subindexes in PDO mapping parameters.

I suggest to generate objects, exactly as specified in files:

CANopenNode commented 1 year ago

I made some updates, mostly in TX and RX PDO mapping tabs. It seems it works correctly now, for both, LEGACY and V4. I updated the bugfix branch and also merged it to the main branch, so all should be clean now. Binaries on build branch are also available.

@trojanobelix can you please verify, if everything works OK for you.

trojanobelix commented 1 year ago

@CANopenNode Thank you for the changes. Basically, it looks good.

A few comments: You reverted the changes from #40 and changed the odObjectType from "ARAAY" to "ARR" and "RECORD" to "REC" again. I tried to keep the object type consistent with the DS301 naming, i.e. ARRAY and RECORD. "ARR" and "REC" caused problems elsewhere (I can't find the issue quickly).

You have changed PDOmapping="optional" to PDOmapping="TPDO" in index="1001", but according to DS301 V4.02 this is "optional".

The new default 0xC000 for PDO is correct according to DS301 from PDO number 5, for 1-4 it is for RPDOs "Node-ID + PDOno. 100h + 100h" (200h...500h) or for TPDOs "Node-ID + PDOno. 100h + 80h" (180h-480h)

In DS301V4.02 there is no 18xxh Sub 6 "SYNC start value" any more. TPDO and RPDO Communication Parameter have a value range from 2-5

I can do some more tests next week

CANopenNode commented 1 year ago

I don't use eds files much, bu I agree, we have make things as correct as possible.

You reverted the changes from https://github.com/CANopenNode/CANopenEditor/issues/40 and changed the odObjectType from "ARAAY" to "ARR"...

Actually didn't. odObjectType is only a local variable, a string used only by OD.h.c files. ObjectType.ARRAY and ObjectType.RECORD are still there.

You have changed PDOmapping="optional" to PDOmapping="TPDO" in index="1001", but according to DS301 V4.02 this is "optional".

I'm never sure about that part of standard. Object 1001 is read-only, 'optional' means it can be mapped to TPDO or RPDO. From CANopen device side this object may be mapped only to TPDO, because it is read-only. PDOmapping="TPDO" is inside DS301_profile.xpd file, but generated eds file seems to be correct: AccessType=ro and PDOMapping=1. As I wrote before, in V4 'optional' keyword is not used, only options for PDO mapping are: t, r, tr, no.

The new default 0xC000 for PDO is correct according to DS301 from PDO number 5, for 1-4 it is for RPDOs "Node-ID + PDOno. 100h + 100h" (200h...500h) or for TPDOs "Node-ID + PDOno. 100h + 80h" (180h-480h)

I will add that.

In DS301V4.02 there is no 18xxh Sub 6 "SYNC start value" any more. TPDO and RPDO Communication Parameter have a value range from 2-5

It is in CiA 301 version 4.2.0: Bit I think, we found a bug in the standard:

301

trojanobelix commented 1 year ago

Ah okay, so the sub 6 seems to be the new fashioned stuff ;-) So they just forgot to change the value range.

After changing the COBID for the first four PDO everything is fine I think.

I got some rare exeptions when adding new PDO items in the RX/TX tab. I will dig into it, if I find the time next week

CANopenNode commented 1 year ago

After changing the COBID for the first four PDO ...

Here: https://github.com/CANopenNode/CANopenEditor/commit/ff637a74d607c92164203ad354f75b1da3ee5464

trojanobelix commented 1 year ago

I use EDS very often. If I have implemented CANopenNode in a device, I usually make the information available as EDS, because most network tools can also read in an EDS to integrate a device.

Therefore, the export should be standard-compliant. I use the EDS Checker from Vector for checking. It can also read XDD, but not those of the CANopenEditor. Unfortunately.

I'll run the checker over some export files next week.

CANopenNode commented 1 year ago

I try to follow the standards quite seriously, but it is not so simple.

Standardization

eds files was the first attempt for device description and as I know, they are most widely used. Then they made xml standard (xdd files), where they tried to follow higher level standards and standards of other communication protocols than CANopen (as I know). xml is more modern than ".ini" type of eds files, but xml seems to be very complex and nobody use it seriously (again, as I know). Besides that, xml has two versions 1.0 and 1.1, I guess you tool only supports v1.0. After xml standards there are also new standards for eds files...

CANopenNode

When I began with new approach to Object Dictionary and started changing the CANopenEditor, I decided to add xml standard 1.1 into it and make its format for default project file. Xml standard allows to include custom properties from CANopenNode inside. I verified generated xdd files against CANopen XML Schema v1.1 and it comply 100.0%. For more information see https://github.com/CANopenNode/CANopenNode/blob/master/doc/objectDictionary.md#xml-device-description-xml-device-description

Testing eds files

Tests were made with CANopen Conformance Test Tool, which includes also eds checker. There were warnings like warning 4: Access type in section [2110sub1] has no clear mapping direction. For the object:

[2110sub1]
ParameterName=int32
ObjectType=0x7
;StorageLocation=RAM
DataType=0x0004
AccessType=rw
DefaultValue=0
PDOMapping=1

I want object to be mappable to both TPDO and RPDO. I see no other solution than to ignore that warning. And I found no other serious warnings. But as I said, also didn't pay much attention on eds files.

trojanobelix commented 1 year ago

EDS seem to be ok. You are right, CANeds can only handle XDD V1.0 and this exporter has bugs:

  1. Exporter uses data type for all subs, even for sub 0 if object type is Array or Record.
  2. Open an exported xdd V1.0 gives an exception because Subindex are x4 formated, not x2.

Example: CANopenSubObject edseditor_extension_notifyonchange="false" subIndex="0000" name="Number of errors" objectType="7" dataType="0007" lowLimit="" highLimit="" accessType="rw" actualValue="" denotation="" PDOmapping="no" uniqueIDRef="UID_PARAM_100300"

Generally I think the current state is sufficiently workable. I think we should first close all PRs and most issues and see if new issues arise.

CANopenNode commented 1 year ago

I already closed most PRs, but I'm not sure for two.