Skip to content

Unable to override default TPDO CAN-ID #51

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

Closed
nattgris opened this issue Jan 2, 2022 · 4 comments · Fixed by #52
Closed

Unable to override default TPDO CAN-ID #51

nattgris opened this issue Jan 2, 2022 · 4 comments · Fixed by #52

Comments

@nattgris
Copy link

nattgris commented Jan 2, 2022

An updated CAN-ID for a PDO that is setup in the defaults array does not stick even if the communication parameters are stored. This can be demonstrated using the example slave, with the addition of a long delay before exiting main, and the following test script:

# Start the node to see the TPDO, then reset again
cansend vcan0 000#0101
sleep 1
cansend vcan0 000#8101
sleep 1

# Reconfigure the TPDO with CAN-ID 0x456 and duplicate the mapping
sdowrite vcan0 1 0x1800 1 4 0x80000456
sdowrite vcan0 1 0x1A00 0 1 0
sdowrite vcan0 1 0x1A00 1 4 0x20010010
sdowrite vcan0 1 0x1A00 2 4 0x20010010
sdowrite vcan0 1 0x1A00 0 1 2
sdowrite vcan0 1 0x1800 1 4 0x00000456

# Store the communication parameters
sdowrite vcan0 1 0x1010 2 4 0x65766173

# Start the node to see the TPDO with updated CAN-ID and mapping
cansend vcan0 000#0101
sleep 1

# Reset the node, then start again and expect the PDO to be identical
cansend vcan0 000#8101
sleep 1
cansend vcan0 000#0101

candump shows the following output:

  vcan0  701   [1]  00
  vcan0  081   [8]  00 10 81 00 00 00 00 00
  vcan0  081   [8]  00 00 00 00 00 00 00 00
  vcan0  000   [2]  01 01
  vcan0  181   [2]  AA 55
  vcan0  000   [2]  81 01
  vcan0  701   [1]  00
  vcan0  601   [8]  23 00 18 01 56 04 00 80
  vcan0  581   [8]  60 00 18 01 00 00 00 00
  vcan0  601   [8]  2F 00 1A 00 00 00 00 00
  vcan0  581   [8]  60 00 1A 00 00 00 00 00
  vcan0  601   [8]  23 00 1A 01 10 00 01 20
  vcan0  581   [8]  60 00 1A 01 00 00 00 00
  vcan0  601   [8]  23 00 1A 02 10 00 01 20
  vcan0  581   [8]  60 00 1A 02 00 00 00 00
  vcan0  601   [8]  2F 00 1A 00 02 00 00 00
  vcan0  581   [8]  60 00 1A 00 00 00 00 00
  vcan0  601   [8]  23 00 18 01 56 04 00 00
  vcan0  581   [8]  60 00 18 01 00 00 00 00
  vcan0  601   [8]  23 10 10 02 73 61 76 65
  vcan0  581   [8]  60 10 10 02 00 00 00 00
  vcan0  000   [2]  01 01
  vcan0  456   [4]  AA 55 AA 55
  vcan0  000   [2]  81 01
  vcan0  701   [1]  00
  vcan0  000   [2]  01 01
  vcan0  181   [4]  AA 55 AA 55

The last line shows that the TPDO COB-ID reverted to the default after an NMT reset. However, the updated mapping was correctly remembered.

@hefloryd
Copy link
Collaborator

hefloryd commented Jan 12, 2022

Loading an enabled COB-ID fails the check that the PDO is disabled before updating the value. This check should be disabled in INIT state, as we do for the mapping parameters. Something like this:

diff --git a/src/co_pdo.c b/src/co_pdo.c
index 8be412e..582a5ce 100644
--- a/src/co_pdo.c
+++ b/src/co_pdo.c
@@ -184,7 +184,7 @@ static uint32_t co_pdo_comm_set (
    {
       if (!co_validate_cob_id (*value))
          return CO_SDO_ABORT_VALUE;
-      if (((pdo->cobid | *value) & CO_COBID_INVALID) == 0)
+      if (((pdo->cobid | *value) & CO_COBID_INVALID) == 0 && net->state != STATE_INIT)
          return CO_SDO_ABORT_VALUE;
       pdo->cobid        = *value;
       pdo->sync_counter = 0;
@@ -197,7 +197,7 @@ static uint32_t co_pdo_comm_set (
       pdo->transmission_type = *value & 0xFF;
       break;
    case 3:
-      if ((pdo->cobid & CO_COBID_INVALID) == 0)
+      if ((pdo->cobid & CO_COBID_INVALID) == 0 && net->state != STATE_INIT)
          return CO_SDO_ABORT_VALUE;
       pdo->inhibit_time = *value & 0xFFFF;
       break;
@@ -207,7 +207,7 @@ static uint32_t co_pdo_comm_set (
    case 6:
       if (is_rx)
          return CO_SDO_ABORT_BAD_SUBINDEX;
-      if ((pdo->cobid & CO_COBID_INVALID) == 0)
+      if ((pdo->cobid & CO_COBID_INVALID) == 0 && net->state != STATE_INIT)
          return CO_SDO_ABORT_VALUE;
       pdo->sync_start = *value & 0xFF;
       break;

Although this still doesn't help with the issue in #48?

@nattgris
Copy link
Author

No this is a separate but mildly related issue, which if it would work could be used as part of a workaround for that problem. But other things would still be needed for that workaround, such as a way of triggering a store parameters locally. I have performed another trick to get around the #48 for now.

@nattgris
Copy link
Author

Also, the application I'm working on doesn't have a need for customizing PDO COB-IDs, so this report is more of a heads-up and nothing I'm blocked by.

@hefloryd
Copy link
Collaborator

Ok, the above diff should fix this problem. I'll add a testcase also.

hefloryd added a commit to hefloryd/c-open that referenced this issue Jan 13, 2022
It should not be possible to update enabled PDO:s, except if in
STATE_INIT when the dictionary is restored.

Fix rtlabs-com#51.
hefloryd added a commit that referenced this issue Jan 14, 2022
It should not be possible to update enabled PDO:s, except if in
STATE_INIT when the dictionary is restored.

Fix #51.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants