5
\$\begingroup\$

I started an Arduino project that could execute instructions from an SD card file on the screen. I managed to do it, but another problem appeared: I can't print all the files from the SD card to the serial port to show it on the pc. I think it happened because of the lack of memory. Since I'm using an Arduino Nano, my sketch takes up 90% of the memory and 71% of the dynamic memory. I assumed so because the function works perfectly if I shorten the code I would like to hear suggestions on how to optimize my code to make it work again or hear what the problem is if I'm wrong.

Here is my code:

#include <Adafruit_GFX.h>
#include <Adafruit_ST7735.h>
#include <SPI.h>
#include <SD.h>

//SD:
//GND = GND
//VCC = 5v
//MISO = 12
//MOSI = 11
//SCK = 13
//CS = 10

//ST7735:
//GND = GND
//VCC = 5v
//DC = 7
//RES = 5
//CS = 6
//SDA = 11
//SCL = 13
//BLK #

Sd2Card card;
SdVolume volume;
SdFile root;

const int chipSelect = 10;
String mode;
int len;
#define TFT_CS     6
#define TFT_RST    5
#define TFT_DC     7
int x = 0;
int y = 0;
int color;
String codeStr = "Screen.SetObjectColor('0')\nScreen.SetCursor('120,32')\nScreen.SetTextSize('3')\nScreen.Print('SF')"; //code to execute
Adafruit_ST7735 tft = Adafruit_ST7735(TFT_CS, TFT_DC, TFT_RST);

String split(String text, String separator, int index){
//splits string by separator and returns string by the index
  for(int i = 0; i < index; i++){
    text = text.substring(text.indexOf(separator)+1,text.length());
  }
  return(text.substring(0,text.indexOf(separator)));  
}

void compile(String code) {
//executes command
  if (code.indexOf("Screen.Print(") != -1) {
    tft.setCursor(x, y);
    tft.print(code.substring(14, code.length()-2));
  } else if (code.indexOf("Screen.SetCursor(") != -1) {
    x = code.substring(18, code.indexOf(",")).toInt();
    y = code.substring(code.indexOf(",")+1, code.length()-2).toInt();
    tft.setCursor(x,y);
  } else if (code.indexOf("Screen.SetObjectColor(") != -1) {
    tft.setTextColor(code.substring(23, code.length()-2).toInt());
    color = code.substring(23, code.length()-2).toInt();
  } else if (code.indexOf("Screen.Fill(") != -1) {
    tft.fillScreen(code.substring(13, code.length()-2).toInt());
  } else if (code.indexOf("Screen.SetTextSize(") != -1) {
    tft.setTextSize(code.substring(20, code.length()-2).toInt());
  } else if (code.indexOf("Screen.DrawLine(") != -1) {
    tft.drawLine(x, y, code.substring(17, code.indexOf(",")).toInt(), code.substring(code.indexOf(",")+1, code.length()-2).toInt(), color);  
  }
}

void showFiles(File dir, int numTabs) {
  //i got this form SD example and it dose not work here but if i delete some code to clear memory it will
  while (true) {
    File entry =  dir.openNextFile();
    if (!entry) {
      dir.rewindDirectory();
      break;
    }
    for (uint8_t i = 0; i < numTabs; i++) {
      Serial.print('\t');
    }
    Serial.print(entry.name());
    if (entry.isDirectory()) {
      Serial.println("/");
      showFiles(entry, numTabs + 1);
    } else {
      Serial.print("\t\t");
      Serial.println(entry.size(), DEC);
    }
    entry.close();
  }
}

//functions that triggers from the serial port in void loop()
void state(){
  if (card.init(SPI_HALF_SPEED, chipSelect)) {
    Serial.print("Wiring is correct and a card is present.");
  }
}

void createFile(String text, String dir){
  File newFile = SD.open(dir, FILE_WRITE);
  newFile.print(text);
  newFile.close();
}

void readFile(String dir){
  File readingFile = SD.open(dir, FILE_READ);
  if (readingFile) {
    while (readingFile.available()) {
      Serial.write(readingFile.read());
    }
    readingFile.close();
  }
}

int countLines(String file){
  int del = -1;
  int count = 0;
  String string = SD.open(file, FILE_READ).readString();
  while(true){
    if(string.indexOf("\n", (del + 1)) == -1){
      return(count);
      break;
      }
    else{
      del = string.indexOf("\n", (del + 1));
      count++;
      }
    }
}

void setup() {
  Serial.begin(9600);
  tft.initR(INITR_MINI160x80);
  tft.setRotation(3);
  tft.fillScreen(ST7735_WHITE);
}
void loop() {
  if (!SD.begin(chipSelect)) {
    Serial.println("SD card mount failed!");
    while (true);
  }
  while (!Serial) {;}
  //gets input from the serial port
  String input = Serial.readString(); 
  if (input == "showfiles" or input == "sf") {
    //prints directory(don`t work)
    showFiles(SD.open("/"), 0);
  } else if (input == "state" or input == "s") {
    state();
  } else if (input == "clearinput" or input == "ci") {
    SD.remove("file.txt");
  } else if (input == "delete" or input == "d" or mode == "d") {
    mode = "d";
    delay(100);
    if (input!="d" and input!="delete"){
      SD.remove(input);
    }
  } else if (input == "deletefolder" or input == "df" or mode == "df") {
    mode = "df";
    delay(100);
    if (input!="df" or input!="deletefolder"){
      SD.rmdir(input);
    }
  } else if (input == "rename" or input == "rn" or mode=="rn") {
    mode = "rn";
    delay(1000);
    if (input!="rn" and input!="rename"){
      File renameFile = SD.open("file.txt");
      String text = "";
      while (renameFile.available()){
        text += (char)renameFile.read();
      }
      renameFile.close();
      createFile(text.substring(text.length()-len ,text.length()), input);
    }
  } else if (input == "create" or input == "c" or mode == "c") {
    mode = "c";
    delay(100);
    if (input!="c" and input!="create" and input.length()!=0){
      len = input.length();
      createFile(input, "file.txt");
    }
  } else if (input == "createfolder" or input == "cf" or mode == "cf") {
    mode = "cf";
    delay(100);
    if (input!="cf" and input!="createfolder"){
      SD.mkdir(input);
    }
  } else if (input == "read" or input == "r" or mode == "r") {
    mode = "r";
    delay(100);
    readFile(input);
  } else if (input == "cl" or mode == "cl") {
    mode = "cl";
    delay(100);
    if (input!="cl"){
      Serial.print(countLines(input));
    }
  }
  for(int i = 0; i < 4; i++){
  //compiles every line in text(i'll make it so that the file executes here)
    compile(split(codeStr, "\n", i));
  }
}
\$\endgroup\$
0

1 Answer 1

Reset to default
7
\$\begingroup\$

The code has multiple possible points of improvement with respect to the Arduino Nano's memory limitations.

Let's start by stating the current memory consumption. Compiling your sketch with PlatformIO and the configuration

[env:nanoatmega328]
platform = [email protected]
board = nanoatmega328
framework = arduino
lib_deps =
  adafruit/Adafruit ST7735 and ST7789 Library@^1.10.4
  arduino-libraries/SD@^1.3.0

gives

Checking size .pio\build\nanoatmega328\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=======   ]  66.4% (used 1360 bytes from 2048 bytes)
Flash: [========= ]  87.5% (used 26884 bytes from 30720 bytes)
Building .pio\build\nanoatmega328\firmware.hex
======== [SUCCESS] Took 10.00 seconds ========

1. Passing Strings by value instead of by reference

Function signatures like void compile(String code) are not good because they pass String code by value instead of by reference. This creates a copy of the original object and passes it. To avoid creating new temporary objects, pass them by reference. If you do not modify the underlying object, additionally mark it as const. E.g.: void compile(const String& code).

2. Reading files fully into memory

The function countLines() is just supposed to count the number of line endings in the file. For that, it nonchalantly reads the whole file into a String object at once.

int countLines(String file){
  int del = -1;
  int count = 0;
  String string = SD.open(file, FILE_READ).readString();
  while(true){
    if(string.indexOf("\n", (del + 1)) == -1){
      return(count);
      break;
      }
    else{
      del = string.indexOf("\n", (del + 1));
      count++;
      }
    }
}

You only have 2KB of RAM physically, and much less free heap memory due to the other statically allocated variables or dynamically allocated memory in the code. So reading a "larger" file of e.g. 4 Kilobyte, this function will absolutely always fail. Functions like these should stream the content of the file in smaller chunks, or even character by character if performance is of no concern over memory usage. Then, in that small chunk, the numbers of \n can be counted, and then the same buffer can be used to read the next chunk. The file does not have to be in memory all at once.

Also, in the code return(count); break;, the break is useless.

A simple less memory intensive replacement could e.g. be

unsigned countLines(String filename){
  unsigned count = 0;
  File file = SD.open(filename, FILE_READ);
  if (!file)
    return 0; // file didn't exist
  while (file.available()){
    char c = (char) file.read();
    if (c == '\n') {
        count++;
    }
  }
  return count;
}

Note that the type was also changed to unsigned.

Furthermore, since the "code execution" logic seems to be line based, a rather quick improvement is to also read the file by line, not as whole. Currently, you read the File in wholly and then have to use a split() method to get the individual lines again. This is extrelemy inefficient. Have a small line buffer of say 128 bytes (self imposed restriction for these "code" file scripts), read bytes into that until you find a newline or the buffer is exhausted (error), then process the read line. Reuse the same buffer for the enxt read.

3. Not using F() for flash strings

You have multiple constant strings in your code. Yet you do not wrap them in F() to make them be stored in program-memory (FLASH) instead of RAM. Each of those non-F() strings is eating away at your precious RAM. Read the Arduino documentation carfeully.

E.g.

    Serial.println("SD card mount failed!");

consumes 22 bytes of Flash and RAM, it should rather be

    Serial.println(F("SD card mount failed!"));

to only consume 22 bytes of Flash and 0 RAM.

4. Constructing tons of dynamic String objects

