rtlabs-com / c-open

CANopen stack for embedded devices
http://www.rt-labs.com
Other
79 stars 40 forks source link

Unable to override default TPDO CAN-ID #51

Closed nattgris closed 2 years ago

nattgris commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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