Go Down

Topic: Use of string char causes Arduino code to restart (Read 231 times) previous topic - next topic

howardinventor

I'm building a tide clock that involves going out to the Web to consume a tidal API.  Being my first real Arduino project it has a number of unknowns so I've been breaking it down into smaller projects to prove the code.  I can consume the API and have been experimenting how to parse the JSON.  Accepted ArduinoJSON might be a solution but I thought, based on the amount I had to parse, a bespoke piece of code would be worth trying.  It has certainly taught me a lot about the string class!  Perhaps not enough.

I've built it up bit by bit to prove it works (and it does!).  However I can Serial.Print the value of the global variable  tideType and tideTime in the function, but if I try to do this after calling the function getTideDetails from setup the monitor reports this
About to start Get Tide Details
Starting Get Tide Details
About to start Get Tide Details
Starting Get Tide Details
About to start Get Tide Details
Starting Get Tide Details
About to start Get Tide Details
Starting Get Tide Details
About to start Get Tide Details
Starting Get Tide Details....(on and on)
It must be the way I'm handling the strings tideType and tideTime because whenever I try to Serial.print them from setup after the function, it looks like the Arduino restarts.  However tideHeight which is a float outputs fine.  
Can someone put me out of my misery?
Code below (be kind)
Code: [Select]
char array[] = "[{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T02:15:00\",\"IsApproximateTime\":false,\"Height\":1.3632871362992067,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T08:25:00\",\"IsApproximateTime\":false,\"Height\":5.2168696552371623,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T14:25:00\",\"IsApproximateTime\":false,\"Height\":1.8097719968974781,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\",{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T20:47:00\",\"IsApproximateTime\":false,\"Height\":5.3284583370257206,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"";
char *ptr = NULL;
char tideType[9] = "";
char tideTime[10] = "";
float tideHeight = 0;


void setup()
{
  Serial.begin(9600);
  delay(500);
  Serial.println("About to start Get Tide Details");
  getTideDetails();
  delay(500);
  Serial.println("getTideDetails has completed");
  Serial.print("Tide type is ");
  Serial.println(tideType);  //As sson as you do this it appears to restart.  Comment this out and it runs normally
  //Serial.print("Tide time is ");
  //Serial.println(tideTime);
 
  Serial.print("Tide Height is set at ");
  Serial.println(tideHeight,4);
 
}


void getTideDetails(){
  Serial.println("Starting Get Tide Details");
  ptr = strtok(array, "\",\{[\}");  // takes a list of delimiters
  while (ptr != NULL)
  {
    //Serial.println(ptr);
    if (strcmp(ptr, "EventType") == 0) {  //Identified tag called EventType
      ptr = strtok(NULL, "\",\{[\}:");  //Put the pointer to the next tag
      strncpy(tideType, ptr, strlen(ptr));  //copy low or high water across
      tideType[strlen(ptr)] = '\0';  //terminate it to avoid corruption
    } else if (strcmp(ptr, "DateTime") == 0) {  //Identified tage called DateTime
      ptr = strtok(NULL, "\",\{[\}");  //skip a tag two avoid unwanted colon
      ptr = strtok(NULL, "\",\{[\}");  //Pointer is now at the start of the time element
      strncpy(tideTime, ptr, strlen(ptr));  //copy it across to tide time string
      tideTime[strlen(ptr)] = '\0';  //terminate it to avoid corruption
    } else if (strcmp(ptr, "Height") == 0){ //Identified tag called Height
      ptr = strtok(NULL, "\",\{[\}:");  //Pointer is now at the start of the tide height tag
      tideHeight = atof(ptr);  //Convert to float and assign.
    }
    ptr = strtok(NULL, "\",\{[\}");  // Move to the next tag
  }
}

void loop()
{
  // put your main code here, to run repeatedly:

}




TheMemberFormerlyKnownAsAWOL

#1
Mar 06, 2021, 12:05 pm Last Edit: Mar 06, 2021, 12:26 pm by TheMemberFormerlyKnownAsAWOL
A good habit to get into when debugging array crashes is to put a Serial.flush () after every print.

Also, why are braces { }  escaped in your delimiters? " "\",\{[\}:");
Please don't PM technical questions - post them on the forum, then everyone benefits/suffers equally

guix

Can "2021-03-05T02:15:00" fit in an array of 10 chars ? :)

TheMemberFormerlyKnownAsAWOL

Also, if you've got two sets of delimiters, give them names.
Please don't PM technical questions - post them on the forum, then everyone benefits/suffers equally

countrypaul

Are you aware that strtok() destroys the original string, by putting NULLs in place of the tokens, returning pointers to the appropriate parts of the original memory?

howardinventor

