Strings vs c strings and strange behavior

Using Arduino Programming Questions
1

I'm trying to chase down why, when I run these commands, my code hangs on my Mega 2560 and I'm wondering if it is because I'm using Strings instead of cstrings?

I'm using Strings because I'm familiar with that due to a .net background and I'm reluctant to switch to cstrings due to unfamiliarity and what appears to be the need to define the array size in advance, however I've read where people have seen strange behavior from Strings and if the following behavior is an example of that?

When I run the commands below and uncomment a line inside of loop() ( Serial.println(" end of (serialInString.startsWith(write:))"); ) the program will freeze up. When I comment it out, the program works as expected.

Does this appear to be due to using Strings? If so, how hard is it to convert over everything to cstrings?

Commands

write:weighttarget=12
read:weighttarget
write:weighttarget=14
read:weighttarget

Code

#include <EEPROM.h>
#include <SoftwareSerial.h>
#include <TimeLib.h>
#include <string.h>
#include <TinyGPS++.h>
#include <Time.h> 

//SoftwareSerial BTSerial(12, 13);   // RX , TX
#define BTSerial Serial1

String serialInString;
boolean newData = false;
boolean newBtData = false;

unsigned long btLastSendTimeStamp;


bool isSpecialChar(char chr)
{
    if (chr == ':')
    {
        return true;
    }
    else if (chr == '=')
    {
        return true;
    }
    else if (chr == '-')
    {
        return true;
    }
    else
    {
        return false;
    }
}

bool validateDataType(String dataType, char* data) // true is indeed 1, and false is 0.
{
    if (dataType == "string")
    {
        //String stringToTest = "isthisanactualstring";
        //for (int i = 0; i < stringLength; i++)
        for (int i = 0; i < strlen(data); i++)
        {
            if (isAlpha(data[i]))
            {
                // continue
                //Serial.print(i);
                //Serial.print(" - ");
                //Serial.println(data[i]);
            }
            else
            {
                //Serial.print(i);
                //Serial.print("? - ");
                //Serial.println(data[i]);
                return false;
            }
        }
        return true;
    }    
    else if (dataType == "int")
    {
        for (int i = 0; i < strlen(data); i++)
        {
            if (isDigit(data[i]))
            {
                // continue
                //Serial.print(i);
                //Serial.print(" - ");
                //Serial.println(data[i]);
            }
            else
            {
                //Serial.print(i);
                //Serial.print("? - ");
                //Serial.println(data[i]);
                return false;
            }
        }
        //Serial.println("About to return true for int");
        return true;
    }
    else if (dataType == "float")
    {
        int decimalPointCount = 0;
        for (int i = 0; i < strlen(data); i++)
        {
            if (isDigit(data[i]))
            {
                // continue
                //Serial.print(i);
                //Serial.print(" - ");
                //Serial.println(data[i]);
            }
            else if (data[i] == '.')
            {
                decimalPointCount++;
                if (decimalPointCount > 1)
                {
                    //Serial.print(i);
                    //Serial.print("? - ");
                    //Serial.println(data[i]);
                    return false;
                }
                //Serial.print(i);
                //Serial.print(" - ");
                //Serial.println(data[i]);
            }
            else
            {
                //Serial.print(i);
                //Serial.print("? - ");
                //Serial.println(data[i]);
                return false;
            }
        }

        if (decimalPointCount != 1)
        {
            return false;
        }
        else
        {
            return true;
        }
    }
    else if (dataType == "time")
    {
        Serial.println("dataType == 'time'");
        Serial.print("data: "); Serial.println(data);
        int colonCount = 0;
        for (int i = 0; i < strlen(data); i++)
        {
            if (isDigit(data[i]))
            {
                // continue
                Serial.print(i);
                Serial.print(" - ");
                Serial.println(data[i]);
            }
            else if (data[i] == ':')
            {
                colonCount++;
                if (colonCount > 1) // will need to make it > 2 if including seconds
                {
                    //Serial.print(i);
                    //Serial.print("? - ");
                    //Serial.println(data[i]);
                    return false;
                }
                Serial.print(i);
                Serial.print(" - ");
                Serial.println(data[i]);
            }
            else
            {
                Serial.print(i);
                Serial.print("? - ");
                Serial.println(data[i]);
                return false;
            }
        }

        if (colonCount != 1) // will need to make it > 2 if including seconds
        {
            return false;
        }
        else
        {
            return true;
        }
    }
} // validateDataType

