[Suggestion] SDO/PDO interface

[npk48]

Hi, currently I'm using CANopenNode for a motion control project. In this project we have been using freemodbus to support modbus communication already, we want to add canopen support for it. While integrating CANopenNode and our custom ds402 implementation, I found that the SDO interface is designed in a very complex way.

In our application we have lots of complex data structures that need to be mapped to CanOpen OD, for example servo loop.

For freemodbus, it uses a lookup function to handle it.

eMBErrorCode eMBRegInputCB(UCHAR * pucRegBuffer, USHORT usAddress, USHORT usNRegs )

So we wrote our own code generation tool for this lookup function. But for CanOpenNode we can't. The only solution I found now is

void CO_OD_configure(
        CO_SDO_t               *SDO,
        uint16_t                index,
        CO_SDO_abortCode_t    (*pODFunc)(CO_ODF_arg_t *ODF_arg),
        void                   *object,
        uint8_t                *flags,
        uint8_t                 flagsSize)

and it seems work.

But the problem is here, in CO_PDO.c

int16_t CO_TPDOsend(CO_TPDO_t *TPDO){
    int16_t i;
    uint8_t* pPDOdataByte;
    uint8_t** ppODdataByte;

#ifdef TPDO_CALLS_EXTENSION
    if(TPDO->SDO->ODExtensions){
        /* for each mapped OD, check mapping to see if an OD extension is available, and call it if it is */
        const uint32_t* pMap = &TPDO->TPDOMapPar->mappedObject1;
        CO_SDO_t *pSDO = TPDO->SDO;

        for(i=TPDO->TPDOMapPar->numberOfMappedObjects; i>0; i--){
            uint32_t map = *(pMap++);
            uint16_t index = (uint16_t)(map>>16);
            uint8_t subIndex = (uint8_t)(map>>8);
            uint16_t entryNo = CO_OD_find(pSDO, index);
            if ( entryNo == 0xFFFF ) continue;
            CO_OD_extension_t *ext = &pSDO->ODExtensions[entryNo];
            if( ext->pODFunc == NULL) continue;
            CO_ODF_arg_t ODF_arg;
            memset((void*)&ODF_arg, 0, sizeof(CO_ODF_arg_t));
            ODF_arg.reading = true;
            ODF_arg.index = index;
            ODF_arg.subIndex = subIndex;
            ODF_arg.object = ext->object;
            ODF_arg.attribute = CO_OD_getAttribute(pSDO, entryNo, subIndex);
            ODF_arg.pFlags = CO_OD_getFlagsPointer(pSDO, entryNo, subIndex);
            ODF_arg.data = CO_OD_getDataPointer(pSDO, entryNo, subIndex); //https://github.com/CANopenNode/CANopenNode/issues/100
            ODF_arg.dataLength = CO_OD_getLength(pSDO, entryNo, subIndex);
            ext->pODFunc(&ODF_arg);
        }
    }
#endif
    i = TPDO->dataLength;
    pPDOdataByte = &TPDO->CANtxBuff->data[0];
    ppODdataByte = &TPDO->mapPointer[0];

    /* Copy data from Object dictionary. */
    for(; i>0; i--) {
        *(pPDOdataByte++) = **(ppODdataByte++);
    }

    TPDO->sendRequest = 0;

    return CO_CANsend(TPDO->CANdevTx, TPDO->CANtxBuff);
}

it's calling CO_OD_find in pdo handling function. and this look up might take some time to complete. So for example if I use tpdo for ds402 cyclic sync position update, CO_OD_find will be the bottleneck of the performance.

In my opinion, it's better handled with something like this

typedef struct
{
  // ...
  uint8_t type;
  void* pdata;
  // ...
} sdo_entry_t;

sdo_entry_t object_dict = {

// code generation

}

sdo_entry_t* sdo_find_entry(uint16_t index, uint8_t sub_index)
{
// code generation
}

typedef struct
{
  // cob-id
  uint16_t cob_id;
  // byte 0
  uint8_t* byte0;
  // byte 1
  uint8_t* byte1;
  // ...
} pdo_entry_t;

pdo_entry_t tpdo = {
// code generation
}

int16_t tpdo_send(...) {
// ...
 uint8_t pdo_send_data[8];
 pdo_send_data[0] = *tpdo[0].byte0;
 ...
//...
}

In this way, for OD and PDO mapping I could set the pointer to my application data directly. It allows easy integration with existing projects.

For now, I'm not sure how much it will affect the performance and will not change it, but I do have plan to rewrite the OD part of CanOpenNode. Just wondering if anyone have the same idea here.

[CANopenNode]

Just wondering if anyone have the same idea here.

Oh yes, I have the same idea for a very long time. Object dictionary is really outdated part. The good new is, I have some more time now and I really want to improve the Object Dictionary. I have some ideas. Implementation should be flexible: simple for simple objects and rich for sophisticated. It will be quite a difficult task.

I will start in next days. I will make a separate branch for new_OD and put some ideas on it. I hope, it will be finished in next two months.

[martinwag]

The callback stuff already has a known issue: https://github.com/CANopenNode/CANopenNode/issues/100 where I had no idea how to fix properly.

[CANopenNode]

Object Dictionary is the oldest part of CANopenNode. It's time to renew it.

The idea is:

• Make SDO API more clean and independent (CO_OD_find(), CO_OD_getLength(), etc.)
• Object Dictionary will be a linked list of different objects.
• Simplest object will contain simple read-only variable.
• Advanced object will contain SDO callbacks, (separate) PDO callbacks, etc.

I will start with separate branch.