Use encoder to turn neopixel on for 1 sec without delay, *SOLVED

What I want to do is assign each pixel to an encoder. I’ve written the first pixel/encoder as a test. I’m using
an interval timer (blinkwithoutdelay example) since I can’t use delays or interrupts with encoders.

Here’s a Youtube video for reference, I’m trying to emulate these arcade machine leds and build my own mini controller.

The code works, but I am noticing a quick blink at the time of every interval, even if I don’t stop rotating the encoder. I know it has something to do with the placement of the interval timer and the neopixel OFF code, but where should I put it to get rid of that blink?

**also, sometimes the led doesn’t stay on for the full interval, sometimes it turns on and off immediately.

//Use encoders to control neopixels. If encoder is moving, turn led on for 1 second.
//If encoder has stopped, turn led off. 


#define ENCODER_DO_NOT_USE_INTERRUPTS
#include <Encoder.h>
Encoder myEnc(2, 3);

#include <Adafruit_NeoPixel.h>
#define PIN 6
#define NUMPIXELS 3
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_BRG + NEO_KHZ400);
long previousMillis = 0; 
long interval = 1000;

void setup() {
  Serial.begin(9600);
  Serial.println("Basic NoInterrupts Test:");
  pixels.begin();
}

long position  = -999;

void loop() {
  unsigned long currentMillis = millis();
  long newPos = myEnc.read();
  
  if (newPos != position) {
    pixels.setPixelColor(0, pixels.Color(50,0,0));
      
    position = newPos;
     delay(10);
    Serial.println(position);
  }
    else {
     
     // After 1000ms, turn led off. Should I move this?
      
    if(currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;
    pixels.setPixelColor(0, pixels.Color(0,0,0));
  }
 }
 pixels.show();
}

All time variables must be declared unsigned long.

Instead of testing “currentMillis” after it has become invalid, test millis() directly, like this:

    if(millis() - previousMillis >= interval) {

@cam_prevail, please do not cross-post. Other thread removed.

Thank you, I couldn't figure out how to remove it myself. I didn't think it fit the other category.

So I changed all the times to unsigned longs and tested millis() directly like you suggested, but I still get the slight blink to “off” while both encoders are rotating. Also, both leds turn off at the same time, where they should actually turn off 1 second after their respective encoder has stopped moving. Any ideas?

//Use encoders to control neopixels. If encoder is moving, turn led on for 1 second.
//If encoder has stopped, turn led off. 


#define ENCODER_DO_NOT_USE_INTERRUPTS
#include <Encoder.h>
Encoder Enc1(2, 3);
Encoder Enc2(8, 9);
unsigned long currentMillis1;
unsigned long currentMillis2;

#include <Adafruit_NeoPixel.h>
#define PIN 6
#define NUMPIXELS 3
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_BRG + NEO_KHZ400);
unsigned long e1previousMillis; 
unsigned long e2previousMillis; 
const unsigned long e1interval = 1000;
const unsigned long e2interval = 1000;

void setup() {
  e1previousMillis = millis();
  e2previousMillis = millis();
  Serial.begin(9600);
  Serial.println("Basic NoInterrupts Test:");
  pixels.begin();
}

long position1  = -999;
long position2  = -999;

void loop() {
  currentMillis1 = millis();
  currentMillis2 = millis();
  long newPos1 = Enc1.read();
  
  if (newPos1 != position1) {
    pixels.setPixelColor(0, pixels.Color(50,0,0));
      
    position1 = newPos1;
     delay(10);
    Serial.println(position1);
  }
    else {
      if(millis() - e1previousMillis >= e1interval) {
    e1previousMillis = millis();
    pixels.setPixelColor(0, pixels.Color(0,0,0));
  }
 }

  long newPos2 = Enc2.read();
  
  if (newPos2 != position2) {
    pixels.setPixelColor(2, pixels.Color(50,0,0));
      
    position2 = newPos2;
     delay(10);
    Serial.println(position2);
  }
    else {
      if(millis() - e2previousMillis >= e2interval) {
    e2previousMillis = millis();
    pixels.setPixelColor(2, pixels.Color(0,0,0));
  }
 }
 pixels.show();
}

