Arduino Crashing (Char Buffers?)

Hey guys,

So I have this Github repo which is a library that supports cellular modems for Arduino and everything works great, except for one little (yet actually quite substantial) thing. I don't expect people here to have the hardware to test this, so I'm going to ask that you just try to see what's going wrong since I've more or less narrowed down where the problem is happening.

The main Arduino sketch to look at is the IoT_Example Sketch which basically configures the modem to send data to a test server (dweet.io). The other thing to look at is the source code in the Adafruit_FONA.cpp file which has the relevant functions "postData()" starting lines 1508 and 1593. The main issue is that the Arduino crashes (restarts the sketch from the beginning) or freezes when it hits the "postData()" function, like on line 340 or 349 (depends on what type of modem it is) but this depends on how some other things are set up. Here is a list of my notes in an attempt to keep my question organized:

  • The crashes or freezes only happen when the sizes of URL[] and body[] arrays defined in lines 308-309 in the sketch are beyond a certain size or too small to contain the sprintf() arguments mentioned in the next point (which I can understand because of memory allocation issues)
  • Whether the sketch crashes or not depends on the size of the URL[] and body[] arrays as well as the sprintf() operations used to construct the "URL" and "body" data being sent via HTTP, like in lines 336-337. The larger the sprintf() arguments, the more likely the program will crash, even when the size of URL and body are declared to be larger than necessary to contain the data.
  • If URL and body are set to some sweet spot relative to the size of the sprintf() commands, the program works perfectly. For example, "URL[200] and body[100] works great for lines 346-347, but if the size is changed to, say, URL[300] and body[100] then it breaks and the Arduino reboots, but again, only when it reaches the point in the code where the postData function is.
  • I've tried resetting the watchdog timer at just about every other command in the Arduino sketch and that didn't help. From what I've tested so far it really seems like a character buffer issue or memory allocation issue.
  • Crashing is very consistent and it either works or it crashes, not just sometimes.
  • Previously I had used a single buffer called "auxStr[]" in the postData() function (seen commented out in line 1510 of the .cpp file) which seemed to work, again for only certain URL and body sizes. What makes no sense to me at all is if auxStr is size 64 (not large enough to hold something like 88 bytes), the program would still run perfectly fine, but simply changing its size to 200 would make the program crash, even though I have the feeling it should be improved.
  • There are two postData() functions but don't worry, they have the same nature except that they are used depending on the modem type
  • I have tested the sketch piece by piece and the problem somehow lies in the postData function, sprintf() lines, and URL and body arrays.
  • The Arduino sketch takes up 64% of flash memory and 59% of RAM on Arduino Uno so it's not a Serial.print issue (I also tried commenting out all the serial prints in the sketch, same problem)

Summary: Changing the size of URL[] and body[] buffers in the Arduino sketch as well as the size of the sprintf() lines used to construct URL and body make the sketch crash or not crash. Even if URL and body arrays are large enough to contain the data copied to them in the sprintf lines, the program could still crash. I have narrowed th

Is there something I'm doing wrong? I suspect some sort of mem allocation issue or weird char buffer thing going on here. I'd appreciate any feedback and keep in mind I'm not a professional programmer by any means, so I'm willing to be corrected!

Thanks!

You have not told us what Arduino you are using.

You have not told us the sizes of the char arrays that cause the problem.

It may not be necessary to use sprintf() to create a big cstring. You may get just the same effect if you send the data as a series of succesive Serial.print() commands.

When you compile a program the IDE tells yu how much SRAM it uses - what message are you getting?

Are you writing or reading beyond the bounds of your arrays?

...R

Thanks for the reply. I did mention "Arduino Uno" in the bullet point that says how large the compiled sketch is, but sorry for not being clear from the beginning.

The sketch breaks with URL[250] and body[100] if the modem is set up to run lines 362-363, for example. Increasing any of those value would cause it to crash as well. It doesn't seem to be an exact science.

I am using sprintf() because I need to format the data in URL and body to perform AT commands. For example, URL[] might contain "GET /sendVal?foo=bar" and body[] might contain a JSON like "{"foo":bar,"hello":world}" which will be used together in lines 1666-1669 in the .cpp file to send after an AT command runs.

So basically the general sequence of events is this:

  • The Arduino sends a bunch of AT commands and gets their replies
  • At a certain point we hit the point in the sketch where the postData() function is used. It runs something like "AT+CHTTPSSEND=" as in the attached picture, then Arduino sends the URL[] and/or body[] arrays when it sees the ">" character response from the modem.
  • Please note that this also crashes with a GET request which doesn't even involve the body[] array at all, although it's initialized. For example, declaring URL[300] can cause the sketch to crash if the modem is configured to run lines 346-347

Those buffers are quite substantial to be allocated on the stack like that. I would assume that you are crashing the stack into the heap.
Try making them globals and retest.

I tried making all these global vars (just placed above setup() function)