int getEePromAddress(String settingName)
{
    int returnValue;
    //Serial.print("getEePromAddress() settingName: "); Serial.println(settingName);
    if (settingName == "automation")
    {
        returnValue = 20;
    }
    else if (settingName == "empty")
    {
        returnValue = 30;
    }
    else if (settingName == "full")
    {
        returnValue = 40;
    }
    else if (settingName == "alldays")
    {
        returnValue = 50;
    }
    else if (settingName == "alldaystime")
    {
        returnValue = 60;
    }
    else if (settingName == "sundays")
    {
        returnValue = 70;
    }
    else if (settingName == "sundaytime")
    {
        returnValue = 80;
    }
    else if (settingName == "mondays")
    {
        returnValue = 90;
    }
    else if (settingName == "mondaytime")
    {
        returnValue = 100;
    }
    else if (settingName == "tuesdays")
    {
        returnValue = 110;
    }
    else if (settingName == "tuesdaytime")
    {
        returnValue = 120;
    }
    else if (settingName == "wednesdays")
    {
        returnValue = 130;
    }
    else if (settingName == "wednesdaytime")
    {
        returnValue = 140;
    }
    else if (settingName == "thursdays")
    {
        returnValue = 150;
    }
    else if (settingName == "thursdaytime")
    {
        returnValue = 160;
    }
    else if (settingName == "fridays")
    {
        returnValue = 170;
    }
    else if (settingName == "fridaytime")
    {
        returnValue = 180;
    }
    else if (settingName == "saturdays")
    {
        returnValue = 190;
    }
    else if (settingName == "saturdaytime")
    {
        returnValue = 200;
    }
    else if (settingName == "chime")
    {
        returnValue = 210;
    }
    else if (settingName == "weighttarget")
    {
        returnValue = 220;
    }
    else
    {
        returnValue = -1;
    }

    
    return returnValue;
}

String eePromReadAllAddresses()
{
    String returnValue = "";
    String s_EEPromValue = "";

    int startAddress = 20;
    int endAddress = 210;
    int addressPadding = 10;
    for (int i = startAddress; i <= endAddress; i = (i + addressPadding))
    {
         String s_eepromReadAll = eePromReadAnyType(i);
        s_EEPromValue = eePromReadAnyType(i);
        //Serial.print(i); Serial.print(" s_EEPromValue: "); Serial.println(s_EEPromValue);
        returnValue += s_EEPromValue + "|";
    }

    return returnValue;
}

String eePromReadAnyType(int eeAddress)
{
    //Serial.print("function: "); Serial.println("eePromReadAnyType");
    int eeAddressIndexer = eeAddress;
    String returnValue;
    char chr;
    int maxeeAddressToReadTo = (eeAddress + 10);
    //Serial.print("eeAddressIndexer: "); Serial.println(eeAddressIndexer);
    //Serial.print("maxeeAddressToReadTo: "); Serial.println(maxeeAddressToReadTo);

    do 
    {
        chr = EEPROM.read(eeAddressIndexer);
        if ((chr != '\0') && ((isAlphaNumeric(chr)) || isSpecialChar(chr)))
        {
            //Serial.print("eeAddressIndexer: "); Serial.println(eeAddressIndexer);
            //Serial.print("eePromReadAnyType: "); Serial.println(chr);
            returnValue += chr;
        }

        eeAddressIndexer = (eeAddressIndexer + 1);
    } while ((chr != '\0') && (eeAddressIndexer <= maxeeAddressToReadTo));

    return returnValue;
}