The famous blog post: The Evils of Arduino Strings highlights how using String objects (wrongly) can lead to memory overhead and heap fragmentation. Specifically, code like

    tft.print(code.substring(14, code.length()-2));

in compile() is constructing a fresh new sub string from the given string. This is highly inefficient and leads to memory duplication. In this specific case, you can just obtain a pointer to the raw character data of the code string using code.c_str(), forward it by the appropriate number of bytes (14) and then using the tft.write() overload where you can specify the number of characters to print. Then, no copied substring is made.

    tft.write(code.c_str() + 14, code.length() - 14 - 2);

One can also argue that you could rewrite the code to avoid String objects for the most part and only operate on (const) char*, aka raw C strings, instead.

Consider every usage of String::substring() of bad and under scrutiny.

5. Make your code file easy to parse

When I read

String codeStr = "Screen.SetObjectColor('0')\nScreen.SetCursor('120,32')\nScreen.SetTextSize('3')\nScreen.Print('SF')"; //code to execute

wich is the "code" you want to interpret at runtime, it seems unceesarily complicated to me. Why have things wrapped in quotes if they, why the long prefixes, etc. Simplification is king. You could e.g. imagine a format like

TEXTCOLOR 0
CURSOR 120 32
TEXTSIZE 3
PRINT SF

This will require less fancy parsing code.

6. Using ISO646 operator names

Code like

  } else if (input == "create" or input == "c" or mode == "c") {
    mode = "c";
    delay(100);
    if (input!="c" and input!="create" and input.length()!=0){
      len = input.length();
      createFile(input, "file.txt");
    }

uses the alternative operator names or and and. This is unusual to see in C/C++ code and almost reads like Python. You should prefer the standard || and && to not trip up the reader.

7. Using stringly typed variables

The String mode variable specifically is only ever assigned constant strings and checked for constant strings.

  } else if (input == "deletefolder" or input == "df" or mode == "df") {
    mode = "df";
    delay(100);
    if (input!="df" or input!="deletefolder"){
      SD.rmdir(input);
    }

As thus, using a String type for this is overkill. All you need is an enum that can keep track of the "mode number". This also reduces the possibility of assigning the String an unhandled value.

enum CLIMode {
  NO_MODE,
  DELETE_FILE,
  DELETE_FOLDER,
  RENAME_FILE,
  CREATE_FILE,
  CREATE_FOLDER
  // ...
};
CLIMode mode = NO_MODE;

8. Logic bugs

E.g., in

  } else if (input == F("rename") || input == F("rn") || mode == F("rn")) {
    mode = F("rn");
    delay(1000);
    if (input != F("rn") && input != F("rename")){
      File renameFile = SD.open("file.txt");
      String text = "";
      while (renameFile.available()){
        text += (char)renameFile.read();
      }
      renameFile.close();
      createFile(text.substring(text.length()-len ,text.length()), input);
    }
  } else if (input == F("create") || input == F("c") || mode == F("c")) {
    mode = F("c");
    delay(100);
    if (input != F("c") && input != F("create") && input.length() != 0){
      len = input.length();
      createFile(input, "file.txt");
    }

If the create xxxxx command was not executed beorehand, then the rename command will use the variable len, which was however never assigned before.

9. Unnecessary global variables

You have

int x = 0;
int y = 0;

Which is used in

  if (code.indexOf(F("Screen.Print(")) != -1) {
    tft.setCursor(x, y);
    //tft.print(code.substring(14, code.length()-2));
    tft.write(code.c_str() + 14, code.length() -2);
  } else if (code.indexOf(F("Screen.SetCursor(")) != -1) {
    x = code.substring(18, code.indexOf(",")).toInt();
    y = code.substring(code.indexOf(",")+1, code.length()-2).toInt();
    tft.setCursor(x,y);
  } else if (code.indexOf(F("Screen.SetObjectColor(")) != -1) {
  ..
  } else if (code.indexOf(F("Screen.DrawLine(")) != -1) {
    tft.drawLine(x, y, code.substring(17, code.indexOf(",")).toInt(), code.substring(code.indexOf(",")+1, code.length()-2).toInt(), color);  
  }

Why must the Screen.Print() command set the cursor again? Can't it not be expected that the user called Screen.SetCursor() before? Not setting the cursor again can make x and y become local variables of that function. You can also retrieve the last cursor position. Global state should be kept to an absolute minimum.

Using only a fraction of these recommendations, e.g. F() strings and reading a file by line instead of whole, already improves the RAM usage (which is the critical one) to

Checking size .pio\build\nanoatmega328\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [=====     ]  47.5% (used 972 bytes from 2048 bytes)
Flash: [========= ]  90.7% (used 27860 bytes from 30720 bytes)
Building .pio\build\nanoatmega328\firmware.hex

from a starting point of 66.4% used RAM. The dynamic usage of RAM during runtime will also be less. See https://pastebin.com/M3k8VUnV.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much! I don't know what I would do without your help. I'm new to C-like languages ​​(I used to write mostly in PHP) so that helped a lot \$\endgroup\$ Commented Aug 30, 2024 at 19:31

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.