Arduino Code Request - Byte Stream to Array

[Chet T16]

I've been looking at this on and off for the last while but i'm admitting defeat now!

I have a byte stream coming in with 43 bytes total and some 'quiet time' in between each lot of 43 bytes, as can be seen in this pic



The first 4 bytes are always the same.

All i need is to get this into an array of the same length so i can access the relevent bytes and display the info somewhere, obviously it would need to be constantly updating.

Is the 'quiet time' (i'm sure theres probably a proper name for it) useful for determining where the stream starts or would checking for the first 4 bytes be better?

Any suggestions and offers to do it for me would be greatly appreciated

Thanks

[Psi]

I'd use the 4 bytes sequence to trigger the code that starts filling the array.

But you should check if this 4 byte sequence could ever occur in other areas of the packet?
If that could happen the you have the potential for missing packets, so you might have to check for the dead time as well.

If the 4 byte sequence starts occuring in a location other than the start and this continues to happen in all following packets then you can get problems where the code keeps trying to assemble a packet starting from halfway through.
In this situation it may never get back into sync.

What kind of data is this? The structure and type of data in the packet can make solving these types of problems easier or harder.

[Mechatrommer]

I have a byte stream coming in with 43 bytes total and some 'quiet time' in between each lot of 43 bytes, as can be seen in this pic
The first 4 bytes are always the same.
Is the 'quiet time' (i'm sure theres probably a proper name for it) useful for determining where the stream starts or would checking for the first 4 bytes be better?

if the 4 bytes (let me call header) is always at the beginning after each pause (silent time) then you can set the pause as the indicator (you can put a listening timeout in your code, it can be done either by polling or "interrupt and timer count"), but if the header is not always in the beginning (or even if it is), then probing for the 4 bytes value is more bulletproof. you can also count the total bytes read, ie after 4 bytes, there will be 39 bytes remaining right? counting this number and if it reaches the "fixed" value, you can start with another stream, or another "header finder" (my term). or header or pause or timeout probing or whatever.

"the beauty of software is you can put your intelligence into it much quicker" - mr shafri

[Psi]

Ya have to be a little careful with how you deal with detecting the dead time.
If at a later date you decide to upgrade the speed of your communication it might reduce the dead time length that you're relying on.

[Chet T16]

The output is not something i have control over so the speed won't be changing although i'm lead to believe its given a low priority so it may not be completely consistent

Should i be reading bytes, comparing to the previous byte and setting flags as i work my way through the sequence?

Whether the sequence can occur elsewhere i don't know but i'm willing to assume not for now!

[Chet T16]

Would this work??

Code: [Select]

   while(Serial.available() > 0)
   {
      byte readByte = Serial.read();
      if(readByte == 0xFF)
      {
            byte readByte = Serial.read();
            if(readByte == 0x00)
              {
                  byte readByte = Serial.read();
                  if(readByte == 0x7A)
                    {
                        byte readByte = Serial.read();
                        if(readByte == 0x01)
                          {
                            READ 39 BYTES TO ARRAY
                          }
                    }
              }
      }
   }

[Psi]

I originally thought that code wouldn't work and i wrote some example code how i might do it.
But as it turns out i was wrong in my understanding of how the Arduino serial library worked. It buffers up to 128 bytes, which i wasn't aware of. I was assuming there was no buffering.

I think the code is still valid but i've not really looked into exactly how the Arduino serial library works
I'll leave the code here since the concept of how it checks the header is still valid, but be aware the code maybe wrong or not the best approach for the Arduino. (I'm not an arduino programmer)

Code: [Select]

byte mydata[39];   // array 0-38 of bytes
byte CheckingFrameHeader = true;  // boolean, Are we checking the header (True) or saving data(False), set to true for first run.
byte Counter = 0; // counter index for serial data

void SetupSerial()
{
    Serial.begin(9600); // set baudrate 9600
}

void main()
{
   SetupSeial();  // setup serial port (occures only once when mcu starts)

   While(1) // main program loop, (executes forever)
   {

    if (Serial.available())
    {
        byte readByte = Serial.read();

        if (CheckingFrameHeader)
{
    if (Counter==0) && (readByte==0xFF)   Counter=1  else  Counter=0;
if (Counter==1) && (readByte==0x00)   Counter=2  else  Counter=0;
if (Counter==2) && (readByte==0x7A)   Counter=3  else  Counter=0;
if (Counter==3) && (readByte==0x01)
{
Counter = 4
CheckingFrameHeader = false;
}
else Counter=0;
}
else
{   
                        // A complete 4 byte header has been detected, now we need to save the next 39 bytes into the array                           
                        if ( (Counter-4) <= 38) 
{
MyData[Counter-4] = ReadByte;
Counter++;

                                if ( (Counter-4) == 38)
{
                                        Counter = 0;
                                  CheckingFrameHeader = true;
}
}
else
{
Counter = 0;
CheckingFrameHeader = true;
}
}
}
   }
}


[Mechatrommer]

so you are expecting FF007A01 as the header... what if the incoming data got extra "valid byte" in the beginning such as FFFF007A01... you will lose the whole header and hence the subsequent data. i like to make the code "corrupted data" proof, so i like to think this way. a quick fix for this particular case will be...

Code: [Select]

while(Serial.available() > 0) {
    byte readByte = Serial.read();

    head1:
    if(readByte == 0xFF) {
        byte readByte = Serial.read();
        if(readByte == 0x00) {
            byte readByte = Serial.read();
            if(readByte == 0x7A) {
                byte readByte = Serial.read();
                if(readByte == 0x01) {
                    READ 39 BYTES TO ARRAY
                } else { goto head1; }
            } else { goto head1; }
        } else { goto head1; }
    }
}
another exploit... what if you're receiving only 2 or 3 bytes instead of 4 bytes minimum? (header)...  i think Psi already gave you a hint on that. my way i think will be adding line or another function to do "do until (Serial.available() > 0 {}" in each "If test", work it out if you will

[Chet T16]

Ok, so here is where i am, based on Psi's code above. I found that way of reading the stream into the array a bit confusing so i've done it my own way. I'm open to correction

Mecha, i was told using a goto is frowned upon! Although i do understand what you were trying to do

Code: [Select]

byte Data[39];   // array to store data
byte index;   // array index
byte readByte = 0; //variable to store byte
int counter = 0;
int numElements = 39; //number of elements we're storing
boolean headerCheck = true;


void setup() {
  // initialize serial communications at 9600 bps:
  Serial.begin(9600);
}
void loop()
{
   if(Serial.available() > 0)
   {
      byte readByte = Serial.read();
      if (headerCheck)
{
      if((readByte == 0xFF) && (counter == 0))
      {     
            counter=1;
            if((readByte == 0x00) && (counter == 1))
              {
                  counter=2;
                  if((readByte == 0x7A) && (counter == 2))
                    {
                        counter=3;
                        if((readByte == 0x01) && (counter == 3))
                          {
                           headerCheck = false; //we've found our header
                          }
                    }
              }
      }
}

else
{
for(int i = 0; i<= numElements-1; i++)
  {
    Data[i] = readByte;
     }
     headerCheck = true;
     
     // parse and display data here
     
    }
   }
  }   

[Mechatrommer]

Psi's code is neater (state machine like code, and you can do housekeeping job in each loop easily) but slower (if speed is your concern). but i dont think you've translated it correctly. i suspect your latest code is more buggy than the old one. what happen in your latest code is it will read 39 data whenever there is no serial data available. worst, it will not read the serial data, instead it will put the previous value of readByte 39 times into the data array. why dont you run the code, read the real input data and see whats the output data on the display? and debug it line by line?

bad practice of goto is only a theoritical thing and lives in abstract world. it will break code structure (jump out of loop or if scope/bracket disgracely) and neat'ness, makes your code harder to debug/trace/follow. but as long as you keep your sane intact, its just a 2 ops instruction. i've found out using brackets with improper vertical alignment (spaces) with its corresponding if/else/loop/while code is much more bad practice (harder to read)

and it was just my "quick fix" to your code, you can give more deeper thought on how to make it more "structured"

[Chet T16]

I think you're reading it wrong (in your defense, my indentation went to hell towards the bottom!)

The 'for' loop to read 39 bytes is only called if serial is available and headercheck is false but yeah, the loop would read in the last byte 39 times

Code: [Select]

byte Data[39];   // array to store data
byte index;   // array index
byte readByte = 0; //variable to store byte
int counter = 0;
int numElements = 39; //number of elements we're storing
boolean headerCheck = true;


void setup() {
// initialize serial communications at 9600 bps:
Serial.begin(9600);
}
void loop()
{
if(Serial.available() > 0)
{
byte readByte = Serial.read();
if (headerCheck)
{
if((readByte == 0xFF) && (counter == 0))
{     
counter=1;
if((readByte == 0x00) && (counter == 1))
{
counter=2;
if((readByte == 0x7A) && (counter == 2))
{
counter=3;
if((readByte == 0x01) && (counter == 3))
{
headerCheck = false;
}
}
}
}
}

else
{
for(int i = 0; i<= numElements-1; i++)
{
Data[i] = Serial.read();
}
headerCheck = true;

// use data

}
}
}   

The output is from an ecu and as yet i haven't managed to get one running on the bench which means sitting in the car with the engine running - not ideal for coding! I want to get the basics done before i have to do that

BTW your username comes up as 'shafri' when using tapatalk

[Mechatrommer]

yes my name is that. you still forgot what Psi told you about the serial.available() in your 39 data reading loop. cheers.

[Chet T16]

I had assumed it was your name, i just thought it odd that it should show up?

Which part am i missing? :-s

[Psi]

There are two ways to do this and i think your getting confused between the two.

-Way one-
The first way is to read in one byte every time the code is executed (from start to end).
The micro runs through all the code and saves one byte into the array (or checks one header byte).
Then the main program loop starts again and it all happens a 2nd time. Saving the next byte into the array. (or checking the next header byte).

-Way two-
This way is only possible because the Arduino has a 128byte buffer built into the serial functions (serial.available, serial.read etc..)
This means at any time in your program the serial buffer might have nothing in it, or anything from 1 to 128 bytes.
Every time you do a serial.read the first byte in the buffer is returned and this byte is deleted from the buffer.
If you call 'Serial.available' it returns the number of bytes currently in the buffer.
So..  "if(Serial.available() > 0)"   means, "if the buffer contains more than 0 bytes"

This means you have the option of reading in multiple bytes from the buffer during one execution run of your code. However, the chances are pretty high that your code is going to run so fast that the buffer won't have time to fill up past 1 byte before you read and clear it. So its probably not worth worrying about unless you plan to have a lot of other code running in the main program loop that might be taking up lots of cpu time.

In essence both ways are the same, it's just that way "one" uses the main program loop to read bytes and way "two" uses a second loop inside the main program loop to do it.
In order to write the code you first need to pick one.
Personally i like the first way as it's quite simple and it keeps the main program loop executing faster (one loop per byte read).

Oh, another thing, you definitely want to avoid having a loop (inside your main loop) which waits for data to arrive in the buffer. This will 'lockup' your main loop during these waiting times and any other code you put in your main program loop later on will be effected by this lockup.

You can read out all bytes in the buffer and save them (way two) and that's fine, but you don't want to wait for new bytes to arrive. Let the main loop keep executing as fast as it can and process bytes as they appear in the buffer.

[Chet T16]

Right, I see where I'm being stupid

a) I had assumed there would always be data present