String getSettingValue(String data, char separator, int index)
{
    int found = 0;
    int strIndex[] = { 0, -1 };
    int maxIndex = data.length() - 1;

    for (int i = 0; i <= maxIndex && found <= index; i++) 
    {
        //Serial.println(data.charAt(i));
        if (data.charAt(i) == separator || i == maxIndex) 
        {
            found++;
            strIndex[0] = strIndex[1] + 1;
            strIndex[1] = (i == maxIndex) ? i+1 : i;
        }
    }
    return found > index ? data.substring(strIndex[0], strIndex[1]) : "";
} // getSettingValue

void eePromWriteAnyType(int eeAddress, String eeValue)
{
    Serial.print("eepromWriteAnyType"); Serial.print(" -- eeAddress: "); Serial.print(eeAddress); Serial.print(", eeValue: "); Serial.println(eeValue);

    char c_eeValue[eeValue.length()]; // instantiate character array
    eeValue.toCharArray(c_eeValue, (eeValue.length() + 1));
    Serial.print("c_eeValue: "); Serial.println(c_eeValue);
    int indexer = 0;
    for (int i = 0; i < strlen(c_eeValue); i++)
    {
        //Serial.println(c_eeValue[i]);
        Serial.print("char: "); Serial.print(c_eeValue[i]); Serial.print(", written to address: "); Serial.println(eeAddress + i);
        EEPROM.write((eeAddress + i), c_eeValue[i]);
        indexer = (eeAddress + i);
    }

    Serial.print("slash 0 written to address: "); Serial.println(indexer + 1);
    EEPROM.write((indexer + 1), '\0');
    Serial.println("Exiting function eePromWriteAnyType()");
}

void setup() 
{
    Serial.begin(9600);
    BTSerial.begin(9600);
    while (!Serial)
    {
      ; // Wait for serial to connect
    }

    while (!BTSerial)
    {
      ; // Wait for BTSerial to connect
    }
   
    delay(2000);

    Serial.println("");
    Serial.println("    ***** Pet Assistant *****    ");
    Serial.println("");

    btLastSendTimeStamp = millis();
}

void loop() 
{
    while(Serial.available()>0 && newData == false)
    { 
        serialInString = Serial.readStringUntil('\n\r');
        Serial.print("New serial data: "); Serial.println(serialInString);        
        newData = true;

        delay(15);
        
        if (serialInString.equals("readall"))
        {
            Serial.println("command = readall");
            Serial.println(eePromReadAllAddresses());
        }

        if (serialInString.startsWith("read:"))
        {
            Serial.print("startsWith(read:) :"); Serial.println(serialInString);
            String s_settingNameToRead = getSettingValue(serialInString, ':', 1);
            Serial.print("s_settingNameToRead: "); Serial.println(s_settingNameToRead);

            int getEepromAddr = getEePromAddress(s_settingNameToRead);
            Serial.print("getEepromAddr: "); Serial.println(getEepromAddr);
            String s_eepromReadAnyType = eePromReadAnyType(getEepromAddr);
            Serial.print("s_eepromReadAnyType: "); Serial.println(s_eepromReadAnyType);
        }

        if (serialInString.startsWith("write:"))
        {
            Serial.print("startsWith(write:) : "); Serial.println(serialInString);

            String s_settingToWriteUnparsed = getSettingValue(serialInString, ':', 1);
            Serial.print("s_settingToWriteUnparsed: "); Serial.println(s_settingToWriteUnparsed);

            String s_settingNameToWrite = getSettingValue(s_settingToWriteUnparsed, '=', 0);
            Serial.print("s_settingNameToWrite: "); Serial.println(s_settingNameToWrite);
            String s_settingValueToWrite = getSettingValue(s_settingToWriteUnparsed, '=', 1);
            Serial.print("s_settingValueToWrite: "); Serial.println(s_settingValueToWrite);

            int getEepromAddr = getEePromAddress(s_settingNameToWrite);
            if (getEepromAddr != -1)
            {
                eePromWriteAnyType(getEepromAddr, s_settingValueToWrite);
                Serial.println("right after eePromWriteAnyType");
                //String s_eepromReadAnyType = eePromReadAnyType(getEepromAddr);
                //Serial.print("s_eepromReadAnyType: "); Serial.println(s_eepromReadAnyType);
            }

            Serial.println(" end of (serialInString.startsWith(write:))"); // somehow this hoses the entire setup function and the code never starts
        }
        
        if (serialInString.startsWith("validate:"))
        {
            Serial.print("startsWith(validate:) :"); Serial.println(serialInString);
            String s_settingDataTypeAndValue = getSettingValue(serialInString, ':', 1);
            Serial.print("s_settingDataTypeAndValue: "); Serial.println(s_settingDataTypeAndValue);

            String s_dataType = getSettingValue(s_settingDataTypeAndValue, '=', 0);
            Serial.print("s_dataType: "); Serial.println(s_dataType);
            String s_dataValue = getSettingValue(s_settingDataTypeAndValue, '=', 1);
            Serial.print("s_dataValue: "); Serial.println(s_dataValue);

            validateDataType(s_dataType, s_dataValue.c_str());
            Serial.print("validateDataType: "); Serial.println(validateDataType(s_dataType, s_dataValue.c_str()));
        }

        newData = false;
        Serial.println("Exiting while(Serial.available()>0 && newData == false)");
    }


    delay(5);

    newData = false;
    newBtData = false;

} // loop
2

