Program space exceeded, how can I shrink it?

Hi all,

I have a big issue here, my program is exceeding the program space on my Arduino Uno V3 by a mere percent >:( it uses up 101% and I have no good idea how to reduce it.

My sketch is attached, so if someone likes to take a look and give me some sound advise, I'd really appreciate it.

To the program itself: It shall read in an NFC tag, get a directory from it and then start playing the corresponding album off the SD card inserted in the Adafruit Music Maker Shield.

So basically I have 3 components:

  1. Arduino Uno V3
  2. Adafruit Music Maker Shield
  3. SunFounder PN532 NFC Module

At the top I then of course include the following libraries:

// include the string header so we can use strcpy() and strcat()
#include <string.h>

// setup SPI, MP3 and SD libraries
#include <SPI.h>
#include <Adafruit_VS1053.h>
#include <SD.h>
// These are the pins used for the music maker shield
#define SHIELD_RESET -1 // VS1053 reset pin (unused!)
#define SHIELD_CS 7 // VS1053 chip select pin (output)
#define SHIELD_DCS 6 // VS1053 Data/command select pin (output)
#define CLK 13 // SPI Clock, shared with SD card
#define MISO 12 // Input data, from VS1053/SD card
#define MOSI 11 // Output data, to VS1053/SD card

// These are common pins between breakout and shield
#define CARDCS 4 // Card chip select pin
// DREQ should be an Int pin, see attachInterrupt() - Arduino Reference
#define DREQ 3 // VS1053 Data request, ideally an Interrupt pin
Adafruit_VS1053_FilePlayer musicPlayer = Adafruit_VS1053_FilePlayer(SHIELD_RESET, SHIELD_CS, SHIELD_DCS, DREQ, CARDCS);

// setup NFC Adapter
#include <PN532_SPI.h>
#include <PN532.h>
#include <NfcAdapter.h>

#define PN532_SCK (13) //changed from pin 2
#define PN532_MISO (12) //changed from pin 5
#define PN532_MOSI (11) //changed from pin 3
#define PN532_SS (10) //changed from 4

PN532_SPI pn532spi(SPI, PN532_SS);
NfcAdapter nfc = NfcAdapter(pn532spi);

for the rest of the sketch, please see the attached ino.

The topic I'd like to solve first, is how to get rid of the String-class usage in my function that reads in the NFC tag content:

boolean getNfcCardData() {
boolean usableContent = false; // in case the tag has everything we need to play an album - in essence there must be a message with the directory - we return this as true
NfcTag tag = nfc.read();
if (DEBUG) { Serial.print(F("Tag Type: "));Serial.println(tag.getTagType()); Serial.print(F("UID: "));Serial.println(tag.getUidString()); }

if (tag.hasNdefMessage()){
NdefMessage message = tag.getNdefMessage();

// If you have more than 1 Message then it wil cycle through them
int recordCount = message.getRecordCount();
for (int i = 0; i < recordCount; i++) {
NdefRecord record = message.getRecord(i);

int payloadLength = record.getPayloadLength();
byte payload[payloadLength];
record.getPayload(payload);

String payloadAsString = ""; // Processes the message as a string vs as a HEX value
for (int c = 0; c < payloadLength; c++) {
payloadAsString += (char)payload

;
      }

      String cleanString = payloadAsString;
      cleanString.remove(0,3);
      if (DEBUG) { Serial.print(F("content element ")); Serial.print(i); Serial.print(F(": ")); Serial.println(cleanString); }

      if (cleanString.charAt(spacePosition - 1) == 'D') {
        usableContent = true;
        String contentString = cleanString.substring(3);
        contentString.toCharArray(plrCurrentFolder, 9);
        if (DEBUG) { Serial.print(F("Directory: "));Serial.println(contentString); }
        usableContent = true;
      }
      if (cleanString.charAt(spacePosition - 1) == 'T') {
        String contentString = cleanString.substring(3);
        firstTrackToPlay = contentString.toInt();
        if (DEBUG) { Serial.print(F("Track: "));Serial.println(contentString); }
      }

      String uid = record.getId();
      if (uid != "") { if (DEBUG) { Serial.print(F("  ID: "));Serial.println(uid); } }
    }
  } else { // does the card have NDEF messages?
    if (DEBUG) { Serial.println(F("THIS CARD DOES NOT HAVE MESSAGES AT ALL - NOT USABLE FOR THE MP3 PLAYER"));}
    usableContent = false;
  }

  return (usableContent);
}

Any other tips and hints are also very much appreciated,

Chris

[main2ndTry.ino|attachment](upload://4w8x09Z7P65SHvSnXINfXim4Ylz.ino) (27.2 KB)
// include the string header so we can use strcpy()
#include <string.h>

Not necessary. The Arduino.h file already makes the strcpy() function available.

const uint8_t btnNext = 356;                    // The value we receive from analog input if the Next (aka Fast Forward) Button is pressed
const uint8_t btnPrev = 1000;                   // The value we receive from analog input if the Previos (aka Prev or Rewind) Button is pressed

Do you understand the range of values that fit in an unsigned 8 bit memory location?

You should have debugged each part of the code individually, before trying to combine them, so get rid of the DEBUG stuff.

      Serial.println(F("Couldn't find VS1053, do you have the right pins defined?"));

could be

      Serial.println(F("No VS1053; check pins"));

and save 34 bytes of flash.

Not much, but you aren't that for over the limit.

Make all of your messages as short as possible.

      String payloadAsString = ""; // Processes the message as a string vs as a HEX value
      for (int c = 0; c < payloadLength; c++) {
        payloadAsString += (char)payload[c];
      }

Quit pissing away resources on the String class. There is NOTHING that it can do that can't be done with c strings.

      int spacePosition = cleanString.indexOf(':');

Just how long ARE your strings?

uint8_t getVolume() {
  soundVolume = map(sensorValue, 0, 1023, 0, 100); // map the sensor value to a sound volume

How much loss would there be just dividing sensorValue by 10, and not using map()?

void printDirectory(File dir, uint8_t numTabs) {

Do you really need this function? It does nothing, as far as the required functionality of the program is concerned.

void errorLed(void) {
  if (DEBUG) { Serial.println(F("error led is flashed")); }
  for (uint8_t i = 0; i < 5; i++) {
    analogWrite(errorPin, 0); delay(5); analogWrite(errorPin, 255); delay(5);
  }
}

Why do you need to use analogWrite() to turn the pin on or off?

How about removing all debug print statements?

Only add debug statements in the parts WHERE and WHEN you are debugging.

I mean lines like this can be commented out or removed if not used actively

if (DEBUG) { Serial.println(F("error led is flashed")); }

PaulS:

// include the string header so we can use strcpy()

#include <string.h>



Not necessary. The Arduino.h file already makes the strcpy() function available.

Ok, I got rid of it

PaulS:

const uint8_t btnNext = 356;                    // The value we receive from analog input if the Next (aka Fast Forward) Button is pressed

const uint8_t btnPrev = 1000;                   // The value we receive from analog input if the Previos (aka Prev or Rewind) Button is pressed



Do you understand the range of values that fit in an unsigned 8 bit memory location?

Ok, I changed all of those and most of the functions to make use of either char or byte

PaulS:
You should have debugged each part of the code individually, before trying to combine them, so get rid of the DEBUG stuff.

      Serial.println(F("Couldn't find VS1053, do you have the right pins defined?"));

could be

      Serial.println(F("No VS1053; check pins"));

and save 34 bytes of flash.

Not much, but you aren't that for over the limit.

Make all of your messages as short as possible.

I removed most of the debugging output and made it all in all shorter

PaulS:

      String payloadAsString = ""; // Processes the message as a string vs as a HEX value

for (int c = 0; c < payloadLength; c++) {
       payloadAsString += (char)payload[c];
     }



Quit pissing away resources on the String class. There is NOTHING that it can do that can't be done with c strings.

THIS is exactly the part I struggle the most with. You need to understand that this is my first program on a micro controller and I really have no clue as how to exchange the String class object with cstrings in affirmer code.

PaulS:

      int spacePosition = cleanString.indexOf(':');

Just how long ARE your strings?

These "strings" are essentially very short. They only contain one of two things:

  • a D: followed by the directory name which is exactly 8 chars wide.
  • a T: followed by track number 1 to 127 (this is now restricted to 127 because I changed the return type from uint8_t to char to have a -1 for an error and positive values for the next track number to be played in my functions.

However, as mentioned above, I have no idea how to get rid of the tedious String operation :frowning:

PaulS:

uint8_t getVolume() {

soundVolume = map(sensorValue, 0, 1023, 0, 100); // map the sensor value to a sound volume



How much loss would there be just dividing sensorValue by 10, and not using map()?

I'm not sure how much loss would this cause but I will certainly give it a try.

PaulS:

void printDirectory(File dir, uint8_t numTabs) {

Do you really need this function? It does nothing, as far as the required functionality of the program is concerned.

I see your point. I will get rid of it, when I'm done with the programming and am certain the sketch works.

PaulS:

void errorLed(void) {

if (DEBUG) { Serial.println(F("error led is flashed")); }
 for (uint8_t i = 0; i < 5; i++) {
   analogWrite(errorPin, 0); delay(5); analogWrite(errorPin, 255); delay(5);
 }
}



Why do you need to use analogWrite() to turn the pin on or off?

What other possibilities would I have for this flashing? I derived it from the function warnLed() that does the dimming of the led.

I really appreciate your help and by following your guidance (except for the String-operations which I do not know how to do) I already shrieked the overall size of my sketch to 90% of program space (29258 Bytes) and 58% of dynamic space (1190 Bytes).

However, I would really like to get some more help with the String operations. How could I get rid of the String operation altogether?

Remember the messages I read off the NFC tag are always in the form:

   D: bluemc99

or something like it for a directory name or

   T: 1-127

for a track number between 1 and 127.

Again, many thanks for your help,

Chris

Ps.: Please find updated sketch attached

main2ndTry.ino (25.7 KB)

You didn't comment on the degree of success in achieving your target of size reduction is.

Ok, I changed all of those and most of the functions to make use of either char or byte

How are you fitting 356 or 1000 in a byte?

These "strings" are essentially very short.

Is "essentially very short" more or less than 255 characters? If less, then why is the variable that holds the position of a specific character in the string an int?

Using the smallest possible data type needs to be your focus all the time. Note, though, that the data has to fit in the type. 356 and 1000 do not fit in a byte, but the position of the comma in the string probably does.

I'm not sure how much loss would this cause but I will certainly give it a try.

None. You need to understand what the functions you are using do, before you decide that the function IS the one to use. map() will return a value between 0 and 100, when the input (as in your code) is in the range 0 to 1023. 0 to 10 map to 0. 11 to 20 will map to 1. This pattern, 10 values map to 1 value, for most of the ranges. There will be 2 ranges where 11 values map to 1 value. If this is important, use map(). If just dividing by 10 is good enough, just divide by 10. Far simpler, and less code.

I will get rid of it, when I'm done with the programming and am certain the sketch works.

Surely, by now, you know what is on the card...

What other possibilities would I have for this flashing?

Turning the pin on or off is what digitalWrite() does. The analogWrite() function is used when you want other than full on and/or full off.

However, I would really like to get some more help with the String operations. How could I get rid of the String operation altogether?

You are making a copy of the string so you can use the == operator. You could look at the source code for the String class, to see that the == operator calls the Equals() method, and that the Equals() method uses strcmp() to determine if the strings that the String instance wrap are equal, or not.

The other methods in the String class use functions defined in the string header file (which operate on strings) to perform the work on the wrapped string. Functions like strtok() (tokenize a string), strstr() (does a string contain a string), etc. can be used without wrapping strings in Strings.