TMS320F28P650DK: In the Endat routine of CLB, the position CRC of the 31-bit Endat encoder is always incorrect, while the position CRC calculated by the 25-bit Endat encoder is correct. Why is this?

Hi Lin - unfortunately I don't have an answer for this. It seems there must be a bug in one of the software routines for 31-bit resolution.

Hello Lori, I'm very grateful that you replied. Could you please help me check and solve the bug in the CRC calculation for 31-bit resolution? 

I am not sure if there will be issues at other resolutions, but supporting 31-bit resolution is crucial for us.

I will take a look. Give me a few days. 

Lin,

I asked Claude Code to analyze the function and why it does not work for 31-bit. Here is the analysis and suggested fix. Keep in mind this is AI generated, but I am hopeful that it will help.  

The problem occurs specifically for 31-bit data, but works for 25-bit data. Let's focus on the relevant section of the PM_endat22_getCrcPos function:

  if(endat22)

  {

      lowpos_temp = __flip32(lowpos);

      if (total_clocks+2 > 32)

      {

          highpos_temp = ((__flip32(highpos)) >> (64 - (total_clocks+2))) & (( 1 << (total_clocks+2) -32) - 1);

          testDataArr[1] = lowpos_temp >> (64 - (total_clocks+2));

          testDataArr[0] = (lowpos_temp << (((total_clocks+2)-32))) | highpos_temp;

      }

      else

      {

          testDataArr[0] = (lowpos_temp) >> (32 - (total_clocks+2));

      }

  }

  else

  {

      lowpos_temp = __flip32(lowpos);

      if (total_clocks+1 > 32)

      {

          highpos_temp = ((__flip32(highpos)) >> (64 - (total_clocks+1))) & (( 1 << (total_clocks+1) -32) - 1);

          testDataArr[1] = lowpos_temp >> (64 - (total_clocks+1));

          testDataArr[0] = lowpos_temp << ((total_clocks+1)-32) | highpos_temp;

      }

      else

      {

          testDataArr[0] = (lowpos_temp) >> (32 - (total_clocks+1));

      }

  }

  I've identified several potential issues:

1. When total_clocks is 31 (for 31-bit data) and endat22 is not set, we get total_clocks+1 = 32, which would cause a division by zero in testDataArr[0] = (lowpos_temp) >> (32 -

  (total_clocks+1)); as (32 - 32 = 0).

2. There's a mathematical issue in the expression lowpos_temp >> (64 - (total_clocks+1)) when total_clocks = 31, as the shift amount would be (64 - 32) = 32, which is the width

  of the operand. In C, shifting by the width of the operand is undefined behavior.

3. The bit masking calculation could be problematic: (( 1 << (total_clocks+1) -32) - 1). When total_clocks = 31, we have 1 << (31+1-32) which equals 1 << 0, which equals 1, and

  then we subtract 1 resulting in 0. This would effectively mask out all bits.

