Logging inactivity for 'screensaver' LED effect

I built a MIDI controller a few years ago consisting of a Teensy 3.2, four translucent arcade-style buttons with Neopixels underneath them, and a 3-way bank selector switch. I programmed it using my very basic Arduino skills at the time (if I'm honest I haven't spent that much time with Arduino since, mainly Python) and used it for a while before I moved on to other things.

It has two main functions: receiving a MIDI clock signal from Ableton and flashing LEDs to create a visual metronome at two different clock divisions (Banks A & B), and sending MIDI CC signals from each of the buttons (Bank C).

Now we're all stuck inside during COVID-19, I dug it out and am having a go at understanding my 17-year-old self's code, making it more robust, cleaner, and adding a few more nice little functions.

Currently, I'm trying to implement a 'screensaver' effect, where after a certain period of inactivity (no MIDI data being transferred in or out of the controller), the LEDs underneath the buttons switch into a rainbow 'chase' pattern, as is fairly common on modern DJ/Production gear. Then, when MIDI data starts being sent or received, the controller switches back to its operational state again. I've written the chase effect, but am struggling with logging inactivity. I've tried a few things, and this is where I am at the moment.

#include <Adafruit_NeoPixel.h>

#define PIN 15
#define N_LEDS 4

Adafruit_NeoPixel strip = Adafruit_NeoPixel(N_LEDS, PIN, NEO_GRB + NEO_KHZ800);

int j = 0;
int k = 0;
byte counter;
byte CLOCK = 248;
byte START = 250;
byte CONTINUE = 251;
byte STOP = 252;

bool togglestate[13];
bool MIDI_state = true;
int channel = 2;

int colours [6][3] = {
  {255, 0, 0},      //Red
  {0, 255, 155},    //Cyan
  {0, 0, 255},      //Blue
  {255, 155, 0},    //Yellow
  {0, 255, 0},      //Green
  {255, 0, 155}     //Magenta
};

int sequence [2][2] = {
  {24, 12}, //quarter notes
  {12, 6} //eighth notes
};

bool onOff [4] = {false, false, false, false};

int button1 = 19;
int button2 = 20;
int button3 = 21;
int button4 = 22;

int bankA = 16;
int bankB = 17;
int bankC = 18;

int lastBank = 0;

int screenSaver = 5; // minutes of inactivity before 'screensaver' kicks in
long lastActivityTime = millis();

void setup() {
  Serial.begin(9600);
  Serial.println("Setup Complete");

  strip.begin();
  usbMIDI.setHandleRealTimeSystem(RealTimeSystem);
  for (int i = 16; i <= 22; i++) {
    pinMode(i, INPUT);
  }

  //Run startup RGB animation
  uint32_t period = 3000;    // 3 seconds

  for (uint32_t tStart = millis();  (millis() - tStart) < period;  ) {
    rainbowCycle(7);
  }

  // Update display based on selected bank
  updateDisplay(whichBank());
}

void loop() {

  if (whichBank() == 1 or whichBank() == 2) {
    usbMIDI.read();
    //Serial.println("Clock Mode");
  }

  if (whichBank() == 3) {
    //Serial.println("MIDI Mode");

    if (digitalRead(button1)) {
      Serial.println("Button 1 Pressed, Bank " + whichBank());
      onOff[0] = true;
      int com = (((whichBank() - 1) * 4) + 1);
      MIDIout(com, channel);
      updateDisplay(whichBank());
      delay(100);
      while (digitalRead(button1));
      onOff[0] = false;
      updateDisplay(whichBank());
    }

    if (digitalRead(button2)) {
      Serial.println("Button 2 Pressed, Bank " + whichBank());
      onOff[1] = true;
      int com = (((whichBank() - 1) * 4) + 2);
      MIDIout(com, channel);
      updateDisplay(whichBank());
      delay(100);
      while (digitalRead(button2));
      onOff[1] = false;
      updateDisplay(whichBank());
    }

    if (digitalRead(button3)) {
      Serial.println("Button 3 Pressed, Bank " + whichBank());
      onOff[2] = true;
      int com = (((whichBank() - 1) * 4) + 3);
      MIDIout(com, channel);
      updateDisplay(whichBank());
      delay(100);
      while (digitalRead(button3));
      onOff[2] = false;
      updateDisplay(whichBank());
    }

    if (digitalRead(button4)) {
      Serial.println("Button 4 Pressed, Bank " + whichBank());
      onOff[3] = true;
      int com = (((whichBank() - 1) * 4) + 4);
      MIDIout(com, channel);
      updateDisplay(whichBank());
      delay(100);
      while (digitalRead(button4));
      onOff[3] = false;
      updateDisplay(whichBank());
    }
  }
  int temp = whichBank();
  if (lastBank != temp) {
    lastBank = temp;
    updateDisplay(temp);
  }
  // Run an activity check
  lastActivity();
  Serial.println(MIDI_state);
}

// =================== UTILITY ===================

void lastActivity() {

  if (millis() > (lastActivityTime + (60000 * screenSaver))) {
    MIDI_state = false;
    if (MIDI_state = false) {
      rainbowCycle(7);
    }
    else {
      lastActivityTime = millis();
    }
  }
  else {
    updateDisplay(whichBank());
  }
}

// =================== DISPLAY ===================

// Update the LEDs based on selected bank
void updateDisplay(int bank) {
  for (int i = 0; i < 4; i++) {
    if (!onOff[i]) {
      strip.setPixelColor(i, strip.Color(colours[bank * 2 - 2][0], colours[bank * 2 - 2][1], colours[bank * 2 - 2][2]));
    }
    else {
      strip.setPixelColor(i, strip.Color(colours[bank * 2 - 1][0], colours[bank * 2 - 1][1], colours[bank * 2 - 1][2]));
    }
    strip.show();
  }
}

