HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Process bytes and check for start key

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
processforbytesstartandcheckkey

Problem

I am working with some wireless communication in Arduino. My data will be wirelessly received like so:

97
97
97
...
...


979797 is the address of the transmitter. I have written the following code to check the address first before processing the data. My main concern is speed of computation.

void printBuffer(const uint8_t *serialBuffer)
{
  //print contents of buffer
  for (int i = 0; i<100; i++) 
    Serial.println(serialBuffer[i]); //remove the print in binary option to decode
}

void processBytes(const uint8_t value)
{ 
  static uint8_t serialBuffer[bufferSize]; //buffer to hold values
  static unsigned int startPosition=0;

  if(startPosition < bufferSize)
  {
    serialBuffer[startPosition] = value; 
    startPosition++;
  }

  else
  {
    printBuffer(serialBuffer);
    startPosition = 0;
  }

}//end of processBytes
void loop() { 

  //check if data is available in serial buffer
  while(RX.available())
  {
    //check for start key
    address(RX.read())
    //process data
    processBytes(RX.read());
  }   
}
void address(const uint8_t value)
{
  int counter = 0;
  int newVal = value;
  while(value == 97 && counter < 3)
  {
    counter++;
    newVal = RX.read();
  }
}


EDIT:
The data packet just has the following components: start key (979797) and 68 bytes of data. There is no termination key. Also, the transmitter transmits one byte at a time. Thus, each value received is less than 256. Hence why I am saving the data into a uint8_t[] array.

Other than the address() method, most of it was borrowed from Gammon. Here is the reference page for the arduino commands I am using. Are there ways I can optimize this computation?

Solution

Bugs

There are several things wrong with this function:

void address(consst uint8_t value)
{
  int counter = 0;
  int newVal = value;
  while(value == 97 && counter < 3)
  {
    counter++;
    newVal = RX.read();
  }
}


-
You never use the variable newVal, which is a mistake. Most likely you meant the while loop to read:

while (newVal == 97 && counter < 3)


As it stands, your code will always read 3 extra bytes if the first is 97, even if the second byte is not 97. You can also get rid of the newVal variable and just use value, if you change the type of value to make it non-const.

-
const is misspelled.

  • When you encounter a series of three 97 bytes, you actually read 4 bytes. The first byte is already read and passed into this function. The function then reads 3 extra bytes. This means that the first byte of the "message" will be missing when you call processBytes() later on.



  • Your message protocol is unclear. From your code, it appears that the message protocol must be 97 97 97 byte 97 97 97 byte which is extremely wasteful. If that is not the protocol, there are problems with how you handle the message stream. For example, the address() function strips all 97s out of the message stream, whether or not they are part of a sequence of three. Also, if the address() function does not detect a sequence of three 97s, it always discards one byte out of the message stream. I'd need to see an example of a proper message stream in order to advise on a solution.

Code Snippets

void address(consst uint8_t value)
{
  int counter = 0;
  int newVal = value;
  while(value == 97 && counter < 3)
  {
    counter++;
    newVal = RX.read();
  }
}
while (newVal == 97 && counter < 3)

Context

StackExchange Code Review Q#135233, answer score: 2

Revisions (0)

No revisions yet.