CANopenNode / CANopenEditor

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

possible bug in fix #220: Set the mapping typedefs to set the max number in the group they are in #31

Closed trojanobelix closed 1 year ago

trojanobelix commented 2 years ago

See: https://github.com/robincornelius/libedssharp/issues/220

The ranges and logical operators are suspicious: It should be


                if (kvp.Key>=0x1600 && kvp.Key<0x1800) // inspect range 0x1600...0x17FF (RPDO communication parameter)
                {
                    //change the OD entry to the largest one
                    od = maxRXmappingsOD;
                }

                if (kvp.Key >= 0x1A00 && kvp.Key < 0x1C00) // inspect range 0x1A00...0x1BFF (TPDO communication parameter)
                {
                    //set the OD entry to the largest one
                    od = maxTXmappingsOD;
                }

instead of

                if (kvp.Key > 0x1600 || kvp.Key < 0x1800)   // why OR? This takes all numbers 0...0xFFFF 
                {
                    //change the OD entry to the largest one
                    od = maxRXmappingsOD;
                }

                if (kvp.Key > 0x1A00 || kvp.Key < 0x1C00)
                {
                    //change the OD entry to the largest one
                    od = maxTXmappingsOD;
                }

And it's good to search the mappings to find the largest one, but setting the "od" variable to maxXXmappingsOD, e.g. od = maxRXmappingsOD; doesn't work because the member list creation still uses kvp.Value.subobjects, which is not the maximum number of members.

so instead using foreach (KeyValuePair<UInt16, ODentry> kvp2 in kvp.Value.subobjects)

it should be something like

foreach (KeyValuePair<UInt16, ODentry> kvp2 in od.subobjects)

Because "od" is set to the max[TX/RX]mappingsOD

trojanobelix commented 2 years ago

Still problems with problems #220

Above solution compiles without errors, but the size of mapping and communication objects from CO_OD are compared with typedefs in CO_PDO.h. The number of members is fixed there (8). The comparison takes place in canopen.c. Therefore you get a CO_ERROR_PARAMETERS error at runtime.

There must be something I missed...

/* Verify parameters from CO_OD */
if(   sizeof(OD_TPDOCommunicationParameter_t) != sizeof(CO_TPDOCommPar_t)
   || sizeof(OD_TPDOMappingParameter_t) != sizeof(CO_TPDOMapPar_t)
   || sizeof(OD_RPDOCommunicationParameter_t) != sizeof(CO_RPDOCommPar_t)
   || sizeof(OD_RPDOMappingParameter_t) != sizeof(CO_RPDOMapPar_t))
{
    return CO_ERROR_PARAMETERS;
}
CANopenNode commented 1 year ago

The number of members(mapping parameters) is must be fixed to 8 in CANopenNode <=v1.3. (This is not required for V4 and for the standard.) So in this case (CanOpenNodeExporter.cs) should be fixed to 8.

CANopenNode commented 1 year ago

The number of members(mapping parameters) is now 8(minimum) in all cases.