Please look over

I want to write serial data to an external I2C FRAM chip. I haven’t recieved my prototype board to physically assemble this yet.

Am I on the right track to making this work? I have alot of other large data streams to read in and transfer to external RAM before sorting and resending out Serial. I am planning to reuse the array tempArray for each read in.

#include <Wire.h>


//ram i2c address
int addr = 80; //B1010000;
unsigned long TimerA;
byte tempArray[900];

typedef enum { 
  messageInit, messageWaitFor10, messageWaitForID, p60sCollectTheData, p70sCollectTheData, p55sCollectTheData, p49sCollectTheData, p74sCollectTheData, pE4sCollectTheData, CheckPacket, Fini } 
IDstate_t;
  
 bool Serial2_WaitForByte( byte & v )  //function like void
{
  while ( Serial2.available () <= 0 );
  v = Serial2.read();
  TimerA =0;
  if (millis()-TimerA >= 500UL){
    Serial.println("timeout no COMM");
  }
  return( true );
}


void inMessage()
{
  IDstate_t state;
  byte inbyte;
  byte n;

  state = messageInit;

  while ( state != Fini )
  {
    switch ( state )
    {
messageWaitFor10:
      TimerA =0;
      if ( Serial2_WaitForByte( inbyte ) )
      {
        if ( inbyte == 0x10 )
        {
          state = messageWaitForID;
        }
      }
      else if (millis()-TimerA >= 1000UL){
        Serial.println("reciever communication time out error message start");
      }
      break;

messageWaitForID:
      TimerA =0;
      if ( Serial2_WaitForByte( inbyte ) )
      {
        if ( inbyte == 0x60 )
        {
          state = p60sCollectTheData;
        }
        if ( inbyte == 0x70 )
        {
          state = p70sCollectTheData;
        }
        if ( inbyte == 0x55 )
        {
          state = p55sCollectTheData;
        }
        if ( inbyte == 0x49 )
        {
          state = p49sCollectTheData;
        }
        if ( inbyte == 0x74 )
        {
          state = p74sCollectTheData;
        }
        if ( inbyte == 0xE4 )
        {
          state = pE4sCollectTheData;
        }
        if ( inbyte == 0x10 )
        {
          state = messageWaitForID; 
        }
      }
      else if (millis()-TimerA >= 500UL){
        Serial.println("reciever communication time out error message ID");
      }
      break;


p60sCollectTheData:   //13 bytes with null
      int n=0;
      TimerA =0;
      if ( Serial2_WaitForByte(tempArray[n]) )
      {
        ++n;

        if ( n >= 13 )
        {
          if (tempArray[11] == 0x03 && tempArray[12] == 0x10){
            for(int i=0; i<14; i++)
            {
              Write60h(addr,i*8,tempArray[i]); 
               state = Fini
            } 
          }
        }
        else if (millis()-TimerA >= 500UL){
          Serial.println("reciever communication time out error 60h");
        }
      }
      break;

    }  
  } 
}   

 //Write to i2c SRAM

void Write60h(int i2c_addr, word address, byte data)
{// 13 byte for array starting at 20 to 33
  Wire.beginTransmission(i2c_addr);
  Wire.write((byte)lowByte(0x14));
  Wire.write((byte)highByte(0x21)); 
  Wire.write(data); 
  Wire.endTransmission(); 
}
  
  
void setup(){
  Wire.begin();
  Serial.begin(9600);
  Serial2.begin(115200);
}

void loop(){
  
  inMessage(); 
}

I find this function interesting:

bool Serial2_WaitForByte( byte & v )  //function like void
{
  while ( Serial2.available() <= 0 );
  v = Serial2.read();
  TimerA =0;
  if (millis()-TimerA >= 500UL){
    Serial.println("timeout no COMM");
  }
  return( true );
}

You are in a while loop until something is recieved on the Serial2 port. The print of “timeout no COMM” seems pointless as if there was no communication, then it wouldn’t ever timeout (it would be stuck in the while loop).
Also, if you are always returning true, then it seems to me there is little point in it having a return type.
The other thing is that usually if you wanted to return a byte (but didn’t want to use the return variable) you would pass a pointer, not a byte which you are for some reason then trying to do something weird with in the function call.

Perhaps something like this was your intention

bool Serial2_WaitForByte( byte* v ) 
{
  TimerA = millis(); //store the current value of millis
  while ( !Serial2.available()){ //available() is never less than 0, so you don't need to bother with the <= sign.
    if (millis()-TimerA >= 500UL){
      //If more than 500 milliseconds have passed, then return false (communication error?).
      Serial.println("timeout no COMM");
      return false;
    }
  }
  *v = Serial2.read(); //Read the byte to the
  return true; //sucessfully read the byte.
}

You would then use it like this in you inMessage() function:

if ( Serial2_WaitForByte( &inbyte ) ){
  //Recieved a byte...
  
} else {
  //Timeout

}

OK I see some of what you are telling me, however I’m still trying to wrap my head around the pointer usage. I tried using the “byte * v” in place of “byte & v” however it would not compile. I want to return a byte value if its recieved or wait untill one comes in.

bool Serial2_WaitForByte(byte v)  //function like void
{
  TimerA =millis();
  while ( Serial2.available () <=0)
  {
    if (millis()-TimerA >= 500UL){
      Serial.println("timeout no COMM");
      return false;
    }
  }
  v = Serial2.read();
  return true ;
}


void inMessage()
{
  IDstate_t state;
  byte n;
  byte inbyte;
  state = messageInit;

  while ( state != Fini )
  {
    switch ( state )
    {
messageWaitFor10:
      TimerA =millis();
      if ( Serial2_WaitForByte( inbyte ) )
      {
        if ( inbyte == 0x10 )
        {
          state = messageWaitForID;
        }
      }
      else if(millis()-TimerA >= 1000UL){
        Serial.println("reciever communication time out error message start");
      }
      break;
// other sections follow like above

The way I just showed you compiles fine, and is how pointers are normally used.

Serial2_WaitForByte( &inbyte );

Means: I have this byte called inbyte. I’m going to tell the function where in the memory the byte is located - the ‘&’ symbol, which means ‘The memory Address of’.

Then you have the function:

bool Serial2_WaitForByte( byte* v ) 
{
  ...
  *v = Serial2.read(); //Read the byte to the
  ...
}

This says: I am a block of code called ‘Serial2_WaitForByte’. In order to use me, you must tell me the location in the memory where a byte is located (the declaration byte* ← note the * representing a pointer).
During the function, you say, “i have this byte named ‘v’ where a byte can be stored”.
v = someByte;
However, if you say this, it wont work - you cannot save anything to ‘v’ as it is a pointer, which is just an address, so how do you save your byte? Well, you instead say:
“I have this pointer/reference to a byte named ‘v’. I want to say my byte to the location where it points”. To do this you have to dereference ‘v’. This is doen with the dereference operator ‘*’:
*v = someByte; //Note the dereference

Hopefully that helps to explain pointers a little better. :slight_smile:

It's called "pass by reference" and @JHEnt is using it correctly.

Ahh, cool. You learn something new every day!

I find this function interesting:

I found it interesting, too, but for a different reason. The function ALWAYS returns true. How useful is that?

PaulS:

I find this function interesting:

I found it interesting, too, but for a different reason. The function ALWAYS returns true. How useful is that?

Except when it returns false, when it times out.

if (millis()-TimerA >= 500UL){
      Serial.println("timeout no COMM");
      return false;
    }

Except when it returns false, when it times out.

That was in Tom Carpenter's code, not JHEnt's code. The original code contained one return statement, and that always returned true.