char URL[200];  // Make sure this is long enough for your request URL
char body[100]; // Make sure this is long enough for POST body
char latBuff[12], longBuff[12], locBuff[50], speedBuff[12],
     headBuff[12], altBuff[12], tempBuff[12], battBuff[12];

but no luck, it still crashes when URL is size 200 or above (again, 175 might still crash but I didn't test that; it's not an exact science). URL[150] and body[100] work just fine. I tested when the URL was 80 bytes and body was 28 bytes. So now that I test it again, I should say that actually for some values it completely restarts the sketch, for other values it freezes (prints a few characters of a Serial.print() line, then freezes), and for other values (like the URL[200] and body[100] above) it pauses for a little bit after sending the URL and body data as response to the AT command, then it says the operation failed and kind of continues running the code, although of course at this point everything's not operational and software serial communication is broken between the Arduino and the modem.

OK, so here's what I found out:

  • With URL[200] and body[100] declared inside the loop the sketch crashes and restarts from the beginning
  • with URL[200] and body[100] but declared globally the sketch kind of keeps running but serial comm between Arduino and the modem goes caput after trying to print out the contents of URL and body.

So yes, it seems like declaring it globally helps a little bit, but there still seems to be an issue with a large enough URL or body array size.

It smells to me like memory corruption of some sort. Chopping and changing buffer sizes isn't going to reasonably fix the problem in any case, so no need to waste your time.

Keep the buffers global to eliminate that as an issue.

It's rather a lot of code to trawl through, so I'll just throw out ideas, maybe something sticks...

Are you using the exact code from the repo or have you tweaked it? Specifically I'm looking at imei which you are treating as a c-string but it may or may not be designed to be (ie. is it definitely 0 terminated). The repo has it with the first byte as a 0, so that would be OK, but maybe you are testing with a real IMEI?

Yes, I'm the owner of the repo and I'm using it verbatum.

I tried changing the declaration "char imei[16] = {0};" to "char imei[16];" instead, and no change in results.

Also, are you saying I might have ruined my Arduino Uno in some way, shape, or form via memory corruption? I just don't get how some buffer sizes work, and some don't. Very confusing problem for me haha.

Also, replying to your statement about the "imei", the IMEI number is pulled from the module via an AT command, so yes, I'm using a real IMEI number to test.

Ohhhhhhh wait a minute, we might be on to something... I created a new dummy array

char IMEI[16] = "123456789012345";

and commented out this bit of code which uses the imei[] array (lower-case, mind you)

uint8_t imeiLen = fona.getIMEI(imei);
  if (imeiLen > 0) {
    Serial.print("Module IMEI: "); Serial.println(imei);
  }

then replaced the "imei" with "IMEI" in the sprintf() function that populates the URL buffer, and voila, it worked, even with the same buffer sizes for URL and body that broke it previously!

The "getIMEI()" function was actually directly from Adafruit's repo (mine is basically a fork of the Adafruit FONA repo but with added functionality and support for more modules)

However, even with that mod, increasing URL and body sizes to something like URL[200] and body[200] still breaks it. But URL[200] and body[100] works, which didn't use to. I'm wondering if IMEI has anything to do with it.

Ok, you need to make sure the IMEI is 100% '\0' terminated before using it as a string in sprintf.

Looking at what I believe are your changes in Adafruit_FONA.cpp, you have gone really big on stack allocations. The Uno only has 2k of RAM to play with and all these local buffers will chomp what remains of that memory very quickly.
eg. you have in your sketch char URL[200]; which is passed to Adafruit_FONA::postData which then does char urlBuff[strlen(URL) + 22]; sprintf(urlBuff, "AT+HTTPPARA="URL","%s"", URL);, essentially duplicating the string. Then there are some more similar stack buffers on top.

Don't get me wrong, there is nothing wrong with using the stack when memory is plentiful, like on a PC, but in the memory constrained Uno you are going to run into problems sooner or later. You need to design your code in a way that it uses RAM more sparingly. Using globals at least lets you see the cost up front at compile time. Using big local variable stack buffers just makes things blow up at run time.

I see you have moved on a little from the last post I read...

Hmmm, so how do I make it more efficient? Because in the postData function I have to use the contents of URL and body to combine it with other things, and that containing buffer has to be larger than URL or body. For example, if URL is "GET /blah/blah?foo=bar" I might need to combine that with an AT command.

androidfanboy:
Hmmm, so how do I make it more efficient? Because in the postData function I have to use the contents of URL and body to combine it with other things, and that containing buffer has to be larger than URL or body. For example, if URL is "GET /blah/blah?foo=bar" I might need to combine that with an AT command.

Make a new sendCheckReply and getReply combination with parameters that meet your requirements. (If none of the existing ones do.)

Or a couple of new functions which split sendCheckReply into two - one function just sends the strings out to mySerial, then the second writes the line termination and waits for a response. You call the first function multiple times with each text string, then the second just once to send and get the response.
sendCheckReply, then just becomes one call to the first function followed by one call to the second.

There may be other/better ways, but these come to mind first.

Awesome, thanks for the help! I'll see what I can do :slight_smile: