code line ordering - int not writing at correct time

I seem to be having a problem with an int writing at the correct time. In the below code I have a button setup to send a serial command to a device($ABCDEF=?#). The device then returns a serial command($CAMGET=1234#) that is converted to an int(integerFromPC) which needs to print to serial monitor.

I can get this to work using two buttons, one to send the command to the device and a second button to read the return command and then print it to the serial monitor. However, if I try and do it all on one button then I get the incorrect value for integerFromPC the first time the button is pressed and the correct value the second time it's pressed.

Can anyone shed some light as to what I have out of order and how I might fix it? Sorry for the sloppy code.

#define HWSERIAL Serial2

const byte numChars = 32;
char receivedChars[numChars];
char tempChars[32];        // temporary array for use by strtok() function

/*---comparing header types from Camera---*/
int messageFound = -1;
char messageBuffer[2][7] = {"CAMACK", "CAMGET"};

/*---THE NEXT 3 LINES ARE PART OF THE PARSE AND HEX CONVERSION---*/
char messageFromPC[32] = {0};
char dataFromPC[5] = {0};
int integerFromPC = 0;

boolean newData = false;

const int buttonPin = 8;
int buttonState = 0;

void setup() {
  Serial.begin(9600);
  Serial.println("<Arduino is ready>");
  HWSERIAL.begin(38400);
  delay(1000);
  pinMode(buttonPin, INPUT);
}

void loop() {
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH) {
    Serial.print("RED    RIGHT  ");          //debug only
    HWSERIAL.println("$ABCDEF=?#");
    delay(500);
    integerFromPC = map(integerFromPC, 0, 10239, 0 , 1000);
    Serial.println(integerFromPC);
  }
  recvWithStartEndMarkers();
  strcpy(tempChars, receivedChars);
  parseData();
  showNewData();
}

/*---CAPTURING THE SERIAL DATA FROM CAMERA STARTING WITH "$" AND ENDING WITH "#" ---*/
void recvWithStartEndMarkers() {
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char startMarker = '

serial monitor returns - where the 0 should read the same value as New Integer

RED RIGHT 0
Camera Returned 1880
New Integer 6272
;
  char endMarker = '#';
  char rc;

// if (Serial.available() > 0) {
  while (HWSERIAL.available() > 0 && newData == false) {
    rc = HWSERIAL.read();

if (recvInProgress == true) {
      if (rc != endMarker) {
        receivedChars[ndx] = rc;
        ndx++;
        if (ndx >= numChars) {
          ndx = numChars - 1;
        }
      }
      else {
        receivedChars[ndx] = '\0'; // terminate the string
        recvInProgress = false;
        ndx = 0;
        newData = true;
      }
    }

else if (rc == startMarker) {
      recvInProgress = true;
    }
  }
}

/---PRINTING THE CAPTURED DATA TO SERIAL MONITOR FOR DEBUG ONLY---/
void showNewData() {
  if (newData == true) {
    /* FOR DEBUG ONLY
      Serial.print("Input = ");
      Serial.println(receivedChars);
      Serial.print("Message = ");
      Serial.println(messageFromPC);
      Serial.print("Data = ");
      Serial.println(dataFromPC);
      Serial.print("Integer = ");
      Serial.println(integerFromPC);
    /
/
---looking at the camera's return message to see if it's valid and what to do with it---*/
    for (int i = 0; i < 2; i++) {
      if (strcmp(messageFromPC, messageBuffer[i]) == 0) {
        messageFound = i;
        break;
      }
    }
    switch (messageFound) {
      case 0:
        Serial.print("Command Acknowledged  ");
        Serial.println(dataFromPC);
        messageFound = -1;
        break;
      case 1:
        Serial.print("Camera Returned  ");
        Serial.println(dataFromPC);
        Serial.print("  New Integer  ");
        Serial.println(integerFromPC);
        messageFound = -1;
        break;
      default:
        Serial.print("Invalid  ");
        Serial.println(receivedChars);
        break;
    }

newData = false;
  }
}

/---SPLIT THE DATA INTO ITS TWO PARTS---/
void parseData() {

char * strtokIndx; // this is used by strtok() as an index

strtokIndx = strtok(tempChars, "=");        // get the first part - the string
  strcpy(messageFromPC, strtokIndx);          // copy it to messageFromPC

strtokIndx = strtok(NULL, "=");              // this continues where the previous call left off
  strcpy(dataFromPC, strtokIndx);              //copy it to dataFromPC
  sscanf(dataFromPC, "%x", &integerFromPC);    //converts HEX string to a DEC int
}


serial monitor returns - where the 0 should read the same value as New Integer

