This simple program freezes. Why?? (0022 vs 0021?)

I'm working on my first real Arduino app, a clock using words rather than digits, employing a Nokia 5110 LCD and a DS1307 RTC. However I'm having major issues with string corruption, lockups and reboots of my Uno and after much effort still have no idea why.

I've reduced the program to something very simple which uses no other hardware (see below). It simply loops, incrementing the seconds from 30 and Serial.println's them as a string (the computation of which is the only significant code). I compile using the IDE Arduino 0022, upload to my Uno, then connect to the serial monitor (@ 38k4). I see "thirty", "thirty-one" etc but nothing after "thirty-seven" (and in fact that's corrupted as "thirty- even"). The program seems to have frozen.

int clock_Second = 30;  // stops at "thirty- even"
//int clock_Second = 40;  // stops at "twenty-two"
int clock_Minute = 2;
int clock_Hour = 1;
int clock_DayOfWeek = 3;
int clock_DayOfMonth = 4;
int clock_Month = 5;
int clock_Year = 11;
bool clock_PM = true;

#define CLOCK_MAX_LINE 8
int clock_DisplayedMinute;
String clock_Lines[CLOCK_MAX_LINE];

char* clock_OrdinalNames[] =
{
  "zero",  "one",  "two",  "three",  "four",  "five",  "six",  "seven",  "eight",  "nine",  
  "ten",  "eleven",  "twelve",  "thirteen",  "fourteen",  "fifteen",  "sixteen",  "seventeen",  "eighteen",  "nineteen",
  "twenty",  "thirty",  "forty",  "fifty"
};

String clock_GetNumberName(int Number)
{
  // return the string representation of Number, eg 21 -> "twenty-one"
  String Name = "";
  if (0 <= Number && Number < 60)
  {
    if (Number <= 20)
    {
      // 0..20 are in there in full
      Name = clock_OrdinalNames[Number];
    }
    else
    {
      // get the tens part
      Name = clock_OrdinalNames[20 + ((Number - 20) / 10)];
      if (Number % 10 != 0)
      {
        // and the units part
        Name += "-";
        Name += clock_OrdinalNames[Number % 10];
      }
    }
  }
  return Name;
}

void clock_ShowTime(void)
{
  Serial.println(clock_Lines[0]);
  Serial.println("-----------");
}

void clock_FormatTime(void)
{
  int Hour = clock_Hour;  // unused remnants of original program
  int Minute = clock_Minute;
  String LineStr;
  
  clock_Lines[0] = clock_GetNumberName(clock_Second);
}

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

void loop(void)
{
  clock_FormatTime();
  clock_ShowTime();
  
  // advance the "clock"
  clock_Second++;
  if (clock_Second > 59)
  {
    clock_Second = 0;
    clock_Minute++;
    if (clock_Minute > 59)
    {
      clock_Minute = 0;
      clock_Hour++;
      if (clock_Hour > 12)
      {
        clock_Hour = 1;
        clock_PM = !clock_PM;
      }
    }
  }
}

I can't see anything wrong with the code, perhaps someone else can. I'd suspect some kind of resource exhaustion, but what? I'm not explicitly using the heap and I can't see any recursion. I'd also suspect a rogue pointer or array over-run, but I don't think that's happening.

Note that slight changes to the code make the problem change or apparently vanish (eg, starting the count at 40 halts at "twenty-two").

Tremendously frustrating, I hope someone can help.

It works fine if I use Arduino 0021 (sketch is 4310 vs 4465 bytes with 0022), so I'm suspicius of the compiler too, unlikely though that may be.

It works fine if I use Arduino 0021 (sketch is 4310 vs 4465 bytes with 0022), so I'm suspicius of the compiler too, unlikely though that may be.

0021 and 0022 ship with exactly the same compiler. The problem is very likely a bug in String.

Try this...

void clock_GetNumberName( int Number, String& Name )
{
// return the string representation of Number, eg 21 -> "twenty-one"
// Remove the following line...
String Name = "";

// Code in the middle stays the same

// Remove the following line...
return Name;
}

void clock_FormatTime(void)
{
int Hour = clock_Hour; // unused remnants of original program
int Minute = clock_Minute;
String LineStr; // Why is this here?

clock_GetNumberName( clock_Second, clock_Lines[0] ); // Change this line
}

Thanks, but that halts with

forty-four
-----------
forty-

As I said, changes to the code change the behaviour. Even if refactoring to use a reference rather than a return worked, it still wouldn't explain why.
"String LineStr" is a remnant of the original program. Taking it out should make no difference, but it does.

I'm not conviced about 0022 vs 0021 either, I think my problems first arose with 0021 and so I upgraded.

Could be a String problem, and I did suspect that early on, so I laboriously reworked my original code to use char[]'s, strcpy() & strcat() etc and still had weirdness.

I'd still like to know what's wrong with what I have. The release notes for 0022 don't mention changes to the String class, other than "Added String.toInt() function."

Board?

Uno.

My problems actually started on a Duemilanove and at some point I switched to the Uno to see if that would help.

I've had a quick look at the String source on github (WString.*). Compiled and ran it with a modified version of my sample code in Visual Studio 2005, ran OK, no sign of heap leakage or bad pointers.

My advise: Get rid of String class objects. Create a static character buffer for the text and write into that buffer directly.

If you play around with objects, it's not always obvious or easy to know, when an object is copied and when just references are passed around. And if the copied objects never run out of scope, they remain on the heap. Once the heap overflows, which happens quite quickly with the little RAM available, weird things start to happen, most of the time lock-ups or reboots.

As much as the String objects in C++ are great for regular programming, for microcontroller like the Arduino, they usually create more trouble than they help. Back to pointers and manual memory management.

Korman

(deleted)

I suspect you are seeing the effect of fragmentation in the heap.
As temporary String objects are created and freed the heap grows, leading to stack and heap colliding, and the program crashing.
A hint for this is - the unused string object on the stack being created and destroyed, changes when the crash happens - and all it does is make the stack a little deeper and allocate and free an empty string.

A test to confirm if it is fragmentation is to reduce the number of causes of fragmentation..
If you can pre-allocate the Strings with a length long enough to hold the result you expect then there will be no fragmentation in the heap as all results will be fit in all free spaces...

Just read code for String .. and .. it is not suitable for use as it always allocates a buffer exactly the length of the string side it, so the fragmentation of the heap can not be avoided that way.

i am out of definitive ideas. i think a character array passed and strcat() will work/ but is ugly.
perhaps the gnu string class works on arduino. it has buffer size args for the constructor.

am out of definitive ideas. i think a character array passed and strcat() will work/ but is ugly.

For this application, why bother with assembling the string? Just pass 1 or 2 indices into the string table and print the string from PROGMEM. If two values are returned, print a "-" between strings.

No data copying, no strings needed.

Korman

Thanks for your ideas. Sadly I'm at work now (writing non-embedded C++ code :)) so I won't be able to try them for a while.

Korman/dafid: I will refactor the test case without Strings. As I noted earlier I'd done that with my real application, and still had problems, but perhaps that was something else. I take your point about embedded programming and the heap. There are of course lots of ways to code the task, I chose the simple, modern, Object Oriented way. It's sad to have to go back to C-style coding rather than C++.

Spycatcher2k: I'd been using 57k6, switched to 38k4 before posting because I though most people would already be using that rate. Makes no difference.

If fragmentation is the issue, an even simpler test program should reveal it. May try verifying that...

kiwimew:
Korman/dafid:There are of course lots of ways to code the task, I chose the simple, modern, Object Oriented way. It's sad to have to go back to C-style coding rather than C++.

No, it's not sad, it just put into perspective what is lost when programming the contemporary way. Programmers these days have no real idea about computing resources. There's just enough or if not, buy a bigger or faster computer next Christmas. Programming the Arduino is a very healthy exercise to sharpen efficient coding skills and teaches a lot of what happens under the hood of those libraries and compilers.

I find that whenever I do more embedded programming work, my regular programming improves too, even if I won't bother counting processor cycles or worrying the size of my stack and heap. I just makes me evaluate available alternatives better and think more about what matters in a given situation.

See it as an exercise, like riding a bicycle when you also could drive there by car faster. Not as fast, not always practicval, but rewarding.

Korman

Programmers these days have no real idea about computing resources.

Not entirely sure I agree. As a professional C++ software engineer I am conscious of having a finite heap -- my code must delete what it new's (we're not dealing with a garbage-collecting language here). And I expect library code to do the same. The String class can't leak. And in this context I would've thought String should have an embedded-friendly allocation strategy (assuming fragmentation is the issue). Or at least have clearly stated caveats about its applicability to Arduino development.

Having said that, one of the things I like about Arduino dev is the limited resources. It takes me back to the days of writing 6502 assembler for my Acorn Electron.

I dunno. A "string" library, to do the things one expects it to do, has to support dynamic allocation and garbage collection (of some kind.) IMNSHO, the 2K of total RAM on an Arduino isn't enough to do that effectively.

(that said, there's an improved string library in the works...)

I am conscious of having a finite heap

How conscious are you of the overhead of having dynamically allocated memory?
Which uses more memory: String s = "short string"; or char s[20] = "short string"; ?
The former will theoretically allocate only enough space for the actual string, but has overhead due to the String type and dynamic allocation. The latter has wasted space in the array, but no overhead.

It's something to do with memory allocation. I tried to find how quickly memory was being used up, but the problem went away! (as usual, huh?)

Added this function to the front, from the playground:

int availableMemory() {
  int size = 1024; // Use 2048 with ATmega328
  byte *buf;

  while ((buf = (byte *) malloc(--size)) == NULL)
    ;

  free(buf);

  return size;
}

Then in loop, at the start, added:

 availableMemory ();

Somehow, computing the available memory fixes the problem. I suspect some idiosyncracy in the malloc library, which I haven't yet been able to track down the source for (if anyone knows exactly where? .... hint hint).

Actually, looking at the code above, I should have changed 1024 to 2048. Do that, and it just freezes immediately. Or change the line in loop to read:

 Serial.print ("Available memory = " );
 Serial.println (availableMemory ());

And you get some odd displays.

My guess here is that the malloc library is not being given a correct figure for available memory, and thus overwrites the stack. In your case, after a few times around the loop.

Bear in mind that in clock_GetNumberName you create something like half-a-dozen strings (all that concatenation).

which I haven't yet been able to track down the source for (if anyone knows exactly where? .... hint hint).

kiwimew:

Programmers these days have no real idea about computing resources.

Not entirely sure I agree. As a professional C++ software engineer I am conscious of having a finite heap -- my code must delete what it new's (we're not dealing with a garbage-collecting language here). And I expect library code to do the same. The String class can't leak. And in this context I would've thought String should have an embedded-friendly allocation strategy (assuming fragmentation is the issue). Or at least have clearly stated caveats about its applicability to Arduino development.

Having said that, one of the things I like about Arduino dev is the limited resources. It takes me back to the days of writing 6502 assembler for my Acorn Electron.

I didn't intend to offend or insult you, it's just my observation. Programmers today don't need to bother how a stack frame is built, about memory fragmentation due to short strings and when an object is copied and when references are used. They might have heard about it and know it in theory, but they don't live it and when it bytes them, they're confused. To be honest, as they don't need it because there are enough resources, it only makes sense to spend the time on more important things.

As to the String library included with the Arduino, I think it's a piece of garbage. Or put it less negative, it's implemented in a naive way, well enough to offer basic functionality to beginners but with far too many rough edges and missing too many useful features (for example handling PROGMEM transparently). Before anyone asks, I'm not interested in spending time on it, I don't use anyway. To me it offers too little benefit for the small amount of strings used on the Arduino.

Korman

According to the Arduino Devloper's list, the string library has known bugs, so I wouldn't go looking as deep as malloc() yet (although at least one of these claims that the malloc() shipped with Arduino has bugs.)

http://code.google.com/p/arduino/issues/detail?id=411&q=string
http://code.google.com/p/arduino/issues/detail?id=451&q=string
http://code.google.com/p/arduino/issues/detail?id=468&q=string

(One of the big problems pointed out is that AVR-land has essentially no error handling capability. If a string operation fails due to lack of memory (or any other reason), there is nothing to DO about it other than go on behaving badly.)

Thanks for the link about malloc.

So, the string library may have bugs, and it uses malloc, which may have bugs. All the more reason to stick to stuffing stuff into simple buffers. With 2048 bytes of RAM, some already taken by global variables and the stack, the amount of memory left for strings must be quite small. Like, 100 x 20 byte strings would do it. Plus the overhead of the string object itself. And the doubt that must be in some people's minds about whether the malloc library is being given the correct amount of free space to start with. This in itself would be a moving target, as the heap and the stack will eventually collide, and you can't know how big the stack will grow.

... It takes me back to the days of writing 6502 assembler ...

It takes me back to the days of writing 6800 assembler. In hex.

This was my first PC:

And this is how you programmed it:

You looked up the hex codes and typed them in. And with 512 bytes of memory you learned to use it wisely. :slight_smile: