[solved] Serial.readStringUntil does not work correctly in my code

[palhares]

Hello All.

I'm learning Arduino, and I'm making a code of one traffic light, to control the time of the LEDs using the serial monitor, through a keyword and the time for each LED to stay on, the code for some reason only works when I try GREEN on serial, when I try to put a value on YELLOW or RED, it doesn't work

#include <SD.h>
#include <SPI.h>
#include <Wire.h>
//============================================================================================
String timeLed_Green_Serial;
String timeLed_Yellow_Serial;
String timeLed_Red_Serial;
//============================================================================================
int led_green_trafficLight;
int led_yellow_trafficLight;
int led_red_trafficLight;
//============================================================================================
void setup () {

  Serial.begin(9600);
  delay(1000);
}
//============================================================================================

void funcGREEN() /// GREEN

{
  timeLed_Green_Serial.remove(0, 5);
  led_green_trafficLight = timeLed_Green_Serial.toInt();
}

void funcYELLOW() /// YELLOW

{
  timeLed_Yellow_Serial.remove(0, 6);
  led_yellow_trafficLight = timeLed_Yellow_Serial.toInt();
}

void funcRED() /// RED

{
  timeLed_Red_Serial.remove(0, 3);
  led_red_trafficLight = timeLed_Red_Serial.toInt();
}

//============================================================================================
void trafficLight() {

  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

  digitalWrite(2, HIGH);
  digitalWrite(3, LOW);
  digitalWrite(4, LOW);

  delay(led_green_trafficLight);      // GREEN LED

  digitalWrite(2, LOW);
  digitalWrite(3, HIGH);
  delay(led_yellow_trafficLight);       // YELLOW LED

  digitalWrite(3, LOW);
  digitalWrite(4, HIGH);
  delay(led_red_trafficLight);       // RED LED

}
//============================================================================================

void loop () {

  trafficLight();

  if (Serial.available() > 0)
  {
    timeLed_Green_Serial = Serial.readStringUntil('\n');
    timeLed_Yellow_Serial = Serial.readStringUntil('\n');
    timeLed_Red_Serial = Serial.readStringUntil('\n');

    if (timeLed_Green_Serial.startsWith("GREEN"))
    {
      funcGREEN();
    }

    if (timeLed_Yellow_Serial.startsWith("YELLOW"))
    {
      funcYELLOW();
    }

    if (timeLed_Red_Serial.startsWith("RED"))
    {
      funcRED();
    }
  }

  //============================================================================================

  delay(1000);
  Serial.println("Green = " + timeLed_Green_Serial);
  delay(1000);
  Serial.println("Yellow = " + timeLed_Yellow_Serial);
  delay(1000);
  Serial.println("Red = " + timeLed_Red_Serial);
  delay(1000);
}

I tried to make the code as short as possible, and it still got a little long, sorry

[J-M-L]

why are you reading the Serial input 3 times?

@palhares

    timeLed_Green_Serial = Serial.readStringUntil('\n');
    timeLed_Yellow_Serial = Serial.readStringUntil('\n');
    timeLed_Red_Serial = Serial.readStringUntil('\n');

read only once and then test against the 3 possible cases.

we would advise against Serial.readStringUntil() and would suggest to study Serial Input Basics to understand how to deal with Serial input

[palhares]

thanks, i tried to read the serial only once, but when i do that i realized that only the first if of the green works, even though i insert in the serial YELLOW for example

#include <SD.h>
#include <SPI.h>
#include <Wire.h>
//============================================================================================
String timeLeds_Serial;

//============================================================================================
int led_green_trafficLight;
int led_yellow_trafficLight;
int led_red_trafficLight;
//============================================================================================
void setup () {

  Serial.begin(9600);
  delay(1000);
}
//============================================================================================

void funcGREEN() /// GREEN

{
  timeLeds_Serial.remove(0, 5);
  led_green_trafficLight = timeLeds_Serial.toInt();
}

void funcYELLOW() /// YELLOW

{
  timeLeds_Serial.remove(0, 6);
  led_yellow_trafficLight = timeLeds_Serial.toInt();
}

void funcRED() /// RED

{
  timeLeds_Serial.remove(0, 3);
  led_red_trafficLight = timeLeds_Serial.toInt();
}

//============================================================================================
void trafficLight() {

  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);

  digitalWrite(2, HIGH);
  digitalWrite(3, LOW);
  digitalWrite(4, LOW);

  delay(led_green_trafficLight);      // GREEN LED

  digitalWrite(2, LOW);
  digitalWrite(3, HIGH);
  delay(led_yellow_trafficLight);       // YELLOW LED

  digitalWrite(3, LOW);
  digitalWrite(4, HIGH);
  delay(led_red_trafficLight);       // RED LED

}
//============================================================================================

void loop () {

  trafficLight();

  if (Serial.available() > 0)
  {
    timeLeds_Serial = Serial.readStringUntil('\n');


    if (timeLeds_Serial.startsWith("GREEN"))
    {
      funcGREEN();
    }

    if (timeLeds_Serial.startsWith("YELLOW"))
    {
      funcYELLOW();
    }

    if (timeLeds_Serial.startsWith("RED"))
    {
      funcRED();
    }
  }

  //============================================================================================

  delay(1000);
  Serial.println("Green = " + led_green_trafficLight);
  delay(1000);
  Serial.println("Yellow = " + led_yellow_trafficLight);
  delay(1000);
  Serial.println("Red = " + led_red_trafficLight);
  delay(1000);
}

and sometimes a strange error happens, I'll send a print, in this case I put it in the serial YELLOW20000

[groundFungus]

+1 for the serial input basics tutorial.

[palhares]

I will study the tutorial, thanks guys

[J-M-L]

what's your protocol? what do you send to the Serial line?

[groundFungus]

The serial input basics methods that return multiple characters return strings (null terminated character arrays), not Strings (of the String class). It is recommenced to not use the String class with the Arduinos with small amounts of SRAM as memory problems can occur. See the evils of Strings.

That means that you cannot use equality (==) to compare strings and you cannot use + to concatenate strings. There are a set of functions to do those things with strings. See HERE.

[palhares]

I managed to solve it, really I should have read the serial read only once, it worked, that error above was some isolated bug, thanks

[palhares]

probably the bugs are occurring because of the strings, I'll study about it, thanks

[groundFungus]

@palhares

of the strings

Did you mean because of the Strings. The string and String are not interchangeable terms. It may seem pedantic, but the 2 are very different.

[palhares]

got it, I shouldn't use the String class then

[groundFungus]

In case you may be interested here is a demo of the serial input basics read with end marker and using the strtok function to read and parse your color and time data from serial monitor. The code uses the strcmp function to compare the incoming color.

const byte numChars = 32;
char receivedChars[numChars];   // an array to store the received data

boolean newData = false;

// you should use unsigned long for timing variables
unsigned long led_green_trafficLight;
unsigned long led_yellow_trafficLight;
unsigned long led_red_trafficLight;

void setup()
{
   Serial.begin(115200);
   Serial.println("<Arduino is ready>  Enter a color and a time separated by a space");
   Serial.println("Like YELLOW 20000");
   Serial.println("make sure newline is enabled in serial monitor line endings");
}

void loop()
{
   recvWithEndMarker();
   //showNewData();
   if (newData)
   {
      parseData();
      Serial.print("Green = ");
      Serial.println(led_green_trafficLight);
      Serial.print("Yellow = ");
      Serial.println(led_yellow_trafficLight);
      Serial.print("Red = ");
      Serial.println(led_red_trafficLight);
      newData = false;
   }
}

void recvWithEndMarker()
{
   static byte ndx = 0;
   char endMarker = '\n';
   char rc;

   while (Serial.available() > 0 && newData == false)
   {
      rc = Serial.read();
      if (rc == '\r') // ignore carruage return
      {
         return;
      }
      if (rc != endMarker)
      {
         receivedChars[ndx] = rc;
         ndx++;
         if (ndx >= numChars)
         {
            ndx = numChars - 1;
         }
      }
      else
      {
         receivedChars[ndx] = '\0'; // terminate the string
         ndx = 0;
         newData = true;
      }
   }
}

void showNewData()
{
   if (newData == true)
   {
      Serial.print("This just in ... ");
      Serial.println(receivedChars);
      //newData = false;
   }
}

void parseData()
{
   char *strings[3]; // an array of pointers to the pieces of the above array after strtok()
   char *ptr = NULL; byte index = 0;
   ptr = strtok(receivedChars, " ");  // delimiters, space
   while (ptr != NULL)
   {
      strings[index] = ptr;
      index++;
      ptr = strtok(NULL, " ");
   }

   /*
      //Serial.println(index);
      // print all the parts
      Serial.println("The Pieces separated by strtok()");
      for (int n = 0; n < index; n++)
      {
      Serial.print("piece ");
      Serial.print(n);
      Serial.print(" = ");
      Serial.println(strings[n]);
      }
   */

   // convert string data to numbers
   char* color = strings[0];
   unsigned long time = atol(strings[1]);
   Serial.print("color = ");
   Serial.print(color);
   Serial.print("   time = ");
   Serial.print(time);
   Serial.println(); // blank line

   if (strcmp(color, "YELLOW") == 0)
   {
      led_yellow_trafficLight = time;
   }
   else
   {
      led_yellow_trafficLight = 0;
   }
   if (strcmp(color, "GREEN") == 0)
   {
      led_green_trafficLight = time;
   }
   else
   {
      led_green_trafficLight = 0;
   }
   if (strcmp(color, "RED") == 0)
   {
      led_red_trafficLight = time;
   }
   else
   {
      led_red_trafficLight = 0;
   }
   //newData = false;
}

[palhares]

thanks, i will study and implement this in my code! =)

[StefanL38]

I recommend using the SafeString-libary
It offers (most) of the comfort of Strings (variable-type with a capital "S"
and is even more secure than c_strings (in short string with a lowcase "s".

GitHub - PowerBroker2/SafeString: This SafeString library is designed for...

best regards Stefan

[J-M-L]

@StefanL38

and is even more secure than c_strings

It’s written using cStrings… so can’t be more secure than cStrings… it’s just that the developer was careful with memory overflow - as you could be without this large library

I recommend understanding cStrings and memory constraints And then pick what suits your case best.

[palhares]

thanks for all the answers and for the library suggestion, i'll analyze which one will be better in my case, anyway it will be useful to learn how to use cstrings