RED RIGHT 0
Camera Returned 1880
New Integer 6272
  recvWithStartEndMarkers();
  strcpy(tempChars, receivedChars);

The recv function sets a global variable to tell you if it's got new data to be parsed but you go ahead and start copying it around without checking if there's any data there to be parsed. This can easily mangle the memory as the receivedChars may not contain a null-terminated string and strcpy() can just spray garbage across your program memory with no limitation.

Always use strncpy() and tell it the maximum number of chars you expect to copy. This should be equal to or less than the size of the destination. Always use #define or const int named constants for this as you may change the size of any of the strings at any time in the future.

But first check if there's something to be parsed.

Thanks for the pointer Morgan. I have changed strcpy(tempChars, receivedChars); to strncpy(tempChars, receivedChars, sizeof(tempChars));

While that was a potential bug it has no effect on my current issue. The serial monitor output in Post 1 shows integerFromPC to be reporting as 0 (RED RIGHT 0). That 0 is coming from the initialization of int integerFromPC = 0; (on line 14 of the code). I have proven this by changing the value on line 14 to 50. The serial monitor will then output RED RIGHT 4. The value shows as 4 instead of 50 because of the map on line 35.

If I push the button a second time then the serial monitor will report the correct mapped value for New Integer.

integerFromPC gets a new value from void parseData() after a valid serial input. My question is why am I getting the initialized value of integerFromPC the first time I push the button. I would have expected to get the updated value as there is a valid serial input before writing to the serial monitor.

Drawing on what @MorganS has said this piece of your code

  recvWithStartEndMarkers();
  strcpy(tempChars, receivedChars);
  parseData();
  showNewData();

should be like this

recvWithStartEndMarkers();
if (newData == true) {  
 strcpy(tempChars, receivedChars);
 parseData();
 showNewData();
 newData = false;
}

and you can take the references to newData out of your modified showNewData().

...R
Edited to correct the location of the line if (newData == true) {

Thanks Robin. I gave that a try, but still get the same result. The code must still be blocking.

Nick, Thanks for the link. I've read through it and will give the state machine a try.

MorganS:
Always use strncpy() and tell it the maximum number of chars you expect to copy. This should be equal to or less than the size of the destination.

Why do you recommend strncpy() over strlcpy()?

strlcpy() is not ISO standard, and can cover up problems. Your dest buffer should always be big enough to copy in the src buffer and the null terminator. Be relatively easy to find why a program is crashing/not working due to a lack of terminator, usually more hard to find where and why its crashing when a terminator is placed at the end of an incomplete string.

Found the problem. The IF statement was blocking the HWSERIAL.read. By moving the map and Serial.print to a separate IF statement everything works correctly

void loop() {
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH) {
    Serial.println("RED    RIGHT  ");          //debug only
    HWSERIAL.println("$ABCDEF_=?#");
    delay(500);
    somethingClever = 1;
  }

  recvWithStartEndMarkers();
  if(newData == true){
    strcpy(tempChars, receivedChars);
    parseData();
    showNewData();
    newData = false;
  }
    if (somethingClever == 1) {
    integerFromPC = map(integerFromPC, 0, 10239, 0 , 1000);
    Serial.print("$ABCDEF_= ");
    Serial.println(integerFromPC);
    somethingClever = 0;
  }
}

tammytam:
Be relatively easy to find why a program is crashing/not working due to a lack of terminator

How does one find that?

I ask because lack of a terminator was causing rather random results for me. Sometimes it would print a random mess to the serial monitor but sometimes it would reset the Arduino.
I was testing what would happen if src was bigger than dest. In case user didn't follow directions or type carefully, what would happen. I was curious.
Using 3 state arrays state1[8]; state2[8]; and state3[8]; sometimes I could throw in a state like Illinois or Kentucky to state2[8] and the output seemed fine sometimes. I was expecting the data to get truncated but it wasn't. That is if I sent Indiana,Kentucky,Ohio I would get Indiana Kentucky Ohio on the serial monitor output. But if I sent Illinois to all 3 state arrays it would generate continuous junk on the serial monitor. Oddly if I sent Kentucky to all 3 arrays the Arduino would reset. Hard to see what happened when it just resets. So same size mismatch yet very different results. And trying each way 5 times resulted in same behavior, 3 x Illinois ended up with random printing, 3 x Kentucky always reset. And even more odd I could enter Indiana,Ohio,Mississippi and get Indiana Mississippi on screen.

Anyway, the point of the rambling with lack of terminator causing all sorts of unpredictable results is again to ask what is this relatively easy way to determine the lack of a terminator is the cause?

A state machine, described in my earlier post, acts on input when it gets the terminator. Therefore you can never process un-terminated input. (Although what you do with input that is too long is up to you - you could consider it terminated, or raise an error).