Serial read problem... Losing my mind

Using Arduino Programming Questions
1

Hey everyone. Problem I've been trying to troubleshoot for more than I'd like to admit but I just can't figure out what's going on here.

For example with this sketch:

#include <SoftwareSerial.h>

SoftwareSerial output(A2, A3);

void setup() {
  Serial.begin(38400);
  output.begin(38400);

}

void loop() {

  if (Serial.available()) {
    output.write(Serial.read());
  }

  if (output.available()) {
    Serial.write(output.read());
  }

}

I have a modem connected to arduino hardware serial port and a Software serial port for debugging.

The modem sends and recieves some messages via the RF modem from another device i have setup

from the other device i send the following string

8200FF01234C5453542359454C230A1

This is what gets output on my debugging terminal:

+CTSDSR: 108,302201300001012,1,302201300001069,1,124
8200FF01234C5453542359454C230A1

This is what i expect... Works like it should.

The problem comes in when i try with a different sketch that is looking for particular messages. To simplify my sketch:

void loop() {

  if (radio.available()) {
    int i = readRadio(serialData, TIMEOUT_LONG);
#ifdef DEBUG
    debugSerial.print("[main] Data returned from radio = ");
    debugSerial.println(serialData);
#endif
  }
}

///////////////////////////////////////////////////////////////////// READ RADIO /////////////////////////////////////////////////////////////////////

int readRadio(char * buf, int timeout) {
  unsigned long startTime = millis();
  byte bytesRead = 0;
  char inByte;

  for (;;) {
    delay(1);
    if (radio.available()) {
      inByte = radio.read();
      if (inByte == '\r' || inByte == '\n') {
        if (bytesRead == 0) {
          //buffer is empty, restart
          continue;
        }
        else if (bytesRead > 0) {
          //we have data, end
          break;
        }
      }
      buf[bytesRead++] = inByte;
    }
    else {
      if ((int)millis() - startTime > timeout) {
        //serial timeout
#ifdef DEBUG
        if (bytesRead > 0) {
          debugSerial.println("[readRadio]Serial timed out");
        }
#endif
        break;
      }
    }
  }
  if (buf) {
    buf[bytesRead] = '\0';
  }
#ifdef DEBUGSERIAL
  if (bytesRead > 0) {
    debugSerial.print("[readRadio] bytes returned = ");
    debugSerial.println(bytesRead);
    debugSerial.print("[readRadio] buffer content = ");
    debugSerial.println(buf);
  }
#endif
  return bytesRead;
}

The output is always missing the last byte. My message from the other device has an incrementing counter on the end... This is what i get out if i send the same message as before
8200FF01234C5453542359454C230A.

That A character increments by one digit, but it's always missing.

My readRadio function is reading until it gets a cr or lf and in other applications has worked fine. I can't understand why it is now consistently missing that last character.

Any ideas?

2

Please post the full sketch so we can see how global variables, etc are defined. Also, want to see the setup() function.

3 
      if ((int)millis() - startTime > timeout) {

Why are you casting the value (an unsigned long) returned my millis() to an int? Less than 33 seconds after the Arduino resets, the cast will start producing garbage. Why do you want to do THAT?

Why do you have a delay() in the code to read each character from the radio? Are you TRYING to lose data?

4

PaulS:

      if ((int)millis() - startTime > timeout) {

Why are you casting the value (an unsigned long) returned my millis() to an int? Less than 33 seconds after the Arduino resets, the cast will start producing garbage. Why do you want to do THAT?

Why do you have a delay() in the code to read each character from the radio? Are you TRYING to lose data?

the cast was an error on my part... I will remove it.

The delay is there because without it reading data from this particular modem is inconsistent and program flow control falls off a cliff.

Because of how the messages are coming in I have to take action based on the first line (+CTSDSR) but the control is on the second. So i either block in a function while i wait for it, or i wait for it in the function that reads the serial port. At this point there isn't so much data that loss is a real concern... I get it, not how you'd do it. But it's really a struggle for me to let main() free run while trying to keep track of what data I'm waiting on. The device has nothing else to do until it gets the control line anyway.

Now i didn't put it in the second example which is my bad but using my sketch i still get the complete first line which is more data than the second so where would it be getting lost?

5

can you use slower SoftwareSerial baud rate?

6

gfvalvo:
Please post the full sketch so we can see how global variables, etc are defined. Also, want to see the setup() function.

#define DEBUG
#define DEBUGSERIAL

#if   defined(ARDUINO_AVR_ADK)
#define BOARD "Mega Adk"

#elif defined(ARDUINO_AVR_MEGA)
#define BOARD "Mega"

#elif defined(ARDUINO_AVR_MEGA2560)
#define BOARD "Mega 2560"

#elif defined(ARDUINO_AVR_MICRO)
#define BOARD "Micro"
#define radio Serial1
#define debugSerial Serial

#elif defined(ARDUINO_AVR_PRO)
#define BOARD "Pro"
#define radio Serial
#include <SoftwareSerial.h>
SoftwareSerial debugSerial(A2, A3);

#elif defined(ARDUINO_AVR_MINI)
#define BOARD "Mini"

#elif defined(ARDUINO_AVR_NANO)
#define BOARD "Nano"

#elif defined(ARDUINO_AVR_UNO)
#define BOARD "Uno"
#define radio Serial
#include <SoftwareSerial.h>
SoftwareSerial debugSerial(A2, A3);

#endif

/*
   ABOVE DEFINITIIONS ARE TO DETERMINE WHAT BOARD YOU'RE USING
   AND ASSIGN THE SERIAL PORTS APPROPRIATELY.
   MODEM WILL ALWAYS BE CONNECTED TO HARDWARE UART. DEBUGGING OUTPUT
   THROUGH SOFTWARE SERIAL (PINS A2,A3) IF NO OTHER PORT IS
   AVAILABLE.
*/

///////////////////////////////////////////////////////////////////// GSSI AND CONTROL TYPE /////////////////////////////////////////////////////////////////////

#define LIGHT1 "1012"
#define LIGHT2 "1013"

#define GSSI "103001"


///////////////////////////////////////////////////////////////////// CONTROL OPTIONS /////////////////////////////////////////////////////////////////////


#define MAX_SDS_RESPONSE_SIZE 128

#define YELLOW "8200FF01234C5453542359454C4C4F5723"
#define RED "8200FF01234C5453542352454423"
#define UPDATE "8151" //STATUS MESSAGE

#define LIGHT_MSG "+CTSDSR: 108,3022013000"

#define DEL "FE00"

///////////////////////////////////////////////////////////////////// SERIAL CHARACTERISTICS /////////////////////////////////////////////////////////////////////

#define TIMEOUT_SHORT 100
#define TIMEOUT_LONG 500
#define MAXCHARS 64 //char array size incoming serial data

///////////////////////////////////////////////////////////////////// PIN ASSIGNMENTS /////////////////////////////////////////////////////////////////////

#define whitePin 2
#define redPin 3
#define yellowPin 4


///////////////////////////////////////////////////////////////////// GLOBAL VARIABLES /////////////////////////////////////////////////////////////////////

char serialData[MAXCHARS + 1];

//FOR LIGHT STATES FALSE = RED, TRUE = YELLOW

bool wandLight = false;
bool lastWandStatus = false;
bool l1State = false;
bool l2State = false;

unsigned long previousTime = 0;

///////////////////////////////////////////////////////////////////// SETUP /////////////////////////////////////////////////////////////////////

void setup() {

#ifdef DEBUG
  debugSerial.begin(38400);
  debugSerial.println("[Setup]Beginning Setup");
#endif

  //Set inputs and outputs
  pinMode (whitePin, OUTPUT);
  pinMode (redPin, OUTPUT);
  pinMode (yellowPin, OUTPUT);


  /*Set initial light state
     don't use setState() as everything isn't initialized yet
     You want the light to be red before anything else
  */
  digitalWrite(redPin, HIGH);
  digitalWrite(yellowPin, LOW);
  digitalWrite(whitePin, LOW);

  //open radio serial port
  radio.begin(38400);

  //Empty Serial Buffer
  if (radio.available() > 0) {
  dumpSerial(serialData);
  }
  
#ifdef DEBUG
  debugSerial.println("[Setup]Complete!");
#endif

}

///////////////////////////////////////////////////////////////////// MAIN /////////////////////////////////////////////////////////////////////

void loop() {

  unsigned long startTime = millis();

  if (radio.available()) {
    int i = readRadio(serialData, TIMEOUT_LONG);
   if (i > 0) {
     checkForResponse(serialData, LIGHT_MSG, LIGHT1, LIGHT2);
    }
#ifdef DEBUG
    debugSerial.print("[main] Data returned from radio = ");
    debugSerial.println(serialData);
#endif
  }
}

///////////////////////////////////////////////////////////////////// READ RADIO /////////////////////////////////////////////////////////////////////

int readRadio(char * buf, int timeout) {
  unsigned long startTime = millis();
  byte bytesRead = 0;
  char inByte;

  for (;;) {
    delay(1);
    if (radio.available()) {
      inByte = radio.read();
      if (inByte == '\r' || inByte == '\n') {
        if (bytesRead == 0) {
          //buffer is empty, restart
          continue;
        }
        else if (bytesRead > 0) {
          //we have data, end
          break;
        }
      }
      buf[bytesRead++] = inByte;
    }
    else {
      if (millis() - startTime > timeout) {
        //serial timeout
#ifdef DEBUGSERIAL
        if (bytesRead > 0) {
          debugSerial.println("[readRadio]Serial timed out");
        }
#endif
        break;
      }
    }
  }
  if (buf) {
    buf[bytesRead] = '\0';
  }
#ifdef DEBUGSERIAL
  if (bytesRead > 0) {
    debugSerial.print("[readRadio] bytes returned = ");
    debugSerial.println(bytesRead);
    debugSerial.print("[readRadio] buffer content = ");
    debugSerial.println(buf);
  }
#endif
  return bytesRead;
}
7 
if (radio.available()) {
      inByte = radio.read();
      if (inByte == '\r' || inByte == '\n') {
        if (bytesRead == 0) {
          //buffer is empty, restart
          continue;
        }
        else if (bytesRead > 0) {
          //we have data, end
          break; 
        }
      }
      buf[bytesRead++] = inByte;
    }

I think that the break at the end of reading is taking you outside of the for(;;) loop and you are not getting the bytesRead index increment. Then, you are overwriting the last character received before the \r or \n when you use

if (buf) {
    buf[bytesRead] = '\0';
  }
8

cattledog:

if (radio.available()) {

inByte = radio.read();
      if (inByte == '\r' || inByte == '\n') {
        if (bytesRead == 0) {
          //buffer is empty, restart
          continue;
        }
        else if (bytesRead > 0) {
          //we have data, end
          break;
        }
      }
      buf[bytesRead++] = inByte;
    }




I think that the break at the end of reading is taking you outside of the ` for(;;) ` loop and you are not getting the bytesRead index increment. Then, you are overwriting the last character received before the \r or \n when you use

if (buf) {
    buf[bytesRead] = '\0';
  }

no, the function is correct. it works with Serial

9

Have a look at the examples in Serial Input Basics - simple reliable ways to receive data.

...R

10

no, the function is correct. it works with Serial

The OP will need to clarify, but I thought the problem happened when the change was made from the simple echo of the Serial.read() to the reading with end marker.

The problem comes in when i try with a different sketch

11

cattledog:
The OP will need to clarify, but I thought the problem happened when the change was made from the simple echo of the Serial.read() to the reading with end marker.

yes

the working sketch is SerialPassthrough sketch

SoftwareSerial is on edge with 38400 baud

12

cattledog:
The OP will need to clarify, but I thought the problem happened when the change was made from the simple echo of the Serial.read() to the reading with end marker.

Yeah that's right... But all other lines (longer) read properly or maybe I'm losing something I don't know about. But at cursory glance you may be right.

Juraj:
yes

the working sketch is SerialPassthrough sketch

SoftwareSerial is on edge with 38400 baud

I didn't know this. because this is only for debugging I can change the baud rate to whatever I want.

13

kmarts:
Yeah that's right... But all other lines (longer) read properly or maybe I'm losing something I don't know about. But at cursory glance you may be right.

I didn't know this. because this is only for debugging I can change the baud rate to whatever I want.

sorry. radio is Serial. I had the impression radio is SoftwareSerial. it was not clear in original post's code

what is the baud rate set at the connected device?

14

Juraj:
sorry. radio is Serial. I had the impression radio is SoftwareSerial. it was not clear in original post's code

what is the baud rate set at the connected device?

38400, but the modem is connected to the hardware UART.

15

could the debug print of the result fail on SoftwareSerial? try 9600 baud

16

cattledog:

if (radio.available()) {

inByte = radio.read();
      if (inByte == '\r' || inByte == '\n') {
        if (bytesRead == 0) {
          //buffer is empty, restart
          continue;
        }
        else if (bytesRead > 0) {
          //we have data, end
          break;
        }
      }
      buf[bytesRead++] = inByte;
    }




I think that the break at the end of reading is taking you outside of the ` for(;;) ` loop and you are not getting the bytesRead index increment. Then, you are overwriting the last character received before the \r or \n when you use

if (buf) {
    buf[bytesRead] = '\0';
  }

After going over it this isn't the problem, this is working as intended. Because i don't want the bytesRead incremented if i hit a cr or lf. It gets incremented after any other character is added to the array which is where i want the null terminator after the break.

Paul was right about the delay at the beginning. That's something that i put there while testing the function when it wasn't in the for(;:wink: loop for reliability. With the infinite loop and timeout it isn't needed because it will keep reading. After i removed it, i started getting what i expected. I'm not sure why i could read incoming data that was longer but in this case removing the delay fixed it.

17

kmarts:
After going over it this isn't the problem, this is working as intended. Because i don't want the bytesRead incremented if i hit a cr or lf. It gets incremented after any other character is added to the array which is where i want the null terminator after the break.

Paul was right about the delay at the beginning. That's something that i put there while testing the function when it wasn't in the for(;:wink: loop for reliability. With the infinite loop and timeout it isn't needed because it will keep reading. After i removed it, i started getting what i expected. I'm not sure why i could read incoming data that was longer but in this case removing the delay fixed it.

rx buffer overflow. I tested the code with the delay(1) with data sent from Serial Monitor, but I made an error. I sent only the last line, not both lines