Program run well once but won't call function in 2nd run

Hey all,

I’ve got a real brain scratcher going on here. I’ve take the time to write my own functions that parse a string of comma separated number into an array of individual float values.

The program works well the first time through and parses everything correctly. However on the second loop it seems to get stuck. I’ve used Serial.print’s to narrow it down to the point where it calls

int numberofvalues = numberofvaluesinstring(stringOne);

The function is called but if I put a Serial.print at the beginning of the numberofvaluesinstring function, it “locks up” and doesn’t start that function even though it did it the first loop.

I’ve added a Serial.print command to the beginning and end of each function so I can debug where the problem is. The code is as follows:

void setup() {
  // put your setup code here, to run once:
 // Open serial communications and wait for port to open:
  Serial.begin(9600);
  while (!Serial) {
    ; // wait for serial port to connect. Needed for native USB port only
  }

 
}

void loop() {


  String stringOne = "1,2,3,12.5,123,543,1234,1,1,1";   //String to Parse

Serial.println("Start of loop");




// Code from here down to next mark must be copied over along with parser function

    int numberofvalues = numberofvaluesinstring(stringOne);
 
    float values[numberofvalues];                //Define array size to hold parsed numbers
   
    parser(stringOne, numberofvalues, values);   //Call parser function

 
   //Optional display parsed values in order
   for(int q=0; q<numberofvalues;q++)            //Display all values parsed
   {
   Serial.println(values[q]);
   }

Serial.println("End of Loop");
Serial.println("");

 
}




int numberofvaluesinstring(String stringOne)    //Take String of values and return the number of comma separated values
{  Serial.println("Numberofvaluesinstring Start");
   int stringlength = stringOne.length();    //How many characters are in the string?
  int dividercount = 0;                     //Counter to count the number of separators
   
  for (int n = 1; n<=stringlength; n++)    //Increment through the string and count the separators
  {
    if (stringOne.charAt(n)==',')
    {
      dividercount = dividercount + 1;
 
    }

  }

    int numberofvalues = dividercount + 1;       //Number of values are always one more than separators

    Serial.println("Numberofvaluesinstring End");
   
    return numberofvalues;
}






void parser(String stringOne, int numberofvalues, float values[])  //Break the string down into invidual float values
{
  Serial.println("parser Start");
 
  int separatorindex[numberofvalues];                  //Store positions of separators in this array   
  String substrings[numberofvalues];                   //Break main string into substrings in this array


  separatorindex[0] = stringOne.indexOf(',');           //Store position of first separator

  for(int i=1; i<(numberofvalues-1); i++)
   {
    separatorindex[i] = stringOne.indexOf(',',separatorindex[i-1]+1);  //Store position of following separators
 
   }


    substrings[0] = stringOne.substring(0,separatorindex[0]);                   //Store first value before first separator to a substring array
    values[0] = substrings[0].toFloat();                                        //Convert first substring to a float
 
  for(int m = 1; m<=numberofvalues; m++)
  {   
   substrings[m] = stringOne.substring(separatorindex[m-1]+1, separatorindex[m]);     //Store subsequent values between separators
   values[m] = substrings[m].toFloat();                                               //Convert it value to a float and store in values array to be refereced back in program.
   }



Serial.println("Parser End");

and the output is as follows:

Start of loop
Numberofvaluesinstring Start
Numberofvaluesinstring End
parser Start
Parser End
1.00
2.00
3.00
12.50
123.00
543.00
1234.00
1.00
1.00
1.00
End of Loop

Start of loop

as you can see, it refuses to go and start the numberofvaluesinstring function. There is literally nothing between the “Start of loop” and “Numberofvaluesinstring Start” prints that would cause it to stop.

Any thoughts?

Thanks
Jim

P.S. Thinking it might be a memory error, I tried running it on a Mega and I get the same issue. According to the compiler, I’m not anywhere close to using all the SRAM…I think it’s something like 320 bytes.

I see a loop that stores a value in substrings[m] where m can be one location past the end of substrings. There may be other issues but this one alone is sufficient.

Lose the Strings!

I think vaj4088 got it right :wink: Fixing the below in the parser function seems to solve it

  for (int m = 1; m <= numberofvalues; m++)

And I agree with lastchancename that you should get rid of String (capital S). Below would be an option.

A function to count the number of elements

/*
    Take nul terminated character array of values and return the number of comma separated values
    Input:
      nul terminated character array
    Returns:
      number of elements in input

    Note:
      returns 1 of the nul terminated character array is empty
*/
int numberofvaluesinstring(char *stringOne)
{
  Serial.println("Numberofvaluesinstring Start");

  int num = 0;
  // pointer to comma; intialized with beginning of nul terminated character array
  char *ptr = stringOne;

  // count
  while ((ptr = strchr(ptr, ',')) != NULL)
  {
    // increment count
    num++;
    // increment pointer to point to net element
    ptr++;
  }

  num++;
  Serial.println("Numberofvaluesinstring End");

  return num;
}

And this will do the parsing in a non-destructive way.

/*
  Break the nul terminated character array of values down into invidual float values; non-destructive
  In:
    nul terminated character array
  Out:
    parsed values
  Returns:
    none
*/
void safeparser(char *stringOne, float values[])
{
  Serial.println("parser Start");

  int valueIndex = 0;

  // work pointer to beginning of an element
  char *pStart = stringOne;
  // pointer to comma
  char *pComma;


  while ((pComma = strchr(pStart, ',')) != NULL)
  {
    // replace comma by nul terminator
    *pComma = '\0';
    // convert
    values[valueIndex++] = atof(pStart);
    // restore comma
    *pComma = ',';
    // set pStart to point to beginning of enxt element
    pStart = pComma + 1;
  }

  values[valueIndex] = atof(pStart);

  Serial.println("Parser End");
}

Note that numberofvalues is no longer a parameter for the above function

And you loop()

void loop()
{
  static int loopCounter = 0;

  Serial.print("loop counter: "); Serial.println(++loopCounter);

  char stringOne[] = "1,2,3,12.5,123,543,1234,1,1.25,1.5";   //String to Parse

  Serial.println("Start of loop");


  // Code from here down to next mark must be copied over along with parser function

  // check if there is data
  if (strlen(stringOne) != 0)
  {
    int numberofvalues = numberofvaluesinstring(stringOne);
    Serial.print("number of values: "); Serial.println(numberofvalues);
    float values[numberofvalues];                 //Define array size to hold parsed numbers

    safeparser(stringOne, values);                //Call parser function


    //Optional display parsed values in order
    for (int q = 0; q < numberofvalues; q++)      //Display all values parsed
    {
      Serial.println(values[q]);
    }
  }
  Serial.println("End of Loop");
  Serial.println("");
}

There is one thing that can be improved. You can build in a check in loop() if stringOne is NULL; this is useful if stringOne comes from a function (that you don’t have control over) which returns NULL if there is no data.

  // check if there is data
  if (stringOne != NULL && strlen(stringOne) != 0)

Thanks guys, yes it looks like if substrings[x] where x was 20, then the array "locations" would be from 0-19, the "=" sign would try to go to array space 20, which doesn't exist. Makes sense.

I've been coding on again off again for years and have never delved into the mysterious land of pointers. So I figure this is a good opportunity to get my feet wet. I went on a bit of a search try to to find some information on them to better understand. Please bear with me and let me know if I've kinda got it right.

I looked at your example code below (I added the first two lines in order to get the code to run without the original longer code....baby steps:) ):

 char *stringOne;

  stringOne = "1,2,3,4";

   int num = 0;

  // pointer to comma; intialized with beginning of nul terminated character array
  char *ptr = stringOne;

  // count
  while ((ptr = strchr(ptr, ',')) != NULL)
  {
    Serial.println(ptr);
    // increment count
    num++;
    // increment pointer to point to net element
    ptr++;
  }

  num++;
  
  delay(5000);

