Go Down

Topic: This simple program freezes. Why?? (0022 vs 0021?) (Read 2179 times) previous topic - next topic

kiwimew

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.

Code: [Select]
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.
// TODO: sig

Coding Badly

Quote
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...

Quote
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
}


kiwimew

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."
// TODO: sig



kiwimew

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.
// TODO: sig

Korman

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

spycatcher2k

Change the serial speed to 57600 - read somewhere that 38400 has issues.
Drew.
http://www.uk-pcb.co.uk - My UK Based PCB Fab & Assembly Company
Design work undertaken
SMD & Thru-Hole assembly

dafid

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.

Korman

Quote
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

kiwimew

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...

// TODO: sig

Korman


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

kiwimew

Quote
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.
// TODO: sig

westfw

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...)

Quote
I am conscious of having a finite heap

How conscious are you of the overhead of having dynamically allocated memory?
Which uses more memory: [font=Times]String s = "short string";[/font] or [font=Times] char s[20] = "short string";[/font] ?
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.

Nick Gammon

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:

Code: [Select]
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:

Code: [Select]
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:

Code: [Select]
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).
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Go Up