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.

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.

(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)). — Mat
– Mat, Commented Jul 4, 2021 at 6:52
@EdgarBonet I updated my question regarding size. Would it help to split printMenu into smaller chunks? Put all constant Strings to PROGMEM? — Thomas Hirsch
– Thomas Hirsch, Commented Jul 4, 2021 at 8:56

Thomas Hirsch · Accepted Answer · 2021-07-04 10:13:22Z

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.

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. — PMF
– PMF, Commented Jul 4, 2021 at 14:03
@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. — Mat
– Mat, Commented Jul 4, 2021 at 16:34
@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... — Thomas Hirsch
– Thomas Hirsch, Commented Jul 4, 2021 at 21:55

Stack Exchange Network

Artefacts on display when adding seemingly unrelated code

1 Answer 1

Your Answer

Hot Network Questions

Artefacts on display when adding seemingly unrelated code

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions