Meggy Jr Aux LEDs madness

[mde]

I don’t know whether this is a Meggy library issue or an Arduino issue, but I think I must have gone mad and would appreciate any help!

Please compare these two sketches. As far as I understand these things, they should produce the same result: toggling between the two leftmost auxiliary LEDs every half a second.

#include <MeggyJrSimple.h>

void setup()
{
 MeggyJrSimpleSetup();
}

void loop()
{
  for (byte i=1; i<=2; i++)
  {
    SetAuxLEDs(i);
    delay(500);
  }
}

#include <MeggyJrSimple.h>

void setup()
{
 MeggyJrSimpleSetup();
}

void loop()
{
  SetAuxLEDs(1);
  delay(500);
  SetAuxLEDs(2);
  delay(500);
}

Only the first sketch (with the for loop) works. The result of the second sketch is that the only the second auxiliary LED comes on (and stays on). Interestingly, if I add a blink on the main display using DrawPx() into the second sketch, it does blink while the aux LED stays stuck.

• This topic was modified 4 years, 6 months ago by mde.

[Windell Oskay]

I am able to reproduce this issue. I think that your examples are correct, and should work (but do not) as you expect them to. I’m not completely certain if this bug has always been there or if it used to work correctly, but no longer does due to changes in the Arduino or compiler optimization settings or the actual optimization that is produced by those settings.

What you can do, as a workaround, is to set the aux LEDs only once within the loop. That will not let you build a complex delay()-driven application if everything happens within a single call of loop(). On the other hand, structuring your program with delay() is somewhat “worst” practice on Arduino.

If we build an example based on the Arduino “BlinkWithoutDelay” example sketch, we might have the following:

#include <MeggyJrSimple.h>

int ledState = LOW;            
unsigned long previousMillis = 0;
const long interval = 500;

void setup() {
  MeggyJrSimpleSetup();
  SetAuxLEDs(1);
}

void loop() {
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    if (ledState == LOW) {
      ledState = HIGH;
      SetAuxLEDs(2);
    } else {
      ledState = LOW;
      SetAuxLEDs(1);
    }
  }
}

And, that should work.

The root cause of the issue (in one sense) is the way that the compiler is optimizing the code. If I open up the platform.txt file within the Arduino application, I can edit the compiler.c.flags= statement and change the -Os optimization flag (optimizing for size) to -O0 (optimization off), and once I have done so, your code examples work correctly.

This bug can also be fixed properly by adding the “volatile” keyword within the declaration of the variables as follows:

In MeggyJr.cpp:
static volatile byte MeggyJr::AuxLEDs;

In MeggyJr.h:
static volatile byte AuxLEDs;

You can manually add these updates to the files in your Arduino library, or download new copies from the repository:
https://github.com/evil-mad/MeggyJrRGB

• This reply was modified 4 years, 6 months ago by Windell Oskay.

[mde]

Changing the library has indeed solved the problem :)
Thank you very much for your detailed response.

[Author]

Posts