Artefacts on display when adding seemingly unrelated code

Question

I have this simple program, where I am implementing a menu structure on an OLED display and a rotary encoder.

The code works functionally as intended, however when I add more menu items, then I get some artefacts on the display, in the lower right corner (see screenshots). The code that I am referring to is inside the block comment in initMenu(). When I uncomment that block, the artefacts start appearing, every other time I turn the knob on the encoder. Sometimes it temporarily disappears, or slightly changes its shape.

no artefacts when turning knob artefacts after uncommenting and turning knob

As I said, other than that, the code behaves as expected.
Could anyone maybe shed some light on why this happens?

#define DEBUG 1
// Display
#include "Adafruit_GFX.h"
#include "Adafruit_SSD1306.h"
#define OLED_RESET -1
#define SCREEN_WIDTH 128
#define SCREEN_HEIGHT 64
Adafruit_SSD1306 display;
// Rotary encoder
#include "RotaryEncoder.h"
#include "YetAnotherPcInt.h"
#define RTRY_ENC_SW A1
#define RTRY_ENC_CLK A2
#define RTRY_ENC_DT A3
RotaryEncoder encoder(RTRY_ENC_CLK, RTRY_ENC_DT);
void onSwitch();
void onRotate();
typedef struct MenuItem {
 String name;
 byte id = 0;
 boolean active = false;
 MenuItem* prevSibling;
 MenuItem* nextSibling;
 MenuItem* currentChild;
};
MenuItem autoModes;
MenuItem manualModes;
MenuItem settings;
MenuItem peakAverage;
MenuItem peakToPeak;
MenuItem rms;
MenuItem* currentItem;
MenuItem* activeMode;
void setup() {
#ifdef DEBUG
 Serial.begin(115200);
#endif
 initEncoder();
 initMenu();
 initDisplay();
}
boolean hasInputs = false;
boolean switchPressed = false;
void loop() {
 if (hasInputs == true) {
 hasInputs = false;
 handleButtonInputs();
 }
}
void initEncoder() {
 pinMode(RTRY_ENC_SW, INPUT_PULLUP);
 PcInt::attachInterrupt(RTRY_ENC_SW, onSwitch, FALLING);
 PcInt::attachInterrupt(RTRY_ENC_CLK, onRotate, CHANGE);
 PcInt::attachInterrupt(RTRY_ENC_DT, onRotate, CHANGE);
}
void initMenu() {
 autoModes.name = "Auto";
 autoModes.id = 1;
 autoModes.active = true;
 autoModes.prevSibling = &settings;
 autoModes.nextSibling = &manualModes;
 autoModes.currentChild = &peakAverage;
 manualModes.name = "Manual";
 manualModes.id = 2;
 manualModes.prevSibling = &autoModes;
 manualModes.nextSibling = &settings;
 settings.name = "Config";
 settings.id = 3;
 settings.prevSibling = &manualModes;
 settings.nextSibling = &autoModes;
/*
 peakAverage.name = "PAvg";
 peakAverage.id = 4;
 peakAverage.active = true;
 peakAverage.prevSibling = &rms;
 peakAverage.nextSibling = &peakToPeak;
 peakToPeak.name = "P2P";
 peakToPeak.id = 5;
 peakToPeak.prevSibling = &peakAverage;
 peakToPeak.nextSibling = &rms;
 rms.name = "RMS";
 rms.id = 6;
 rms.prevSibling = &peakToPeak;
 rms.nextSibling = &peakAverage;
 activeMode = &peakAverage;
*/
 currentItem = &autoModes;
}
void initDisplay() {
 display = Adafruit_SSD1306(SCREEN_WIDTH, SCREEN_HEIGHT, &Wire, OLED_RESET);
 display.begin(SSD1306_SWITCHCAPVCC, 0x3C);
 display.clearDisplay();
 display.setCursor(36, 24);
 display.setTextSize(2);
 display.setTextColor(1);
 display.print("Hello");
 display.display();
}
void handleButtonInputs() {
 if (switchPressed) {
 switchPressed = false;
 if (currentItem->id <= 3) {
 currentItem = currentItem->currentChild;
 printMenu();
 } else if (4 <= currentItem->id && currentItem->id <= 6) {
 if (activeMode->id != currentItem->id) {
 activeMode->active = false;
 currentItem->active = true;
 activeMode = currentItem;
 }
 currentItem = &autoModes;
 printMenu();
 }
 }
 RotaryEncoder::Direction direction = encoder.getDirection();
 if (direction == RotaryEncoder::Direction::CLOCKWISE) {
#ifdef DEBUG
 Serial.println("clockwise");
#endif
 currentItem = currentItem->nextSibling;
 printMenu();
 } else if (direction == RotaryEncoder::Direction::COUNTERCLOCKWISE) {
#ifdef DEBUG
 Serial.println("counterclockwise");
#endif
 currentItem = currentItem->prevSibling;
 printMenu();
 } else if (direction == RotaryEncoder::Direction::NOROTATION ) {
#ifdef DEBUG
 Serial.println("no rotation");
#endif
 // ?
 }
}
void printMenu() {
#ifdef DEBUG
 Serial.println(currentItem->prevSibling->name);
#endif
 display.clearDisplay();
 display.setTextSize(2);
 Serial.println(currentItem->prevSibling->name);
 int x1 = 64 - currentItem->prevSibling->name.length() * 6;
 if (currentItem->prevSibling->active) {
 display.setTextColor(0, 1);
 display.fillRect(x1 - 2, 0, 2, 16, 1);
 } else {
 display.setTextColor(1, 0);
 }
 display.setCursor(x1, 0);
 display.print(currentItem->prevSibling->name);
#ifdef DEBUG
 Serial.println(currentItem->name);
#endif
 int x2 = 64 - currentItem->name.length() * 6;
 int width = currentItem->name.length() * 12 + 10;
 if (currentItem->active) {
 display.setTextColor(0, 1);
 display.fillRect(x2 - 6, 19, width, 24, 1);
 } else {
 display.setTextColor(1, 0);
 display.drawRect(x2 - 6, 19, width, 24, 1);
 }
 display.setCursor(x2, 23);
 display.print(currentItem->name);
#ifdef DEBUG
 Serial.println(currentItem->nextSibling->name);
#endif
 int x3 = 64 - currentItem->nextSibling->name.length() * 6;
 if (currentItem->nextSibling->active) {
 display.setTextColor(0, 1);
 display.fillRect(x3 - 2, 47, 2, 16, 1);
 } else {
 display.setTextColor(1, 0);
 }
 display.setCursor(x3, 47);
 display.print(currentItem->nextSibling->name);
 display.display();
}
void onSwitch() {
 hasInputs = true;
 switchPressed = true;
}
void onRotate() {
 hasInputs = true;
 encoder.tick();
}