"char *stringOne;" This defines a pointer to char stringOne, they're not pointing to anything yet.

" stringOne = "1,2,3,4";" Put the characters 1,2,3,4 into the actual variable "stringOne"

"char *ptr = stringOne;" Put the actual value stored in stringOne into character variable ptr

" while ((ptr = strchr(ptr, ',')) != NULL)" When an * is not used, we're talking NOT about the value stored in ptr, we're talking about the address location of the values stored in ptr.

This is where I'm a little stuck. Let me break down how I interpret this line:

search the characters in ptr for the first comma, then give the address of that comma until NULL is received when no more commas are found.

Why wouldn't it be strchr(*ptr,',')? Don't we want to tell strchr to search in the actual variable?

I would think since *ptr is a value and ptr is an address to a value that you'd want to say "Search the value and report back the address or:

while ((ptr = strchr(*ptr,','))!=NULL)

Obviously this is wrong because it won't compile, but can someone clarify why the other way is correct so I can understand correctly?

Anyway, moving on:

The other issue I'm having trouble understanding is the following:

If ptr is the address to a memory location, then how come when I say

Serial.println(ptr); it prints ,2,3,4 instead of printing the memory location number?

This was a bit confusing because you do ptr++; If ptr is a memory location, ptr++ makes sense since you're just bumping the pointer to the very next character location (I think). But it's confusing since Serial.println(ptr) does not show ptr is a memory location. My brain is having trouble figuring out how the code can increment ptr if ptr is ",2,3,4"? Can someone clarify please?

Anyway, that's as far as I've gotten with pointers. I want to understand them before moving to the next "chunk" of code suggestion so I make sure I've got a semi-good grasp before I make any more wrong assumptions. Thank you guys for helping out. Learning new stuff can be a struggle, but I have a feeling this one will be worth it and very useful.

“char *stringOne;” This defines a pointer to char stringOne, they’re not pointing to anything yet.

No, it declares a variable called stringOne that points to a character.

" stringOne = “1,2,3,4”;" Put the characters 1,2,3,4 into the actual variable “stringOne”

This will place the nul terminated character array }1,2,3,4" in memory. Your memory in the arduino could now look something like

 variable   | address | content | statement
------------+---------+---------+------------------
stringOne   | 0000    | xxxx    | char *stringOne;
...
...
"1,2,3,4"   | 0200    | '1'     | "1,2,3,4";
            | 0201    | ','     |
            | 0202    | '2'     |
            | 0203    | ','     |
            | 0204    | '3'     |
            | 0205    | ','     |
            | 0206    | '4'     |
            | 0207    | '\0'    |

Addresses are thumbsuck, the c-style string “1,2,3,4” is considered a variable as well for this example (after all, you can modify it :wink: ).

And next assign the memory location (address) of the first element of the character array to stringOne after which the memory would look like

 variable   | address | content | statement
------------+---------+---------+------------------
 variable   | address | content | statement
------------+---------+---------+------------------
stringOne   | 0000    | 0200    | ------+
...                                     |
...                                     |
"1,2,3,4"   | 0200    | '1'     | <-----+      
            | 0201    | ','     |
            | 0202    | '2'     |
            | 0203    | ','     |
            | 0204    | '3'     |
            | 0205    | ','     |
            | 0206    | '4'     |
            | 0207    | '\0'    |

Note the change of the first memory location.

“char *ptr = stringOne;” Put the actual value stored in stringOne into character variable ptr

Declare a variable ptr which is a pointer to a character array and assign it the content of the variable stringOne (which is also a pointer). After this, your memory could look like

 variable   | address | content | statement
------------+---------+---------+------------------
stringOne   | 0000    | 0200   |
...
...
"1,2,3,4"   | 0200    | '1'     | "1,2,3,4";
            | 0201    | ','     |
            | 0202    | '2'     |
            | 0203    | ','     |
            | 0204    | '3'     |
            | 0205    | ','     |
            | 0206    | '4'     |
            | 0207    | '\0'    |
...
...
ptr         | 0300    | 0100    | char *ptr = stringOne;

" while ((ptr = strchr(ptr, ‘,’)) != NULL)" When an * is not used, we’re talking NOT about the value stored in ptr, we’re talking about the address location of the values stored in ptr.

If a * is NOT used, we’re talking about the content of the variable ptr which is 200 (initially).
If a * is used, we’re talking about the content of the memory location (address) that is stored in the variable ptr; so if the variable ptr contains 200, we look at what is stored in location 200.

Why wouldn’t it be strchr(*ptr,’,’)? Don’t we want to tell strchr to search in the actual variable?

We tell strchr to start searching at the location (address) that is stored in ptr (so at address 0200).

Serial.println(ptr); it prints ,2,3,4 instead of printing the memory location number?

You have told the compiler that ptr is a pointer to a character (array). So it will print as if it’s a string. You can force the compiler to treat the content of ptr as an integer or long which will print the content of ptr and not what ptr points to.

Serial.println(ptr);        // print the c-style string that ptr points to
Serial.println(*ptr);       // print the character that ptr points to
Serial.println((int)ptr);   // interprete the content of ptr as an integer and print it

The choice between long and integer depends on the size of an address. In most 8-bit microcontrollers, it is 16 bits (and hence the cast to int in above).

I hope that this helps in understanding.

Thank you for the detailed response. I've been out of town and I've been spending time slowly going through this so I can really get a handle on pointers. I know C basics but this seems like a tool that would be useful to really understand (plus a lot of libraries use pointers so it'd be nice to know what they're doing instead of saying "oh there's that random asterix again!).

I was wondering if there's an error in your explanation (or more likely I'm missing something).

When you put char *ptr = stringOne; I thiiiink you're saying the following

  1. stringOne points to the start of the memory location where the character array is found (holds the address of the start of the character array....in your case 0200)

  2. char *ptr = stringOne; take the memory location, 0200, stored in stringOne pointer and put that memory location ALSO in ptr. In other words, stringOne points to the memory location of the beginning of the character array AND ptr points to the memory location of the beginning of the character array. They should, I think, both be 0200.

In your example though:

variable   | address | content | statement
------------+---------+---------+------------------
stringOne   | 0000    | 0200   |
...
...
"1,2,3,4"   | 0200    | '1'     | "1,2,3,4";
            | 0201    | ','     |
            | 0202    | '2'     |
            | 0203    | ','     |
            | 0204    | '3'     |
            | 0205    | ','     |
            | 0206    | '4'     |
            | 0207    | '\0'    |
...
...
ptr         | 0300    | 0100    | char *ptr = stringOne;

Shouldn't ptr be:

ptr |0300|0200| ?

Perhaps a typo but if I'm wrong I need to know why!

If a * is NOT used, we're talking about the content of the variable ptr which is 200 (initially).
If a * is used, we're talking about the content of the memory location (address) that is stored in the variable ptr; so if the variable ptr contains 200, we look at what is stored in location 200.

Ok let me see if I have this right based on the example:

ptr is the initial memory location ptr points at. In this case 0200 *ptr is the value located inside that memory location, in this case 1

Said more simply, no asterix means we're messing with the actual address location, with an asterix means we're messing with the contents inside the memory location?

I hope I've got that right.

Serial.println(ptr);        // print the c-style string that ptr points to
Serial.println(*ptr);       // print the character that ptr points to
Serial.println((int)ptr);   // interprete the content of ptr as an integer and print it

Based on the three examples, and using your example, the different outputs (before running strchr and incrementing the pointer at all)

Serial.println(ptr) would be 1,2,3,4 Serial.println(*ptr) would be 1 Serial.println((int)ptr) would be 0200

Baby steps...I hope I'm getting there!

Thanks Jim

In the table, ptr must indeed have the content 200.

Never sure what the println(*ptr) prints; probably 49 which is the numeric value of the character '1'. Further you're right.

You can easily test yourself ;)

I will see if I can confirm in the weekend.