The problem with Strings is that poor memory management leads to unpredictable program crashes, especially on AVR based Arduinos. The best approach is to avoid using Strings.

C-strings (zero terminated character arrays) and their associated functions are are faster, much more reliable and use less program memory. There are countless tutorials on line describing to use them, along with this excellent reference guide.

3

Thanks for the link. Does my issue sound like a memory management issue? Did you try to repro it?

4

Freezing is commonly a result of memory fragmentation, but there are other possible causes, like running out of RAM due to all of those string constants.

Your code is a mishmash of the two different approaches and is hard to follow.

Check out the C/C++ enumerated data type "enum", which takes vastly less program and data memory than the following, and is just as easy to read:

    if (settingName == "automation")
    {
        returnValue = 20;
    }
    else if (settingName == "empty")
    {
        returnValue = 30;
    }

Replace all that with something like this:

enum settings { automation, empty, full, /*etc.*/) };

settings action = empty;
 
switch(action)
{
    case automation: 
	{
	returnValue = 20;
	break;
	}
	
    case empty: 
	{
	returnValue = 30;
	break;
	}
    // etc...
}
5

And after you are using an enum, which means your settingName will be a number, you can replace the lengthy if/else chain with an array of return values.

    returnValue = valueArray[settingName];

which line of code implies

enum settings { automation = 0, empty, full, /*etc.*/) };

settings settingName;

int valueArray[] = {20, 30, // and so forth

which as I type it makes me wonder if there isn't an even simpler mathematical relationship like

     returnValue = 20 + 10 * settingName;

which is less flexible, and I didn't look far enough to see if the pattern breaks, just sayin'.

You could name things different/better than I did.

Haha, about on the problem you are asking about, I learned real small s strings first and never use capital S strings, and probably wouldn't even if there weren't reasons, so I can't help w/ that.

Other than as we point out how variously to code without them.

a7

6

I will look into enum.

I see Strings vs strings a lot when I search. When I write in c#, I use string = "test"; However I don't think that's the case with c++ . When you say 'strings' do you mean cstrings or char arrays?

7

This will not work as you expect, the parameter to readStringUntil() should be a single character.

Are you intending to use the standard time.h library file, or do you have a specific library with the Time.h header file?

8

I'll check out enum. I'm trying to merge my experience (.net) with c++, which is probably why it's an amalgamation (poor one) of approaches.

I'm trying to get off of the ground with this and when I type "test" into the Message spot on the Arduino IDE, I get the below output. How can I read the serial stream into a cstring?


char serialInCstring = '\0';
void loop() 
{   
    while(Serial.available() > 0 && newData == false)
    {
        char chr = Serial.read();
        Serial.println(chr);
        strcat(serialInCstring, chr);
        Serial.println(serialInCstring);

    }
}

2:09:58.363 -> t

22:09:58.363 ->|-|

22:09:58.363 -> e

22:09:58.363 ->|-|

22:09:58.363 -> s

22:09:58.363 ->|-|

22:09:58.363 -> t

22:09:58.405 ->|-|

22:09:58.405 ->

22:09:58.405 ->|-|

9

Please spend time here, one stop shopping:

HTH

a7

10 
void eePromWriteAnyType(int eeAddress, String eeValue)
{
  Serial.print("eepromWriteAnyType"); Serial.print(" -- eeAddress: "); Serial.print(eeAddress); Serial.print(", eeValue: "); Serial.println(eeValue);

  char c_eeValue[eeValue.length()]; // instantiate character array
  eeValue.toCharArray(c_eeValue, (eeValue.length() + 1));

The size of c_eeValue is too small. You need enough space for the text plus the terminating null. Even worse, in the arguments to toCharArray(), you have told the compiler c_eeValue is larger than you have declared it.

11

I had both #include <TimeLib.h> and #include <Time.h> from two different examples I was learning from. I am not using the Time.h example and I do not need that any longer. It is not a specific library of mine, that was how it was typed in the example, with the uppercase T.

12

So it should be like this then? I believe I had it like that originally and it didn't work, however it was working the way you quoted? I don't remember much else, but I'll change it back.

char c_eeValue[eeValue.length() + 1]; // instantiate character array
eeValue.toCharArray(c_eeValue, eeValue.length());

13

The Time library originally used Time.h, but later changed that to TimeLib.h, because of confusion with the standard c library that uses time.h. On an operation system that is not case-sensitive (such as Windows), there is no difference between Time.h and time.h as a file name, which could result in the compiler using the wrong library.

14

No, the size should be the same in both places:

  char c_eeValue[eeValue.length() + 1]; // instantiate character array
  eeValue.toCharArray(c_eeValue, (eeValue.length() + 1));
15

If someone can have some mercy on me, I've tried 40+ different possibilities over hours of time and none have worked and I really need a simple example based off of the below, to learn from and build off of.

char serialInCstring[1];
void loop()
{
    while(Serial.available() > 0 && newData == false)
    {
        char chr = Serial.read();
        if (strlen(serialInCstring) == 0)
        {
            serialInCstring = chr;
        }
        else if (strlen(serialInCstring) > 0)
        {
            char serialInCstringT[strlen(serialInCstring)] = serialInCstring;
            char serialInCstring[strlen(serialInCstringT) + 1];
            int i = 0;
            for (i = 0; i < strlen(serialInCstringT); i++)
            {
                serialInCstring[i] = serialInCstringT[i];
            }

            serialInCstring[i + 1] = chr;
        }
    }
}
16

I don't usually criticize code without actually answering the original questions, but...
Your code looks like JavaScript code, translated with as little thought as possible to C/C++/Arduino. While I myself will somewhat ashamedly claim to be able to "write C code in several languages", it's a particularly bad idea to write "big system" style code when moving to a very resource-constrained system like an Arduino, and it would probably do you good to figure out the preferred ways to do things in C. Hint: it won't involve passing around variable types as strings...

As for collecting serial data, I'm old-fashioned and like to have a nice line-oriented interface with some editing (at least BACKSPACE!), and then process the full line after it's been terminated with a newline character. This isn't ideal on a very-small memory system, either (you have to have space for a "full" line), but it nicely separates the driver issues from the lexical analysis and/or parsing.

That looks a bit like:

/*
   cliGetline
   Read a line of text from Serial into the internal line buffer.
   With echoing and editing!
   Non-blocking.  Returns 0 until end-of-line seen.
*/
uint8_t cliGetline (void)
{
  int c;

  c = Serial.read();
  switch (c) {
    case 127:
    case CTRL('H'):
      /*
         Destructive backspace: remove last character
      */
      if (inptr > 0) {
        Serial.print("\010 \010");
        linebuffer[--inptr] = 0;
      }
      break;
    case CTRL('R'):
      /*
         Ctrl-R retypes the line
      */
      Serial.print("\r\n");
      Serial.print(linebuffer);
      break;
    case CTRL('U'):
      /*
         Ctrl-U deletes the entire line and starts over.
      */
      Serial.println("XXX");
      cliReset();
      break;
    case CTRL('J'):
    case CTRL('M'):
      linebuffer[inptr++] = '\n';
      Serial.println();     /* Echo newline too. */
      return inptr;
    case -1:
      /*
         No character present; don't do anything.
      */
      return 0;
    default:
      /*
         Otherwise, echo the character and put it into the buffer
      */
      linebuffer[inptr++] = c;
      Serial.write(c);
  }
  return 0;
}

/*
    cliGetlineWait
    like cliGetline, bu block until a complete line is read
*/

uint8_t cliGetlineWait (void)
{
  uint8_t status;
  do {
    status = cliGetline();
  } while (status == 0);
  return status;
}

Then I separate out keywords and numbers and such:

    static const char *cmd_strings =
      PSTR("board ""bytes ""hwords "   "words " "pwm " "interrupts " "sercom "   "pins "   "help "    "? ");
    enum {
      CMD_BOARD, CMD_BYTE, CMD_HWORD, CMD_WORD, CMD_PWM, CMD_INT,    CMD_SERCOM, CMD_PINS, CMD_HELP, CMD_HELP2  // make sure this matches the string
    };

    cmd = ttycli.keyword(cmd_strings); // look for a command.

    switch (cmd) {
      case CMD_BOARD:
       // :
        break;
      case CMD_INT:
       // :
          break;

      case CMD_PWM:
       //  : etc...

I keep "planning" on wrapping this all up in a nice library and documenting/publicizing it, but ... I keep getting distracted by all the things it COULD do. (spoiled by the COMND% Jsys!)
The current state of things is ... not good, but you can look at it here: GitHub - WestfW/parser: Simple serial command line parser for Arduino/etc
With several examples (each having their own slightly different copy, alas) here: GitHub - WestfW/Duino-hacks: Tiny Arduino (and related) hacks not worth of their own repository.

17

This does @Robin2 's code as a library. It's something I wrote for myself so it isn't really documented. I could I guess turn it into a proper library. GitHub - delta-G/StreamParser: A DiscoBot Library that handles data with start and end markers

The unique thing about it is that if you want to send pure digital data and not ascii and you're worried that one of the bytes might happen to match the end marker, then you can send control code 0x11, 0x12, or 0x13 as the first byte after the start marker and then another byte that indicates the size and it will receive that many bytes before looking for the end marker.

It often gets combined in my world with this library GitHub - delta-G/CommandParser: A DiscoBot Library that allows one letter text commands to match callbacks. which takes in an array of objects that match a character to a function pointer and call various functions based on the first character in a command string.

18

Is my non working example code something that can't be done?

I appreciate your replies, however I need something small to implement in my existing project, something simplistic that I can build off of and learn from.

19

Why doesn't this work? Nothing is copied into serialInCstring, it is empty.

while(Serial.available() > 0 && newData == false)
    {
        const char* chr = Serial.read();
        Serial.print("serialInString: "); Serial.println(serialInString);
        strcpy(serialInCstring, chr);
        Serial.print("chr: "); Serial.println(chr);
        Serial.print("serialInString: "); Serial.println(serialInString);
    }

Why doesn't this work? Nothing is copied into serialInCstring, it is empty.

while(Serial.available() > 0 && newData == false)
    {
        // strlcpy(char *dest, const char *src, size_t size);
        char chr = Serial.read();
        Serial.print("chr: "); Serial.println(chr);
        strlcpy(serialInCstring, chr, sizeof(serialInCstring));
        Serial.print("serialInString: "); Serial.println(serialInString);
    }
20

In general, it is an excellent idea to always explain what you mean by "why doesn't this work?"

const char* chr = Serial.read();

The result of Serial.read() is a single character. You should put that in a char variable, preferably a character array, not a pointer.

Please spend more time studying the Serial Input Basics tutorial.