[solved] I must be blind... don't see why it doesn't work

Hi,

Setup is a 2x16 lcd.

I have this function which is working correctly:

void lcdText(String firstLine, String secondLine){       
  while (firstLine.length()<16) {
    firstLine=firstLine+" ";                                               //add spaces at the end of firstLine until has a length of 16 characters
  }
  while (secondLine.length()<16) {
    secondLine=secondLine+" ";
  }
  lcd.setCursor(0,0);
  lcd.print(firstLine);
  lcd.setCursor(0,1);
  lcd.print(secondLine);
}

Now I'd like to replace it by the one below so my text is always centered in the display.

Somehow this new version makes my sketch stall after running through this function once.

void lcdText(String firstLine, String secondLine){
  while (firstLine.length()<15) {
    firstLine=" "+firstLine+" ";                                                 // as long as firstLine is shorter than 15, add a space in front and a space in the back
  }
  if (firstLine.length()==15) {
        firstLine=firstLine+" ";                                                  // if necessary add one more space at the end so the length becomes 16
  }        
  
  while (secondLine.length()<15) {
    secondLine=" "+secondLine+" ";
  }
  if (secondLine.length()==15) {
        secondLine=secondLine+" "; 
  }        
  
  lcd.setCursor(0,0);
  lcd.print(firstLine);
  lcd.setCursor(0,1);
  lcd.print(secondLine);
}

Any ideas where it goes wrong?

You are doing a lot of awkward String manipulation. It is probable that you are running into a memory allocator bug, or simply running out of memory.
Which version of the Arduino IDE are you using? Try building with 1.0.4 and see if that helps.

There will be much less memory=hungry ways to achieve the centring or justification of your strings using plain old char arrays. If your sketch still wedges with 1.0.4, then I would advise asking for help re-designing to use "C strings" instead of String.

Any ideas where it goes wrong?

Yes

  1. You have not posted all the code so we can't see what feeds this.
  2. You are using strings and strings are buggy on the arduino because the memory they use is not recovered correctly. Especailly the way you keep creating new strings from old ones.

Thanks for your answers!

Grumpy_Mike:

  1. You have not posted all the code so we can't see what feeds this.

I don't think anyone on this forum will be foolish enough to read through about 1000 lines of code, even if it is reasonably well structured and commented (I think...)

OK, you're probably right, must be a String problem.

I HATE STRINGS, I HATE STRINGS, I HATE STRINGS.

Ok, now this is off my chest, anyone know of a good (readable) tutorial about strings/text manipulation somewhere on the net?

I wouldn't bother with string manipulation. Just calculate how many spaces you need before the string and print them out, then print the string out. Maybe use a for loop to print the spaces.

e: Although, I don't want to discourage you from coming up with your own way of doing it with string manipulation. Take a look at the String functions Arduino provides here - http://arduino.cc/en/Reference/StringObject. I think the concat method would be helpful for you.

Remember to pass strings around as references where possible, this prevents copies from filling memory with every nested call.
I have a new library which can solve the problem. As it seems the lcd class is already using the Print class, you may benefit from it as well.

Moving away from strings will help your memory situation a lot. cstrings can easily replace your Strings, str.length with strlen( str ) for example. And my library works with both.

I notice you want the spaces after the text, is this to overwrite previous text?

void LineOut( const String &str, char line ){
  char storage[ 17 ];
  GString gstr( storage );

  //Format line
  gstr.repeat( ' ', ( str.length() < 15 ) ? ( 16 - str.length() ) / 2 : 0 );
  gstr.print( str );
  gstr.repeat( ' ', 16 - gstr.count() );

  //Output to screen
  lcd.setCursor( 0, line );
  lcd.print( gstr );
}

void lcdText( const String &firstLine, const String &secondLine ){
  LineOut( firstLine, 0 );
  LineOut( secondLine , 1 );
}

The library is here: String/sprintf alternative specifically for Arduino. - Libraries - Arduino Forum

C strings are pretty much just character arrays with a 0 character on the end. You obviously know enough basic programming to do what it would take to mess with spaces. With a little bit of work you could do a simple version that just used character arrays. Here is one implementation of your first function.

// untested code

void lcdText(char *firstLine, char *secondLine){
  char outputArray[17];         // Make sure there is space for the null terminator
  int i;

  for (i = 0; i < 16 && firstLine[i] != 0; ++i)    // Copy the string to the outputArray
    outputArray[i] = firstLine[i];
  for (; i < 16; ++i)                              // Pad spaces onto the right
    outputArray[i] = ' ';
  outputArray[16] = 0;                             // Add the null terminator
  lcd.setCursor(0,0);
  lcd.print(outputArray);

  for (i = 0; i < 16 && secondLine[i] != 0; ++i)
    outputArray[i] = secondLine[i];
  for (; i < 16; ++i)
    outputArray[i] = ' ';
//  outputArray[16] = 0;    // Should still be null
  lcd.setCursor(0,1);
  lcd.print(outputArray);
}

There are C string functions like strcpy() and strlen() you could use to do some stuff for you, but it's pretty easy to understand it when you manipulate the strings yourself.

A quick reply before trying all the tips you guys gave here (THANKS!).

Yes, I use the spaces to overwrite previous text. If I use lcd.clear() it makes my text flash on the lcd.

I am pretty new to arduino but not to programming (I am of the Apple ][ generation so I have done lots of programming in Basic).

Have you tried just printing 16 spaces to the line that you are about to print to rather than clearing the LCD as a whole ?

yes I have, pretty much the same flashing.

But to solve this I'll work with a few variables so printing to LCD only happens once something changes instead of every loop.

=> toPrintToLCD
=> textOnLCD

if toPrintToLCD == textOnLCD then no need to print.

if toPrintToLCD == textOnLCD then no need to print.

Be sure to compare centered text to centered text, or un-centered text to un-centered text.

I don't think anyone on this forum will be foolish enough to read through about 1000 lines of code, even if it is reasonably well structured and commented (I think...)

Yes so what a smart person does is to write a small sketch that will compile that shows the error. The very act of doing this solves over 70% of problems.

PaulS:
Be sure to compare centered text to centered text, or un-centered text to un-centered text.

Of course... compare apples to apples.

My problem is solved (for now), moved from v1.0.3 to v1.0.4 (still hate Strings though :0)