Skip to content
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

reentering an abandoned segmented SDO session #132

Open
ltskylabs opened this issue Jan 16, 2023 · 0 comments
Open

reentering an abandoned segmented SDO session #132

ltskylabs opened this issue Jan 16, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@ltskylabs
Copy link

I encountered the following situation:

  1. SDO client starts a segmented upload transfer for Index1, Subindex1
  2. SDO server starts sending segments
  3. Client restarts
  4. Client starts an expedited Upload for Index2, Subindex2
  5. SDO server returns OD entry data at Index1, Subindex1

expected behavior:

  • SDO server should send an SDO Abort

The problem can be spotted in following code snippets from canopen-stack source code.

void CONodeProcess(CO_NODE *node)
 ...
    if ((allowed & CO_SDO_ALLOWED) != (uint8_t)0) {
        srv = COSdoCheck(node->Sdo, &frm);
        if (srv != NULL) {
            err = COSdoResponse(srv);
            if ((err == CO_ERR_NONE     ) ||
                (err == CO_ERR_SDO_ABORT)) {
                (void)COIfCanSend(&node->If, &frm);
            }
            allowed = 0;

In step 2 of the scenario above the Initiate segmented SDO properly initializes session in COSdoCheck by setting srv[n].Idx and srv[n].Sub to Index1 and Subindex1

CO_SDO *COSdoCheck(CO_SDO *srv, CO_IF_FRM *frm)
{
    CO_SDO  *result = 0;
    uint8_t  n;

    if (frm != 0) {
        n = 0;
        while ((n < CO_SSDO_N) && (result == 0)) {
            if (CO_GET_ID(frm) == srv[n].RxId) {
                CO_SET_ID(frm, srv[n].TxId);
                srv[n].Frm   = frm;
                srv[n].Abort = 0;
                if (srv[n].Obj == 0) {
                    srv[n].Idx = CO_GET_WORD(frm, 1);
                    srv[n].Sub = CO_GET_BYTE(frm, 3);

by setting srv[n].Idx and srv[n].Sub to Index1 and Subindex1 because there is no previous session present (srv[n].Obj == 0). However in step 5 srv[n].Idx and srv[n].Sub are not set to Index2 and Subindex2 because a previous session had not been closed (hence srv[n].Obj != 0). When subsequently COSdoResponse is called ( in 1st code snippet)

CO_ERR COSdoResponse(CO_SDO *srv)
 ...
   } else if (cmd == 0x40) {
        result = COSdoGetObject(srv, CO_SDO_RD);
        if (result == 0) {
            result = COSdoUploadExpedited(srv);

COSdoGetObject returns the same OD entry (at Index1, Subindex1) as in step 2 resulting in wrong data being sent by COSdoUploadExpedited.

Note that other combinations of segmented/expedited upload/download transfers trigger similar undesired behavior.

@michael-hillmann michael-hillmann added the bug Something isn't working label Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants