For-Loop Stuck between Serial.println((String)) sometimes

I was attempting to Serial Read using readBytesUntil and parsing them by commas within a function. The parsed char array will be returned to the main function using pointers and individually converted to int using atoi for other variable manipulation.

But I'm just wondering why Serial Printing currentChannelValue cause the For-Loop to break halfway, as shown in the image (Print becomes corrupted on the 8th loop in this instance).
Screenshot 2021-12-02 162553

void setup() {
   Serial.begin(9600);
 }
 
 void loop() {
   char buffer[33];
   char *slots[33];
   int charsRead;
   int channelLoop;
   int sizeofdata;
   
     if (Serial.available() > 0) {
 
       charsRead = Serial.readBytesUntil('\n', buffer, sizeof(buffer) - 1);
       buffer[charsRead] = '\0';
       
       sizeofdata = DoBreakup(buffer, ",", slots);
       Serial.println(sizeofdata);
       Serial.println();
 
       for(channelLoop = 0; channelLoop < sizeofdata ; channelLoop++)        // Expected to loop 11 times
       {
         int channelRegister = 0;
         int currentChannelValue = 0;
         const char *bufferCharToInt[2];
         
         bufferCharToInt[channelLoop] = slots[channelLoop];
         bufferCharToInt[channelLoop + 1] = '\0';
         
         currentChannelValue = atoi(bufferCharToInt[channelLoop]);
          
          // Loop stops halfway whenever these three lines are added
          Serial.println((String)"Slots Value: " + slots[channelLoop]);
          Serial.println((String)"Conversion Buffer: " + bufferCharToInt[channelLoop]);
          Serial.println((String)"Current Channel: " + currentChannelValue);                               

         char binary[5] = {0};                     
         channelRegister = channelLoop + 32;      
         itoa(channelRegister,binary,2);             
         char* string = binary + 1;                   
         Serial.println(string);
         Serial.println();
       }
       memset(slots, 0, sizeof(slots));
       delay(1);                            
   } 
 }
 
 
 
 uint8_t DoBreakup(char s[], char delimit[], char *ptr[])
 {
     char *p;
     int i = 0;
     
     p  = strtok(s, delimit);
     while (p) {
         ptr[i++] = p;        
         p = strtok(NULL, delimit);  
     }
     return i;
 }

The Serial.println((String)) will not be necessary for the final assembly as I'm just using it for troubleshooting now, but it is still a curious thing, the memory used is 11% of dynamic memory for now.

you only have room for 2 char pointers in bufferCharToInt yet you use channelLoop as an index which can (does) go way above 1...

➜ you write beyond the bounds of your pointer array (which has no use probably) and thus you corrupt your memory. From there all bets are off.

I would suggest to study Serial Input Basics to better handle Serial input and not rely on readBytesUntil which can timeout and give you an incomplete view of what was coming in depending on your circumstances.

PS: may be you meant

         bufferCharToInt[0] = slots[channelLoop];
         bufferCharToInt[1] = '\0';

but I don't know why you would need this, just perform your atoi() on slots[channelLoop].

PS2: strtol() is better than atoi() as you can tell if the parsing was successful.

1 Like

Don't use the String type on controllers with little RAM.

1 Like

Oh right! Sorry for the obvious mistake, not too sure why I didn't spot that logic error :confused: but it works after replacing the indexing.

Thanks for the Serial Input Basic & strtol() references too, will check them out too.

Yup thanks! I am aware of the heap issue using String type, mainly using it now for labelling the things I'm printing for troubleshooting; it will be removed later on.

Could I just check why the Serial.Read sometimes do not register things that are printed on the Serial Monitor? i.e., I have to send the serial data twice before it recognizes the startMarker and runs the other blocks of code.

const byte numChars = 33;
char receivedChars[numChars];
boolean processingNewData = false;
boolean recvAnything = false;

void setup() {
  Serial.begin(9600);
}

void loop() {
  
  char *slots[33];
  int charsRead;
  int channelLoop;
  int sizeofdata;
  char SPIChannelSends;

  recvWithEndMarker();
  if(recvAnything && processingNewData)
  {
      sizeofdata = DoBreakup(receivedChars, ",", slots);
      Serial.println(sizeofdata);
      Serial.println();

        for(channelLoop = 0; channelLoop < sizeofdata ; channelLoop++)
        {
            int channelRegister = 0;
            long currentChannelValue;
            char *postValueBuffer;
            const char *bufferCharToInt[2];
            
            bufferCharToInt[0] = slots[channelLoop];
            bufferCharToInt[1] = '\0';

            currentChannelValue = strtol(bufferCharToInt[0], &postValueBuffer, 10);
            Serial.println((String)"Slots Value: " + slots[channelLoop]);
            Serial.println((String)"Conversion Buffer: " + bufferCharToInt[0]);
            Serial.println(currentChannelValue);
            
            char binary[5] = {0};                  
            channelRegister = channelLoop + 32 + 1;       
            Serial.println(channelRegister);
            itoa(channelRegister,binary,2);             
            char* string = binary + 1;              
            Serial.println(string);
            Serial.println();
            delay(1);                                     
        }
        processingNewData = false;
        recvAnything = false;
    }
    memset(slots, 0, sizeof(slots));
}

void recvWithEndMarker() {
  static boolean recvInProgress = false;
  static byte ndx = 0;
  char startMarker = '<';
  char endMarker = '>';
  char rc;
  
  if (processingNewData && recvAnything)
    Serial.println("Connected!");
   
  while (Serial.available() > 0 && processingNewData == false) {
    rc = Serial.read();
    
    if (rc == startMarker) {
        recvInProgress = true;
        recvAnything = true;
    }
    
    else if (recvInProgress && recvAnything) {
        if (rc != endMarker) {
            receivedChars[ndx] = rc;
            ndx++;
            if (ndx >= numChars) {
                ndx = numChars - 1;
            }
        }
        else {
            receivedChars[ndx] = '\0'; // terminate the string
            recvInProgress = false;
            ndx = 0;
            processingNewData = true;
        }
    }
  } 
}

uint8_t DoBreakup(char s[], char delimit[], char *ptr[])
{
    char *p;
    int i = 0;
    
    p  = strtok(s, delimit);
    while (p) {
        ptr[i++] = p;
        
        //sizeofdata = i;
        //Serial.println(p);
        //Serial.println(ptr[i]);
        //Serial.println(slots);
        //Serial.println(sizeofdata);
        
        p = strtok(NULL, delimit);  
    }
    ptr[i + 1] = '\0';
    return i;
}

Sorry it was modified a bit from the Serial Input Tutorial.

The code is not complete, so can't test.

what do you send exactly? how is the Serial monitor configured - ie are you sending any end of line characters?

Sorry about that, just added DoBreakup() foo.

In this case, I would send <24,53,60,2,5,21,32,46,35,54,48>, < is the Start Marker, > is the End Marker, , is the delimiter.

For now, it's set as 'New Line.'

try with this code - typed here from your code, fully untested

const byte numChars = 33;
char message[numChars];
char *slots[numChars];
boolean dataReady = false;

byte DoBreakup(char s[], const char* delimit)
{
  byte count = 0;
  char *p  = strtok(s, delimit);
  while (p && count < numChars) {
    slots[count++] = p;
    p = strtok(NULL, delimit);
  }
  return count;
}

void recvWithEndMarker() {
  static boolean recvInProgress = false;
  static byte ndx = 0;
  const char startMarker = '<';
  const char endMarker = '>';


  while ((Serial.available() > 0) && (dataReady == false)) {
    char rc = Serial.read();

    if (rc == startMarker) {
      ndx = 0;
      recvInProgress = true;
    } else if (recvInProgress) {
      switch (rc) {
        case '\r': break; // ignore
        case '\n': break; // ignore
        case endMarker:
          message[ndx] = '\0'; // terminate the string
          recvInProgress = false;
          ndx = 0;
          dataReady = true;
          break;

        default:
          message[ndx++] = rc;
          if (ndx >= numChars)ndx = numChars - 1;
          break;
      }
    }
  }
}

void setup() {
  Serial.begin(115200);
}

void loop() {
  int channelLoop;
  byte sizeofdata;

  recvWithEndMarker();
  if (dataReady) {
    Serial.print("Parsing : <"); Serial.print(message); Serial.println(">");
    sizeofdata = DoBreakup(message, ",");
    Serial.print("Identified "); Serial.print(sizeofdata); Serial.println(" tokens.");
    for (channelLoop = 0; channelLoop < sizeofdata ; channelLoop++)     {
      char *endPtr;
      long currentChannelValue = strtol( slots[channelLoop], &endPtr, 10);
      Serial.print("Channel: "); Serial.print(channelLoop + 1);
      Serial.print(" -> Slots Value: "); Serial.print(currentChannelValue);
      Serial.print("\t(from parsing: "); Serial.print(slots[channelLoop]);
      if (endPtr != slots[channelLoop] + strlen(slots[channelLoop])) Serial.print(" --> WRONG INPUT");
      Serial.println(")");
    }
    dataReady = false;
  }
}

if you enter something like <24,53,xxx,5,21,32,46,35,54,48> the code should flag the xxx input as being incorrect

1 Like

Hi @J-M-L, thanks for the revised code. Just for my understanding, both codes seems to still suffer from a lack of serial reading if the serial data is sent to the Serial.Monitor almost immediately (~1s) after Uploading.

But otherwise, it seems to be ok, so it was probably due to the inherent Bootloader/startup lead time of the Arduino Uno.

Anyways thanks so much for your guidance man, really appreciate it, sorry about the rookie mistakes :sweat_smile:.

When you open the serial monitor the arduino reboots. So Indeed needs a bit of time to get fully ready. If this is critical for you because for example there is no human intervention then either add a small pause in the program connecting to the arduino or establish a handshake (the caller waits for some OK message from the arduino before dumping data onto the Serial line)

1 Like