Enhancing parser for 8 byte command string (expect problems with type conversions) for Arduino/Radio communication

Question

Because the throughput of my radio and device is limited, I wanted to compress my control string. I decided to compress the command into an 8 bytes (quasi) string (throttle high, throttle low, pitch, roll, yaw plus/minus, yaw value, checksum, stop byte) and after parsing in four variables:

• Pitch (between -45° and 45°)
• Roll (between -45° and 45°)
• Yaw (between -180° and 180°)
• Throttle (between 1000 and ~1900)

Because Some number ranges don't fit into a single byte (0 to 255 number range), I used two instead (throttle: one byte for hundreds (1-9) and one byte for tenners (1-99); Yaw: One byte for plus or minus and one byte for 0° - 180°). I send and read these bytes as characters. Then I convert them into numbers (0-255).

I noticed that sometimes a few reads are totally out of range (e.g. value: -31001) but checksum test is OK. I don't know exactly why (maybe radio, maybe I caused unexpected behavior because of type conversions here). This is why I made an additional range test. Now the remote control seems to be reliable.

However I don't feel very nice about it, because I do signed/unsigned and! number range conversions, the checksum function is also not very reliable. Maybe someone has suggestions how to make the code better.

Code (The command stored in buffer is already red correctly until the stop byte and valid in length):

bool Receiver::parse_radio(char *buffer) {
  int16_t throttle_high  = (uint8_t)buffer[0];       // 1. 1 - 9
  int16_t throttle_low   = (uint8_t)buffer[1];       // 2. 0 - 99
  int16_t pit            = (uint8_t)buffer[2] - 127; // 3. -45° - 45°
  int16_t rol            = (uint8_t)buffer[3] - 127; // 4. -45° - 45°
  int16_t yaw_pm         = (uint8_t)buffer[4] - 127; // 5. -1 || +1
  int16_t yaw_val        = (uint8_t)buffer[5];       // 6. yaw angle in degrees 180°

  int16_t chk            = (uint8_t)buffer[6];       // 7. checksum

  int16_t thr            = 1000 + (throttle_high * 100) + throttle_low;
  int16_t yaw            = yaw_val * yaw_pm;

  // Calculate checksum
  uint8_t checksum = 0;
  for(int i = 0; i < 6; i++) {
    checksum = (checksum + buffer[i]) << 1; // % 256 can be saved because of the uint8_t
  }

  // Validity check: 
  // First checksum
  if(checksum != chk)
    return false;
  // Later other values
  if(yaw_pm < -1 || yaw_pm > 1)
    return false;
  if(rol > 45 || rol < -45)
    return false;
  if(pit > 45 || pit < -45)
    return false;
  if(yaw > 180 || yaw < -180)
    return false;
  if(thr > RC_THR_80P || thr < RC_THR_MIN)
    return false;

  // Do rest here
  return true;
}

Answer 1

I don't know exactly why (maybe radio, maybe I caused unexpected behavior because of type conversions here).

Can you keep statistics on how many checksum errors there are?

• If there are many that implies that the communications link is noisy
• If there are few or none then it's more probably something else (e.g. a software error)

I'm not sure that you're using a good checksum algorithm.; you might like to use a CRC instead.

You're only using 7 of your 8 bytes so you could have a 2-byte CRC (or checksum).

You could compress your data better:

1. '1 - 9'
2. '0 - 99' => 10 bits to store 0..999 throttle
3. '-45° - 45°' => 7 bits to store 90 angle values
4. '-45° - 45°' => 7 bits to store 90 angle values
5. '-1 || +1'
6. 'yaw angle in degrees 180°' => 9 bits to store 360 angle values

10+7+7+9 => 37 bits = 5 bytes.

Alternatively 999 * 90 * 90 * 360 = 2913084000 which is less than 2³². So you could fit your data into 4 bytes and use a 4-byte CRC.

Answer 2

I suggest you cast you buffer elements to int16_t not uint8_t.

Here is an example

#include <stdio.h>
#include <inttypes.h>

int main()
{
    char buffer = -45;
    printf("Original = %d\n", buffer);
    printf("uint8_t  = %d\n", (uint8_t)buffer);
    printf("int16_t  = %d\n", (int16_t)buffer);

    int16_t a = (uint8_t)buffer - 127;
    int16_t b = (int16_t)buffer - 127;
    printf("a = %d\n", a);
    printf("b = %d\n", b);
}

Output:

Original = -45
uint8_t  = 211
int16_t  = -45
a = 84
b = -172

As you can see in the second line, when you change a negative integer to an unsigned integer it cannot retain its negative status and instead becomes a large positive number.

Edit: Sorry I didn't realize the date on this question. I guess I'll just leave it.