This is the output of the compilation process regarding program size:

Sketch uses 18198 bytes (59%) of program storage space. Maximum is 30720 bytes.
Global variables use 827 bytes (40%) of dynamic memory, leaving 1221 bytes for local variables. Maximum is 2048 bytes.

• Mat: (4 <= currentItem->id <= 6) is parsed as ((4 <= currentItem->id) <= 6) which is always true since comparisons evaluate to 0 or 1. You need to split it up ((4 <= currentItem->id) && (currentItem->id <= 6)).
• Edgar Bonet: How much free RAM do you have?
• Majenko: Looks like your stack is ramming into your heap.
• Thomas Hirsch: @EdgarBonet I updated my question regarding size. Would it help to split printMenu into smaller chunks? Put all constant Strings to PROGMEM?

Answer 1

Thanks for the comments about memory usage, I was able to solve this specific problem.

I have updated my program, and optimized it by splitting up the menu rendering, using smaller variable sizes where possible, and putting all Strings to PROGMEM using the F() macro. I also had a look at this page https://learn.adafruit.com/memories-of-an-arduino/measuring-free-memory and copied freeMemory() from there:

#define DEBUG 1
#define DEBUG_MEMORY 1
// Display
#include "Adafruit_GFX.h"
#include "Adafruit_SSD1306.h"
#define OLED_RESET -1
#define SCREEN_WIDTH 128
#define SCREEN_HEIGHT 64
Adafruit_SSD1306 display;
// Rotary encoder
#include "RotaryEncoder.h"
#include "YetAnotherPcInt.h"
#define RTRY_ENC_SW A1
#define RTRY_ENC_CLK A2
#define RTRY_ENC_DT A3
RotaryEncoder encoder(RTRY_ENC_CLK, RTRY_ENC_DT);
void onSwitch();
void onRotate();
typedef struct MenuItem {
 String name;
 byte id = 0;
 boolean active = false;
 MenuItem* prevSibling;
 MenuItem* nextSibling;
 MenuItem* currentChild;
};
MenuItem autoModes;
MenuItem manualModes;
MenuItem settings;
MenuItem peakAverage;
MenuItem peakToPeak;
MenuItem rms;
MenuItem* currentItem;
MenuItem* activeMode;
#ifdef DEBUG_MEMORY
#ifdef __arm__
// should use uinstd.h to define sbrk but Due causes a conflict
extern "C" char* sbrk(int incr);
#else // __ARM__
extern char *__brkval;
#endif // __arm__
int freeMemory() {
 char top;
#ifdef __arm__
 return &top - reinterpret_cast<char*>(sbrk(0));
#elif defined(CORE_TEENSY) || (ARDUINO > 103 && ARDUINO != 151)
 return &top - __brkval;
#else // __arm__
 return __brkval ? &top - __brkval : &top - __malloc_heap_start;
#endif // __arm__
}
#endif
void setup() {
#ifdef DEBUG
 Serial.begin(115200);
#endif
 initEncoder();
 initMenu();
 initDisplay();
}
boolean hasInputs = false;
boolean switchPressed = false;
void loop() {
 if (hasInputs == true) {
 hasInputs = false;
 handleButtonInputs();
 }
}
void initEncoder() {
 pinMode(RTRY_ENC_SW, INPUT_PULLUP);
 PcInt::attachInterrupt(RTRY_ENC_SW, onSwitch, FALLING);
 PcInt::attachInterrupt(RTRY_ENC_CLK, onRotate, CHANGE);
 PcInt::attachInterrupt(RTRY_ENC_DT, onRotate, CHANGE);
}
void initMenu() {
 autoModes.name = F("Auto");
 autoModes.id = 1;
 autoModes.active = true;
 autoModes.prevSibling = &settings;
 autoModes.nextSibling = &manualModes;
 autoModes.currentChild = &peakAverage;
 manualModes.name = F("Manual");
 manualModes.id = 2;
 manualModes.prevSibling = &autoModes;
 manualModes.nextSibling = &settings;
 settings.name = F("Config");
 settings.id = 3;
 settings.prevSibling = &manualModes;
 settings.nextSibling = &autoModes;
 peakAverage.name = F("PAvg");
 peakAverage.id = 4;
 peakAverage.active = true;
 peakAverage.prevSibling = &rms;
 peakAverage.nextSibling = &peakToPeak;
 peakToPeak.name = F("P2P");
 peakToPeak.id = 5;
 peakToPeak.prevSibling = &peakAverage;
 peakToPeak.nextSibling = &rms;
 rms.name = F("RMS");
 rms.id = 6;
 rms.prevSibling = &peakToPeak;
 rms.nextSibling = &peakAverage;
 activeMode = &peakAverage;
 currentItem = &autoModes;
}
void initDisplay() {
 display = Adafruit_SSD1306(SCREEN_WIDTH, SCREEN_HEIGHT, &Wire, OLED_RESET);
 display.begin(SSD1306_SWITCHCAPVCC, 0x3C);
 display.clearDisplay();
 display.setCursor(36, 24);
 display.setTextSize(2);
 display.setTextColor(1);
 display.print(F("Hello"));
 display.display();
}
void handleButtonInputs() {
 if (switchPressed) {
 switchPressed = false;
 if (currentItem->id <= 3) {
 currentItem = currentItem->currentChild;
 renderMenu();
 } else if (4 <= currentItem->id && currentItem->id <= 6) {
 if (activeMode->id != currentItem->id) {
 activeMode->active = false;
 currentItem->active = true;
 activeMode = currentItem;
 }
 currentItem = &autoModes;
 renderMenu();
 #ifdef DEBUG_MEMORY
 Serial.println(freeMemory());
 #endif
 }
 }
 RotaryEncoder::Direction direction = encoder.getDirection();
 if (direction == RotaryEncoder::Direction::CLOCKWISE) {
#ifdef DEBUG
 Serial.println(F("cw"));
#endif
 currentItem = currentItem->nextSibling;
 renderMenu();
 } else if (direction == RotaryEncoder::Direction::COUNTERCLOCKWISE) {
#ifdef DEBUG
 Serial.println(F("ccw"));
#endif
 currentItem = currentItem->prevSibling;
 renderMenu();
 } else if (direction == RotaryEncoder::Direction::NOROTATION ) {
#ifdef DEBUG
 Serial.println(F("nr"));
#endif
 // ?
 }
}
void renderMenu() {
 display.clearDisplay();
 printPrevSibling();
 printCurrentItem();
 printNextSibling();
#ifdef DEBUG_MEMORY
 Serial.println(freeMemory());
#endif
 display.display();
}
void printPrevSibling() {
#ifdef DEBUG
 Serial.println(currentItem->prevSibling->name);
#endif
 display.setTextSize(2);
 Serial.println(currentItem->prevSibling->name);
 byte x1 = 64 - currentItem->prevSibling->name.length() * 6;
 if (currentItem->prevSibling->active) {
 display.setTextColor(0, 1);
 display.fillRect(x1 - 2, 0, 2, 16, 1);
 } else {
 display.setTextColor(1, 0);
 }
 display.setCursor(x1, 0);
 display.print(currentItem->prevSibling->name);
}
void printCurrentItem() {
#ifdef DEBUG
 Serial.println(currentItem->name);
#endif
 byte x2 = 64 - currentItem->name.length() * 6;
 byte width = currentItem->name.length() * 12 + 10;
 if (currentItem->active) {
 display.setTextColor(0, 1);
 display.fillRect(x2 - 6, 19, width, 24, 1);
 } else {
 display.setTextColor(1, 0);
 display.drawRect(x2 - 6, 19, width, 24, 1);
 }
 display.setCursor(x2, 23);
 display.print(currentItem->name);
}
void printNextSibling() {
#ifdef DEBUG
 Serial.println(currentItem->nextSibling->name);
#endif
 byte x3 = 64 - currentItem->nextSibling->name.length() * 6;
 if (currentItem->nextSibling->active) {
 display.setTextColor(0, 1);
 display.fillRect(x3 - 2, 47, 2, 16, 1);
 } else {
 display.setTextColor(1, 0);
 }
 display.setCursor(x3, 47);
 display.print(currentItem->nextSibling->name);
}
void onSwitch() {
 hasInputs = true;
 switchPressed = true;
}
void onRotate() {
 hasInputs = true;
 encoder.tick();
}

Now the size is like this:

Sketch uses 18588 bytes (60%) of program storage space. Maximum is 30720 bytes.
Global variables use 761 bytes (37%) of dynamic memory, leaving 1287 bytes for local variables. Maximum is 2048 bytes.

However the improvement is not that great, and I am still left wondering, when the next problem will arise. I am still thankful for further insights besides using a bigger controller, because I am not quite done yet on this one.

If anyone has more ways for me to improve this program without sacrificing performance, as I will be adding a radio, and a microphone, I will accept that as an answer over this one.

• PMF: Does repeatedly printing the free memory show something suspicious? You have quite a bit of RAM left and you don't seem to have an recursive code, so I wonder where the memory would overflow. I also see no dynamic memory allocations.
• Mat: @PMF: the Adafruit SSD1306 library mallocs 1k of RAM in this configuration for a screen buffer. Makes it very convenient but barely usable on a Uno. There are probably better libraries if all that is needed is printing some text.
• Thomas Hirsch: @PMF Yes, as @Mat pointed out, instantiating the display object eats a lot of RAM at once. I eventually resorted to using lower SCREEN_WIDTH and SCREEN_HEIGHT...