b) I had assumed it would be coming fast enough that I needed to read it as fast as possible

Btw the data is coming at 62500bps, not the 9600 I have in the code

I'll redo the code when I'm on the laptop and post it up

[Chet T16]

Is this code you posted correct?

Code: [Select]

                        if ( (Counter-4) <= 38) 
{
MyData[Counter-4] = ReadByte;
Counter++;

                                if ( (Counter-4) == 38)
{
                                        Counter = 0;
                                  CheckingFrameHeader = true;
}
}
Or should it be "if ( (Counter-4) == 39)"?

[Chet T16]

Code: [Select]

byte Data[39];   // array to store data
byte index;   // array index
byte readByte = 0; //variable to store byte
int counter = 0;
int numElements = 39; //number of elements we're storing
boolean headerCheck = true;
boolean arrayFilled = false;


void setup() {
// initialize serial communications at 62500 bps:
Serial.begin(62500);

}
void loop()
{
if(Serial.available() > 0)
{
byte readByte = Serial.read();
if (headerCheck)
{
if((readByte == 0xFF) && (counter == 0))
{     
counter=1;
}

else
{
counter = 0;
}
if((readByte == 0x00) && (counter == 1))
{
counter=2;
}
else
{
counter = 0;
}
if((readByte == 0x7A) && (counter == 2))
{
counter=3;
}
else
{
counter = 0;
}
if((readByte == 0x01) && (counter == 3))
{
counter = 4;
headerCheck = false;
}
else
{
counter = 0;
}
}

else
{
if(counter-4 <38)
{
Data[counter-4] = readByte;
counter++;
if(counter-4==39)
{
counter = 0;
headerCheck = true;
arrayFilled = true;
}
}
}
}
if(arrayFilled)
{
// use data
}
}   


[baljemmett]

Code: [Select]

if((readByte == 0xFF) && (counter == 0))
{     
counter=1;
}

else
{
counter = 0;
}


That won't do what you expect; it'll be stuck toggling between the 'idle' and 'read first header byte' states, because once you're in the counter==1 state any incoming byte will reset to counter==0.  Try something along these lines:

Code: [Select]

if (headerCheck)
{
    switch(counter)
    {
        case 0: if (readByte==0xFF) counter = 1; else counter = 0; break;
        case 1: if (readByte==0x00) counter = 2; else counter = 0; break;
        case 2: if (readByte==0x7A) counter = 3; else counter = 0; break;
        case 3: if (readByte==0x01) headerCheck = false; else counter = 0; break;
    }
}

I condensed the logic into a switch because I find it neater for what is basically a state machine; if you don't like that the equivalent (following your style) is the rather more verbose:

Code: [Select]

if (headerCheck)
{
    if (counter == 0)
    {
        if (readByte == 0xFF)
        {
            counter=1;
        }
        else
        {
            counter=0;
        }
    }
    ...
}

(Edit: blast, so used to typing quote tags I forgot I wanted a code block!)

[Mechatrommer]

Or should it be "if ( (Counter-4) == 39)"?

on behalf of Psi... no! zero indexing array 0-38 = 39 bytes. after the last read/write (ie array 38), toggle back to state 0. hence Psi's code is correct, your proposal is not. state 0-3, reading 4 bytes header, state (counter) 4 start data read/write at array index zero (counter-4 = 4-4 = 0). you should imaginate or have the picture flowing in your mind. please ammend your code in reply #16.