Thanks for your help everyone  :)
Can "2021-03-05T02:15:00" fit in an array of 10 chars ? :)
That's a very good point.   Typo on my part. I've increased it to 20, to give room to spare.  Thought that could have been the reason, writing beyond the size of my char and corrupting memory but it still behaves the same
A good habit to get into when debugging array crashes is to put a Serial.flush () after every print.

Also, why are braces { }  escaped in your delimiters? " "\",\{[\}:");
I've now put flushes after my Serial statements.  That's a new thing to me.
I marked curly brackets as delimiters because they were being extracted as tokens.  As I don't need them, treating them as delimiters meant they did not get extracted.   
Also, if you've got two sets of delimiters, give them names.
As in create meaningful constants for reuse and readability?
Are you aware that strtok() destroys the original string, by putting NULLs in place of the tokens, returning pointers to the appropriate parts of the original memory?
I had read that.  However that does not matter too much to me.  Once I've looped through the tokens and got my tide information, the JSON string should be refreshed the next time I need tide details.
I've put the flushes in and sized tideTime appropriately but it still fails in the same way.
Code: [Select]
char array[] = "[{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T02:15:00\",\"IsApproximateTime\":false,\"Height\":1.3632871362992067,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T08:25:00\",\"IsApproximateTime\":false,\"Height\":5.2168696552371623,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T14:25:00\",\"IsApproximateTime\":false,\"Height\":1.8097719968974781,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\",{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T20:47:00\",\"IsApproximateTime\":false,\"Height\":5.3284583370257206,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"";
char *ptr = NULL;
char tideType[9] = "";
char tideTime[20] = "";
float tideHeight = 0;


void setup()
{
  Serial.begin(9600);
  delay(500);
  Serial.println("About to start Get Tide Details");
  Serial.flush();
  getTideDetails();
  delay(500);
  Serial.println("getTideDetails has completed");
  Serial.flush();
  Serial.print("Tide type is ");
  Serial.flush();
  Serial.println(tideType);  //As soon as I try to output tideType it seems too restart
  Serial.flush();
  //Serial.print("Tide time is ");
  //Serial.println(tideTime);
 
  Serial.print("Tide Height is set at ");
  Serial.println(tideHeight,4);
 
}


void getTideDetails(){
  Serial.println("Starting Get Tide Details");
  Serial.flush();
  ptr = strtok(array, "\",\{[\}");  // takes a list of delimiters
  while (ptr != NULL)
  {
    //Serial.println(ptr);
    if (strcmp(ptr, "EventType") == 0) {  //Identified tag called EventType
      ptr = strtok(NULL, "\",\{[\}:");  //Put the pointer to the next tag
      strncpy(tideType, ptr, strlen(ptr));  //copy low or high water across
      tideType[strlen(ptr)] = '\0';  //terminate it to avoid corruption
    } else if (strcmp(ptr, "DateTime") == 0) {  //Identified tage called DateTime
      ptr = strtok(NULL, "\",\{[\}");  //skip a tag two avoid unwanted colon
      ptr = strtok(NULL, "\",\{[\}");  //Pointer is now at the start of the time element
      strncpy(tideTime, ptr, strlen(ptr));  //copy it across to tide time string
      tideTime[strlen(ptr)] = '\0';  //terminate it to avoid corruption
    } else if (strcmp(ptr, "Height") == 0){ //Identified tag called Height
      ptr = strtok(NULL, "\",\{[\}:");  //Pointer is now at the start of the tide height tag
      tideHeight = atof(ptr);  //Convert to float and assign.
    }
    ptr = strtok(NULL, "\",\{[\}");  // Move to the next tag
  }
}

void loop()
{
  // put your main code here, to run repeatedly:

}

Any further ideas?

guix

Can "HighWater" fit in an array of 9 characters ?

It's 9 characters, so when you do
Code: [Select]
tideType[strlen(ptr)] = '\0';

You are writing a 0 in tideType[9], which is outside of the array

howardinventor

Can "HighWater" fit in an array of 9 characters ?

It's 9 characters, so when you do
Code: [Select]
tideType[strlen(ptr)] = '\0';

You are writing a 0 in tideType[9], which is outside of the array
Oh my goodness, my understanding of sizing arrays must be flawed!  I thought stating [9] as an array size gave you 
0, 1, 2, 3, 4, 5, 6, 7, 8, 9
0 = H
1= i
2 = g
3 = h
4 = W
5 = a
6 = t
7 = e
8 = r
9 = \0
Hence strlen(tideType) would return 9 (for HighWater) and tideType(strLen) would be TideType(9) putting a null terminator in the last element.  
I've just increased it to 10 and it works! :)   There's me think I was being efficient.  Time to do some more experimenting with arrays.
Thanks for you help everyone, it is very much appreciated.  Every day is a learning day.

wildbill

Nope, an array of nine items has indexes 0-8, hence the crash.

Also, are you sure that LowWater and HighWater are the only event types you'll ever see?

guix

And, you shouldn't do
Code: [Select]
strncpy(tideType, ptr, strlen(ptr));
tideType[strlen(ptr)] = '\0';

Instead you should do
Code: [Select]
size_t len = min(sizeof(tideType)-1, strlen(ptr));
strncpy(tideType, ptr, len);
tideType[len] = '\0';

Same for tideTime, of course :)

howardinventor

Thanks for the useful pointers (no pun intended).  That is a rabbit hole I won't fall down again.
Thanks again.

wildbill

Thanks for the useful pointers (no pun intended).  That is a rabbit hole I won't fall down again.
Thanks again.
Sorry, unless you're very unusual, you probably will  >:(  You have to hit your thumb with the hammer a few times before you remember.

drmpf

Try switching to my SafeStrings library, it will print out an error message when you try and do something 'unsafe' with your char[] / c-strings
SafeString was designed to avoid problems just like this one and has detailed tutorial.



drmpf

#14
Mar 06, 2021, 07:18 pm Last Edit: Mar 06, 2021, 07:23 pm by drmpf Reason: missing ) in debug statement
Your latest code from #5 shows these errors, after fixing bad escape in "\",\{[}"
Code: [Select]
About to start Get Tide Details
Starting Get Tide Details
delimiters1:",{[}
delimiters2:",{[}:
Error: sftideType = "HighWater"
 needs capacity of 9
        sftideType cap:8 len:8 'LowWater'
Error: sftideType = "HighWater"
 needs capacity of 9
        sftideType cap:8 len:8 'LowWater'
getTideDetails has completed
Tide type is
Tide Height is set at 5.3285


Here is the code using SafeStrings
Code: [Select]
#include <SafeString.h>
char array[] = "[{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T02:15:00\",\"IsApproximateTime\":false,\"Height\":1.3632871362992067,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T08:25:00\",\"IsApproximateTime\":false,\"Height\":5.2168696552371623,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"},{\"EventType\":\"LowWater\",\"DateTime\":\"2021-03-05T14:25:00\",\"IsApproximateTime\":false,\"Height\":1.8097719968974781,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\",{\"EventType\":\"HighWater\",\"DateTime\":\"2021-03-05T20:47:00\",\"IsApproximateTime\":false,\"Height\":5.3284583370257206,\"IsApproximateHeight\":false,\"Filtered\":false,\"Date\":\"2021-03-05T00:00:00\"";
cSFA(sfarray, array); // wrap char[] in a SafeString
char *ptr = NULL;
char tideType[9] = "";
cSFA(sftideType, tideType); // wrap char[] in a SafeString
char tideTime[20] = "";
cSFA(sftideTime, tideTime); // wrap char[] in a SafeString
float tideHeight = 0;


void setup()
{
  // Open serial communications and wait a few seconds
  Serial.begin(9600);
  for (int i = 10; i > 0; i--) {
    Serial.print(' '); Serial.print(i);
    delay(500);
  }
  Serial.println();
  Serial.println("About to start Get Tide Details");
  SafeString::setOutput(Serial); // for error msgs
  if (SafeString::errorDetected()) {
    Serial.println(F("Error setting up global SafeStrings"));
  }
  getTideDetails();
  Serial.println("getTideDetails has completed");
  Serial.print("Tide type is ");
  Serial.println(tideType); // SafeString updates original char[]
  //Serial.print("Tide time is ");
  //Serial.println(tideTime);

  Serial.print("Tide Height is set at ");
  Serial.println(tideHeight, 4);

}


void getTideDetails() {
  Serial.println("Starting Get Tide Details");
  Serial.flush();
  cSF(sftoken, 20); // a SafeString to hold the token
  char delimiters1[] = "\",\{[}";
  char delimiters2[] = "\",\{[}:";
  Serial.print(F("delimiters1:")); Serial.println(delimiters1);
  Serial.print(F("delimiters2:")); Serial.println(delimiters2);
  int idx = 0;
  while ( (idx = sfarray.stoken(sftoken, idx, delimiters1)) >= 0) {
    //sftoken.debug(F("at top of While loop"));
    if (sftoken == "EventType") {
      idx = sfarray.stoken(sftoken, idx, delimiters2);
      sftideType = sftoken; // copy and check there is not overflow
    } else if (sftoken == "DateTime") {
      idx = sfarray.stoken(sftoken, idx, delimiters1);
      idx = sfarray.stoken(sftoken, idx, delimiters1);
      sftideTime = sftoken; // copy and check there is not overflow
    } else if (sftoken == "Height") {
      idx = sfarray.stoken(sftoken, idx, delimiters2);
      if (!sftoken.toFloat(tideHeight)) {
        Serial.print(F("Invalid Tide Height:")); Serial.println(sftoken);
      }
    }
    // next token done at top
  }
}

void loop()
{
  // put your main code here, to run repeatedly:

}

Go Up