• Now I'll create a fix for the CRC calculation for 31-bit data. Based on the identified issues, here's my proposed solution:

  uint32_t PM_endat22_getCrcPos(uint32_t total_clocks, uint32_t endat22, uint32_t lowpos, uint32_t highpos, uint32_t error1, uint32_t error2, uint16_t *crc_table)
  {
      uint16_t accumulator, nBytes;
      uint32_t testDataArr[2] = {0}; // Initialize array to zero
      uint32_t lowpos_temp, highpos_temp;

      lowpos_temp = lowpos;
      if(endat22)
      {
          lowpos = (lowpos << 2) | (error2 << 1) | error1;
          highpos = (highpos << 2) | (lowpos_temp >> 30);
      }
      else
      {
          lowpos = (lowpos << 1) | error1;
          highpos = (highpos << 1) | (lowpos_temp >> 31);
      }

      if(endat22)
      {
          lowpos_temp = __flip32(lowpos);
          if (total_clocks+2 > 32)
          {
              // Handle the special case for 31-bit data (resulting in 33-bit with endat22)
              if (total_clocks == 31) {
                  highpos_temp = __flip32(highpos) & 0x1; // Only get the lowest bit
                  testDataArr[1] = lowpos_temp >> 31;     // Right shift by 31 bits to get the upper bit
                  testDataArr[0] = lowpos_temp << 1 | highpos_temp; // Left shift by 1 and OR with highpos
              } else {
                  highpos_temp = ((__flip32(highpos)) >> (64 - (total_clocks+2))) & (( 1 << ((total_clocks+2) -32)) - 1);
                  testDataArr[1] = lowpos_temp >> (64 - (total_clocks+2));
                  testDataArr[0] = (lowpos_temp << ((total_clocks+2)-32)) | highpos_temp;
              }
          }
          else
          {
              testDataArr[0] = (lowpos_temp) >> (32 - (total_clocks+2));
          }
      }
      else
      {
          lowpos_temp = __flip32(lowpos);
          if (total_clocks+1 > 32)
          {
              // Handle the special case for 31-bit data
              if (total_clocks == 31) {
                  testDataArr[0] = lowpos_temp; // Use the full lowpos value
                  testDataArr[1] = 0;           // highpos will not be used
              } else {
                  highpos_temp = ((__flip32(highpos)) >> (64 - (total_clocks+1))) & (( 1 << ((total_clocks+1) -32)) - 1);
                  testDataArr[1] = lowpos_temp >> (64 - (total_clocks+1));
                  testDataArr[0] = lowpos_temp << ((total_clocks+1)-32) | highpos_temp;
              }
          }
          else
          {
              // Add special case for exactly 31-bit data (total_clocks == 31)
              if (total_clocks == 31) {
                  testDataArr[0] = lowpos_temp >> 1; // Shift right by 1 bit
              } else {
                  testDataArr[0] = (lowpos_temp) >> (32 - (total_clocks+1));
              }
          }
      }

  The key changes I've made are:

1. Added special case handling for total_clocks == 31 in both the endat22 and non-endat22 sections.
2. Fixed the potential division by zero and undefined shift behavior.
3. Correctly handled bit masking for the 31-bit case.
4. Initialized the testDataArr to zero to ensure no random data affects the calculation.
5. Added parentheses in the expression 1 << ((total_clocks+1) -32) to ensure the correct precedence.

Hello Lori,

I tried this program and found that it didn't solve the problem. I rechecked the original program and found that the bit shifting and value extraction in this part of the original program were correct. The problem should not be here.

I will need a week or so before I can take a look. I'll try to pull that in sooner. 

Hi Lin,

I do not have access to a 31-bit encoder. To help speed this up, can you send me a couple of examples of the command, the raw data, the extracted data, and the golden CRC from the encoder? Having some example data will help in figuring out where things are going wrong.

You mentioned the raw data and the extracted data is correct? Only the calculated CRC is incorrect - but the new CRC function did not help.  Did I understand correctly?

Thank you

Lori

Hi Lori,

You're right. It's exactly this situation.
I tried to filter out the CRC errors, manually rotated the 31-bit resolution Endat encoder for five turns, and used our upper computer tool to check the position changes and the number of turns calculated by the program during the rotation process. They matched up, indicating that the DSP program has no problem extracting the raw data.

At present, I have only used the EEPROM-related commands and the ENCODER_SEND_POSITION_VALUES command. Below are several sets of data I randomly grabbed. I hope they can be of help.And please forgive me for forgetting to capture the data returned by the original encoder.

SELECTION_OF_MEMORY_AREA

PM_endat22_setupCommand(SELECTION_OF_MEMORY_AREA, 0x00B9, 0x5555, 0); 

//data1=MRS code; data2=any

rdata[0] = 32768   (0x8000)

rdata[1] = 0

rdata[2] = 11861   (0x2E55)

rdata[3] = 21833   (0x5549)

rdata[4] = 26116   (0x6636)

(encoder) data_crc : 4

(DSP) crc_result : 4

ENCODER_SEND_PARAMETER

PM_endat22_setupCommand(ENCODER_SEND_PARAMETER, 0x00, 0xAAAA, 0);

//data1=Address; data2=any

rdata[0] = 32768   (0x8000)

rdata[1] = 0

rdata[2] = 0

rdata[3] = 49 (0x0031)

(encoder) data_crc : 24 (0x18)

(DSP) crc_result : 24      (0x18)

ENCODER_SEND_POSITION_VALUES    1

PM_endat22_setupCommand(ENCODER_SEND_POSITION_VALUES, 0, 0, 0);

rdata[0] = 32796   (0x801C)

rdata[1] = 18990   (0x4A2E)

rdata[2] = 11719   (0x2DC7)

rdata[3] = 21 (0x15)

(encoder) data_crc : 3   (0x03)

(DSP) crc_result : 21      (0x15)

ENCODER_SEND_POSITION_VALUES    2

PM_endat22_setupCommand(ENCODER_SEND_POSITION_VALUES, 0, 0, 0);

rdata[0] = 32788   (0x8014)

rdata[1] = 49998   (0xC34E)

rdata[2] = 52729   (0xCDF9)

rdata[3] = 21 (0x15)

(encoder) data_crc : 28 (0x1C)

(DSP) crc_result : 10      (0x0A)

ENCODER_SEND_POSITION_VALUES    3

PM_endat22_setupCommand(ENCODER_SEND_POSITION_VALUES, 0, 0, 0);

rdata[0] = 32771   (0x8003)

rdata[1] = 45260   (0xB0CC)

rdata[2] = 19927   (0x4DD7)

rdata[3] = 21 (0x15)

(encoder) data_crc : 11 (0x0B)

(DSP) crc_result : 29      (0x1D)

ENCODER_SEND_POSITION_VALUES    4

PM_endat22_setupCommand(ENCODER_SEND_POSITION_VALUES, 0, 0, 0);

rdata[0] = 32780   (0x800C)

rdata[1] = 9593     (0x2579)

rdata[2] = 36289   (0x8DC1)

rdata[3] = 21 (0x15)

(encoder) data_crc : 0

(DSP) crc_result : 22      (0x16)

ENCODER_SEND_POSITION_VALUES    5

PM_endat22_setupCommand(ENCODER_SEND_POSITION_VALUES, 0, 0, 0);

rdata[0] = 32780   (0x800C)

rdata[1] = 37344   (0x91E0)

rdata[2] = 36307   (0x8DD3)

rdata[3] = 21 (0x15)

(encoder) data_crc : 9   (0x09)

(DSP) crc_result : 31      (0x1F)

Lin - Thank you for the data - give me a few days to look into it. 

Thank you, Lori, for helping review the program.