it wont reach the inner scope (if) by doing code below... recheck your "inequality" or "set" formulation.

Code: [Select]

if (counter - 4 < 38) {
    if(counter - 4 == 39) {
        // inner scope
    }
}

edit: i think "CheckingFrameHeader" in Psi's code is redundant, "counter" variable is enough to make the "state machine code" stable. but i have'nt check his code thouroughly, so i'm not 100% sure.

[Psi]

hehe, yeah,

That's one of the annoying things in programming, trying to deal with stuff that starts at zero. It actually all makes perfect sense once you've done programmed for a while but when you're starting out it traps lots of people.

The human brain doesn't want to think of the first occurrence of something as being the zeroth occurrence

edit: i think "CheckingFrameHeader" in Psi's code is redundant, "counter" variable is enough to make the "state machine code" stable. but i have'nt check his code thouroughly, so i'm not 100% sure.

Yeah,
rereading my code I see a bug in that part of the code.

I've recoded/simplified it using baljemmett's switch statement above.
This is much shorter and it should work fine, as long as Ardunio C accepts the case range syntax (gcc does so i think it will)

Code: [Select]

if (Serial.available())
{
      byte readByte = Serial.read();
      switch(counter)
        {
case 0:   if (readByte==0xFF) counter = 1 else counter = 0;   break;
case 1:   if (readByte==0x00) counter = 2 else counter = 0;   break;
case 2:   if (readByte==0x7A) counter = 3 else counter = 0;   break;
case 3:   if (readByte==0x01) counter = 4 else counter = 0;   break;
case 4 ... 42:   // 39 bytes
MyData[Counter-4] = ReadByte;
if (Counter == 42) Counter=0 else Counter++;
break;
default: Counter=0;
}
}


[Chet T16]

on behalf of Psi... no! zero indexing array 0-38 = 39 bytes. after the last read/write (ie array 38), toggle back to state 0. hence Psi's code is correct, your proposal is not. state 0-3, reading 4 bytes header, state (counter) 4 start data read/write at array index zero (counter-4 = 4-4 = 0). you should imaginate or have the picture flowing in your mind. please ammend your code in reply #16.

This was my interpretation of the code

                        if ( (Counter-4) <= 38)   <-assume counter is 42 (-4=38), condition is true
         {
            MyData[Counter-4] = ReadByte; <- we read element 38, the 39th byte
            Counter++; <- counter increases to 43

                                if ( (Counter-4) == 38) <- counter-4 is now 39 and so this condition isn't met
            {
                                        Counter = 0;
                                    CheckingFrameHeader = true;
            }   
         }

[Chet T16]

Now using the switch statemement, it compiles in the Arduino IDE so i take it the case range is ok

Code: [Select]

byte Data[39];   // array to store data
byte index;   // array index
byte readByte = 0; //variable to store byte
int counter = 0;
boolean headerCheck = true;
boolean arrayFilled = false;