// Determine which bank is currently selected
int whichBank() {
  if (digitalRead(bankA)) {
    //Serial.println("Bank A");
    return 1;
  }
  else if (digitalRead(bankB)) {
    //Serial.println("Bank B");
    return 2;
  }
  else if (digitalRead(bankC)) {
    //Serial.println("Bank C");
    return 3;
  }
}

// Run a rainbow cycle effect
void rainbowCycle(int SpeedDelay) {
  byte *c;
  uint16_t i, j;

  for (j = 0; j < 256; j++) {
    for (i = 0; i < N_LEDS; i++) {
      c = Wheel(((i * 256 / N_LEDS) + j) & 255);
      strip.setPixelColor(i, *c, *(c + 1), *(c + 2));
    }
    strip.show();
    delay(SpeedDelay);
  }
}

byte * Wheel(byte WheelPos) {
  static byte c[3];

  if (WheelPos < 85) {
    c[0] = WheelPos * 3;
    c[1] = 255 - WheelPos * 3;
    c[2] = 0;
  }
  else if (WheelPos < 170) {
    WheelPos -= 85;
    c[0] = 255 - WheelPos * 3;
    c[1] = 0;
    c[2] = WheelPos * 3;
  }
  else {
    WheelPos -= 170;
    c[0] = 0;
    c[1] = WheelPos * 3;
    c[2] = 255 - WheelPos * 3;
  }
  return c;
}

// =================== MIDI ===================

void RealTimeSystem(byte realtimebyte) { //receive and process midi clock messages from DAW
  if (whichBank() == 1 or whichBank() == 2) {
    if (realtimebyte == 248) {
      counter++;
      if (counter == sequence[whichBank() - 1][0]) {
        j++;
        if (j == 4) {
          j = 0;
        }
        counter = 0;
        //Serial.println("Beat");
        strip.setPixelColor(j, colours[whichBank() - 1][0], colours[whichBank() - 1 ][1], colours[whichBank() - 1][2]);
      }
      if (counter == sequence[whichBank() - 1][1]) {
        //Serial.println("Offbeat");
        strip.setPixelColor(j, 0, 0, 0);
      }
    }

    if (realtimebyte == START || realtimebyte == CONTINUE) {
      counter = 0;
      j = 0;
      Serial.println("Start");
      whichBank();
      strip.setPixelColor(j, colours[whichBank() - 1][0], colours[whichBank() - 1 ][1], colours[whichBank() - 1][2]);
    }
    if (realtimebyte == STOP) {
      Serial.println("Stop");
      whichBank();
      strip.setPixelColor(j, 0, 0, 0);
    }
  }
  else (whichBank());
  strip.show();
}

void MIDIout(int CC, int chan) { //send MIDI CC data to DAW
  Serial.println("MIDI out");
  Serial.println(MIDI_state);
  Serial.println("MIDI state true");
  if (togglestate[CC]) {
    usbMIDI.sendControlChange (CC, 0, chan);
    togglestate[CC] = false;
  }
  else {
    usbMIDI.sendControlChange (CC, 127, chan);
    togglestate[CC] = true;
  }

}

The rainbow cycle screensaver effect works wonderfully at startup, runs for 3 seconds, and then the controller settles into operation mode. However, after the allotted inactivity period, the screensaver effect does not trigger as expected. I did get it to trigger before, but then pressing a button or feeding the controller with a clock wouldn't break it out of the screensaver effect.

I'm fairly sure the error is in the utility section, and concerns how I'm treating the logging of inactivity.

Thanks for your help!

You missed a double equals...

    if (MIDI_state = false) {

This expression is therefore always FALSE.

Thanks for that! Getting closer to a solution. Having fiddled with it some more I've found a few things:

  1. Now the screensaver is on all the time, but exits into operation mode for as long as a button is held down.

With hindsight, I can see that my solution might be one of those 'you got what you asked for, but not what you wanted' problems. Really what I want to happen is the screensaver to turn off for good after it receives a MIDI signal, and not re-engage until a certain time has elapsed, within which there have been no MIDI signals at all.

  1. The loop for Banks A and B is very fast, but in Bank mode C, the main loop is far too slow (one every half-second or so). Presumably I need to take out most of that button-press code and package it in a more efficient function, preserving the loop speed - unless there's a better way to do this.

Any ideas?

A couple of other notes

long lastActivityTime = millis();

that should be an unsigned long variable

Your lastActivity() function is not doing millis() math properly. You can only subtract some previous time from the current time to guarantee accurate results even with rolloever.

void lastActivity() {

//  if (millis() > (lastActivityTime + (60000 * screenSaver))) {
  if (millis() - lastactivityTime >= 60000UL * screenSaver) {
   //...
}

Nice one, thanks. What's the meaning of the UL after 60000? Have seen it done a few times but wasn't quite sure why.

Also just realised that my Bank C loop is running slowly because of my 4 button debounce delays (duh!). All the more reason to get those button statements out the loop ASAP.

Not everything is working yet, but I'll keep chipping away at it!

fm34:
Nice one, thanks. What's the meaning of the UL after 60000? Have seen it done a few times but wasn't quite sure why.

It tells the compiler to treat the numeric constant as an unsigned long. Without it, the compiler will make the constant as small as possible (byte, int, long, long long). It most likely does not matter in this case but you did declare screenSaver as in int and not as a constant so... just making sure everything is large enough to do the entire expression.