What is the purpose of delay(10) and print()? If time is critical, don't do those things.

Why is the following done after other stuff, like delaying and printing?

 pixels.show();

I threw delay(10) in there to try to solve the issue, I didn't think it would work anyways. There's no print(), but I am printing the encoders updated positions to the serial monitor for testing...

As for the pixels.show(), I experimented putting it different places. Originally I had a pixels.show() line after each pixels.setPixelColor line. Should I put it back the way it was?

        delay(10);
    Serial.println(position1);

This looks a lot like a useless delay, followed by a print.

pixels.show() should be placed immediately after any change.

Thanks for the tips. I have made the modifications you suggested (at least I think I did, haha). However, there isn’t a noticeable difference in performance. There still seems to be something wrong with the timing function I’m using to turn off the pixels.

Also, I edited the original post to include a youtube link of the effect I’m trying to emulate here.

//Use encoders to control neopixels. If encoder is moving, turn led on for 1 second.
//If encoder has stopped, turn led off. 


//#define ENCODER_DO_NOT_USE_INTERRUPTS
#include <Encoder.h>
Encoder Enc1(2, 3);
Encoder Enc2(8, 9);
unsigned long currentMillis1;
unsigned long currentMillis2;

#include <Adafruit_NeoPixel.h>
#define PIN 6
#define NUMPIXELS 3
Adafruit_NeoPixel pixels = Adafruit_NeoPixel(NUMPIXELS, PIN, NEO_BRG + NEO_KHZ400);
unsigned long e1previousMillis; 
unsigned long e2previousMillis; 
const unsigned long e1interval = 2000;
const unsigned long e2interval = 2000;

void setup() {
  e1previousMillis = millis();
  e2previousMillis = millis();
  Serial.begin(9600);
  Serial.println("Basic NoInterrupts Test:");
  pixels.begin();
}

long position1  = -999;
long position2  = -999;

void loop() {
  currentMillis1 = millis();
  currentMillis2 = millis();
  long newPos1 = Enc1.read();
  
  if (newPos1 != position1) {
    pixels.setPixelColor(0, pixels.Color(50,0,0));
      pixels.show(); 
    position1 = newPos1;
     
    //Serial.println(position1);
  }
    else {
      if(millis() - e1previousMillis >= e1interval) {
    e1previousMillis = millis();
    pixels.setPixelColor(0, pixels.Color(0,0,0));
    pixels.show();
  }
 }

  long newPos2 = Enc2.read();
  
  if (newPos2 != position2) {
    pixels.setPixelColor(2, pixels.Color(50,0,0));
     pixels.show();
       
    position2 = newPos2;
    //Serial.println(position2);
  }
    else {
      if(millis() - e2previousMillis >= e2interval) {
    e2previousMillis = millis();
    pixels.setPixelColor(2, pixels.Color(0,0,0));
     pixels.show();
  }
 }

}

If encoder is moving, turn led on for 1 second.

At present, there is nothing in the code to reset the "1 second start time" when or while the encoder is moving.

Any time you turn on the LED, or the encoder position is changing, you should reset the start time, for example like this:

  if (newPos1 != position1) {
    pixels.setPixelColor(0, pixels.Color(50,0,0));
    pixels.show();
    position1 = newPos1;
    e1previousMillis = millis();  //restart timeout
   }

That's exactly what I needed! Solved both problems. Thank you so much!

Another trick is to keep a flag variable (values 0/1) stating whether the LED is already on. If it is on, you can obviously skip this step:

    pixels.setPixelColor(0, pixels.Color(50,0,0));
    pixels.show();

Just be sure to turn the flag off when you turn the LED off.