void setup() {
// initialize serial communications at 62500 bps:
Serial.begin(62500);
}
void loop()
{
if(Serial.available() > 0)
{
byte readByte = Serial.read();
switch(counter)
{
case 0:   if (readByte==0xFF) counter = 1; else counter = 0;   break;
case 1:   if (readByte==0x00) counter = 2; else counter = 0;   break;
case 2:   if (readByte==0x7A) counter = 3; else counter = 0;   break;
case 3:   if (readByte==0x01) counter = 4; else counter = 0;   break;
case 4 ... 42:   // 39 bytes
Data[counter-4] = readByte;
if (counter == 42) {counter=0; arrayFilled = true;} else counter++;
break;
default: counter=0;
}
}
if(arrayFilled)
{
// use data
}
}   

[Psi]

About previous example....
Well spotted. Yeah, you're right it would have to be 39 for it to work properly. Or the increment line could be moved to the else part of the IF statement, either would work.

About new example....
I see you've added a variable (arrayFilled) to indicate that a frame has been read into the array and is ready to be processed.
If a new byte arrives before you have finished processing the old data it might corrupt your array with half old half new data.

To solve that issue i recommend you have a rest area in the case/select statement that does nothing, maybe case 255.
The Counter can be set to this after the last byte is saved. Then the data in the array is safe and will never be updated until you set the counter back to zero.
It also means you dont need the new variable, as you can check for Counter==255

Eg,

Code: [Select]

if(Serial.available() > 0)
{
byte readByte = Serial.read();
switch(counter)
{
case 0:   if (readByte==0xFF) counter = 1; else counter = 0;   break;
case 1:   if (readByte==0x00) counter = 2; else counter = 0;   break;
case 2:   if (readByte==0x7A) counter = 3; else counter = 0;   break;
case 3:   if (readByte==0x01) counter = 4; else counter = 0;   break;
case 4 ... 42:   // 39 bytes
Data[counter-4] = readByte;
if (counter == 42) {counter=255} else counter++;
break;
        case 255;
                    // rest state after message has been saved, nothing will happen until counter is set to 0
                break;
default: counter=0;
}
}
if(Counter==255)
{      // use data to do some stuff
                Counter==0;   // final command to allow array to be filled with the next message.
}
This way, if you take to long to process a frame it will just skip all subsequent frames until you are ready.

You could use a define to make it easier to read
eg,  #define ArrayFull 255

[Chet T16]

Ok, revision 4365!

Code: [Select]

byte Data[39];   // array to store data
byte index;   // array index
byte readByte = 0; //variable to store byte
int counter = 0;
boolean headerCheck = true;
boolean arrayFilled = false;


void setup() {
// initialize serial communications at 62500 bps:
Serial.begin(62500);
}
void loop()
{
if(Serial.available() > 0)
{
byte readByte = Serial.read();
switch(counter)
{
case 0:   if (readByte==0xFF) counter = 1; else counter = 0;   break;
case 1:   if (readByte==0x00) counter = 2; else counter = 0;   break;
case 2:   if (readByte==0x7A) counter = 3; else counter = 0;   break;
case 3:   if (readByte==0x01) counter = 4; else counter = 0;   break;
case 4 ... 42:   // 39 bytes
Data[counter-4] = readByte;
if (counter == 42) counter=255; else counter++;
break;
case 255:   // rest state after message has been saved, nothing will happen until counter is set to 0
break;
default: counter=0;
}
}
if(counter == 255)
{
// use data
counter=0;
}
}   

[Psi]

Looks good 
That state machine should allow you to read in the data and do whatever you want with it.

[Chet T16]

Looks good 
That state machine should allow you to read in the data and do whatever you want with it.

Thanks for the help (everyone!)

I've added in a loop to print the array to a software serial port so i'll give it a test and see what happens

[Mechatrommer]

MyData[Counter-4] = ReadByte;
Counter++;

ops, you are right, i should imaginate and have the picture flowing in my mind but still your previous code is not 100% right, so maybe we all got some mistake. thats why i used to extend the possibility, such as...

Code: [Select]

if ((Counter - 4) <= 38) {
MyData[Counter - 4] = ReadByte;
Counter++;

if ((Counter - 4) >= 38) {
DisplayAndOrClearData(); // another function
Counter = 0;
}
}


[Chet T16]

Dammit, a hardware issue somewhere

To read the data i need to invert the signal from the ECU which i use this circuit for



I use this with an FTDI cable to read the stream directly and its fine. When there is data coming the LED flashes but when i connect Rx to the arduino rx pin the LED goes out. I just checked at theres 5v at the arduino rx and tx pins. Whats that all about??