Hi,
I'm having issues where by if I continually swipe a card over the RFID reader in reasonably quick succession, the unit locks up.
I have a feeling it is an issue with the rfidRead() function, however this is nothing more than a gut feel.
Can anyone see anything obviously wrong with my code?
Code is too long to paste here, so I've put it on pastebin: #include <SPI.h>#include <Ethernet.h>#include <LiquidCrystal.h>#define T - Pastebin.com
Ok, after some playing, it seems to be something to do with this code, as removing it removes the issue:
char nameLen[4];
nameLen[3] = '\0';
client.find("NameLen: ");
client.readBytes(nameLen, 3);
int iNameLen;
iNameLen = atoi(nameLen);
char name[iNameLen+1];
name[iNameLen] = '\0';
client.find("Name: ");
client.readBytes(name, iNameLen);
clear2and3();
lastNamePrint = millis();
lcd.setCursor(0, 2);
lcd.print(centerText(name));
I've finally traced it down to the centerText() function, however I'm failing to see what the issue is
String centerText( String text, int width=LCDWIDTH )
{
int strlen = text.length();
if( strlen < width )
{
int padding = (width-strlen) /2;
String space = String(" ");
for(int i=1;i<=padding;i++)
{
text = String(space + text + space);
}
if( text.length() < width )
text = String( text + space );
}
return text;
}
String space = String(" ");
Three calls to the String constructor here. One for the left side, one for the right side, and one to the copy constructor.
String space = " ";
Invokes ONE call to the constructor.
for(int i=1;i<=padding;i++)
{
text = String(space + text + space);
}
Suppose that the original text was "MIke" and that the width was 16. 16 - 4 = 12. 12/2 = 6, so this loop will be executed 6 times. That's 6 calls to the constructor for the right side, and 6 calls to the copy constructor.
A statically define char array would waste far less memory, and be much easier to manipulate.
Why, by the way, do you need to add space AFTER the name?
Ditch the String class altogether.
You overuse the string class there, too many temporaries are created, each one calls malloc which cuts your memory into pieces. If the system is interrupt driven, quick swipes could instantiate multiple centerText functions which will devour your memory.
Like PaulS said, ditch the string class and replace with well managed char arrays for more efficient code.
Thanks for the tips guys, that makes a lot of sense.
As for why I need space after the name, whilst in normal circumstances the line would be cleared before anything would be written to it again, in the case that it isn't, those extra spaces ensure that there are no leftover characters from the previous text.
This function was born purely out of laziness if I'm honest, I think what I'm going to do is just clear the line and then work out the padding from the left as an int to determine where the cursor should be placed rather than butchering the original string.
This function was born purely out of laziness if I'm honest, I think what I'm going to do is just clear the line and then work out the padding from the left as an int to determine where the cursor should be placed rather than butchering the original string.
Simplest thing to do is determine padding, as you are doing now, and then simply print that number of spaces to the LCD, then print the string, then print some more spaces.
Well, this was my solution in the end - as I'm selectively clearing the lines I need to before calling centerText(), there's no point printing a load more wasted spaces to the LCD
void centerText( String text, int row, int width=LCDWIDTH )
{
int strlen = text.length();
if( strlen < width && width-strlen > 2 )
{
int padding = (width-strlen) /2;
lcd.setCursor(padding, row);
lcd.print(text);
}
else
{
lcd.setCursor(0, row);
lcd.print(text);
}
}
What happens with that code if the 1st name is Christina and the next name is Sam?
As I said, because I'm clearing the line first, it's fine.
Because I have future scenarios where I will want to center it inside a smaller space (because the right portion of the line is displaying something else - hence why width is configurable), it's more practical to do it this way.
I could clear the width that is set, but quite often I'm clearing multiple lines, so clearing is handled with a different function.