incorrect canopennode SDO CRC

[iperry]

I am testing block SDO transfers from a master using the Lely Industries CANOpen stack to a slave running the Zephyr canopennode stack.

When attempting to use block SDO uploads (reading a test chunk from the slave), CRC errors are encountered. I narrowed the problem down to the CRC check used inside the canopen stack, and its interaction with the zephyr CRC. I've only tested SDO block upload, but I suspect this problem exists everywhere crc16_ccitt() is used inside CO.

1. The Zephyr CO stack is configured to use Zephyr internal CRC16 implementation (CO_driver_target.h)

   /* Use Zephyr provided crc16 implementation */
   #define CO_USE_OWN_CRC16

2. The CANopen CRC implementation and the Zephyr implementation differ in call signatures. The CO stack uses the following signature (modules/lib/canopennode/stack/crc16-ccitt.h):

   #ifdef CO_USE_OWN_CRC16
   extern
   #endif
   unsigned short crc16_ccitt(
       const unsigned char     block[],
       unsigned int            blockLength,
       unsigned short          crc);

   But, the zephyr signature is (zephyr/include/sys/crc.h)

   uint16_t crc16_ccitt(uint16_t seed, const uint8_t *src, size_t len);

3. The zephyr version of crc16_ccitt() is not the same version as specified by CIA 301. The Zephyr version of the CRC as specified in CIA 301 is named crc16_itu_t in crc.h. I tested this by looking at the test message with data "123456789,", which should yield the checksum 0x31C3 (see 7.2.4.3.16 CRC calculation algorithm to verify SDO block transfer).

I applied the following patch and it "works for me:"

index 9d3344f..aebf442 100644
--- a/stack/crc16-ccitt.h
+++ b/stack/crc16-ccitt.h
@@ -56,12 +56,15 @@ extern "C" {
  * @return Calculated CRC.
  */
 #ifdef CO_USE_OWN_CRC16
-extern
-#endif
+extern uint16_t crc16_itu_t(uint16_t seed, const uint8_t *src, size_t len);
+#define crc16_ccitt(block, blockLength, crc) \
+  crc16_itu_t(crc, block, blockLength)
+#else
 unsigned short crc16_ccitt(
         const unsigned char     block[],
         unsigned int            blockLength,
         unsigned short          crc);
+#endif

 #ifdef __cplusplus
 }

candump of an SDO block request WITHOUT the patch above:

  can0  TX - -  602   [8]  A4 00 20 00 7F 00 00 00   '.. .....'
  can0  RX - -  582   [8]  C6 00 20 00 09 00 00 00   '.. .....'
  can0  TX - -  602   [8]  A3 00 00 00 00 00 00 00   '........'
  can0  RX - -  582   [8]  01 31 32 33 34 35 36 37   '.1234567'
  can0  RX - -  582   [8]  82 38 39 00 00 00 00 00   '.89.....'
  can0  TX - -  602   [8]  A2 02 7F 00 00 00 00 00   '........'
  can0  RX - -  582   [8]  D5 4C 1B 00 00 00 00 00   '.L......'

candump of an SDO request WITH the patch above:

  can0  TX - -  602   [8]  A4 00 20 00 7F 00 00 00   '.. .....'
  can0  RX - -  582   [8]  C6 00 20 00 09 00 00 00   '.. .....'
  can0  TX - -  602   [8]  A3 00 00 00 00 00 00 00   '........'
  can0  RX - -  582   [8]  01 31 32 33 34 35 36 37   '.1234567'
  can0  RX - -  582   [8]  82 38 39 00 00 00 00 00   '.89.....'
  can0  TX - -  602   [8]  A2 02 7F 00 00 00 00 00   '........'
  can0  RX - -  582   [8]  D5 C3 31 00 00 00 00 00   '..1.....'
  can0  TX - -  602   [8]  A1 00 00 00 00 00 00 00   '........'

Please also mention any information which could help others to understand the problem you're facing:

• Zephyr v3.2.0
• See patch above for workaround

To Reproduce Steps to reproduce the behavior:

1. Run canopennode example
2. Send SDO block read
3. Check CRC

Expected behavior A clear and concise description of what you expected to happen.

Impact What impact does this issue have on your progress (e.g., annoyance, showstopper)

Logs and console output If applicable, add console logs or other types of debug information e.g Wireshark capture or Logic analyzer capture (upload in zip archive). copy-and-paste text and put a code fence (```) before and after, to help explain the issue. (if unable to obtain text log, add a screenshot)

Environment (please complete the following information):

• OS: Arch Linux
• Toolchain: Zephyr SDK 0.15.1
• v3.2.0 sdo_test.tar.gz

[henrikbrixandersen]

@iperry Thank you. Please test https://github.com/zephyrproject-rtos/zephyr/pull/53620 and report back.

[iparobin]

I am testing the sending of a large number of data and I wanted to know how to select in the config the choice between segmented date or block transfer ?