A problem with measuring time in a for loop

Question

I am familiar with timing using millis() function, but it seems that I have run into a problem with a for loop. The thing is, I am comparing encoder readings with a value entered via 4x4 keypad. Since I have to enter more than one value, i placed all values from the keypad into an array. Then I created a for loop that will cycle through that array and compare each value with encoder readings, when encoder reached the entered keypad value an LED should light up for 2 seconds. This is where the problem occurs. When it reaches the set value the LED seems to light up for like a fraction of a second. My guess is that the loop is creating some trouble here. Maybe because the loop cycles so fast the LED doesn't have enough time to stay lit. What do you think?

When I tried this program without a for loop (with only one value entered from a keypad), it works fine. The code below has two arrays with values already entered, so no need or keypad. I used it just for testing. When I solve this problem I will add a keypad.

Please ask if you need any clarification of the code , and thank you in advance

#include <Encoder.h>
#include<Wire.h>
#include<LiquidCrystal_I2C.h>

int led1=14;
int led2=15;
unsigned long encoder;
unsigned long prev_value;
int array1[]={100, 200, 300, 400, 500, 600};
int array2[]={150, 250, 420};
int i,j,k,l;
int brV=6;
int brM=3;
int tolerance=10;
unsigned long oldTime=0.0;
unsigned long newTime;
int dt=2000;
bool testRange=true;
bool testRange2=true;

Encoder myEnc(2,3);
LiquidCrystal_I2C lcd1(0x27, 16, 2);


void setup() {
pinMode(led1,OUTPUT);
pinMode(led2,OUTPUT);
lcd1.init();
lcd1.backlight();
}

void loop() {
  newTime=millis();
  lcd1.setCursor(0,0);
  encoder=myEnc.read()*446/400;
  if(encoder!=prev_value){
    lcd1.clear();
    delay(10);
    prev_value=encoder;
  }
  lcd1.print(encoder);
  digitalWrite(led1,LOW);
  digitalWrite(led2,LOW);

  k=brV+brM;

  for(l=0;l<k;l++){
     for(i=0;i<brV;i++){
        if(encoder>=array1[i] && encoder<=(array1[i]+tolerance) && testRange){
           oldTime=newTime;
           digitalWrite(led1,HIGH);
           testRange=false;
         }
         else if(oldTime-newTime >= dt){
          oldTime=newTime;
          digitalWrite(led1,LOW);
         }
         if(encoder>(array1[i]+tolerance){
          testRange=true;
         }
     }
      for(j=0;j<brM;j++){
         if(encoder>=array2[j] && encoder<=(array2[j]+tolerance) && testRange2){
           oldTime=newTime;
           digitalWrite(led2,HIGH);
           testRange2=false;
         }
         else if(newTime-oldTime >= dt){
          oldTime=newTime;
          digitalWrite(led2,LOW);
         }
         if(encoder>array2[j]+tolerance){
          testRange2=true;
         }
      }
  }


  }

Answer 1

in loop() replace

digitalWrite(led1,LOW);

with

if (newTime - oldTime >= dt){
   digitalWrite(led1,LOW);
}

and you can remove the

else if(oldTime-newTime >= dt){
   oldTime=newTime;
   digitalWrite(led1,LOW);
}

from the for loop

Answer 2

Old time and new time are wrong way round on a part where the LED'd are switched off

else if(oldTime-newTime >= dt){
          oldTime=newTime;
          digitalWrite(led1,LOW);
         }

When simple processors reach the max or min value and continue counting, they overflow and start counting from far end: So 5-8=maxint(largest number)-3 Result is that the difference is a massive number greater then 2000 except every few weeks when millis() overflows and become larger then Unsigned large int can store and starts counting from zero. (many codes fail working on that occasion, your would only works then ;-)
Correct version:

else if(newTime-oldTime >= dt){
          oldTime=newTime;
          digitalWrite(led1,LOW);
         }

Although I have noticed that testrange (both) only returns its value onto true on limited circumstance if input from encoder increased considerably very quickly to catch-up with growing inputs, so the next values will never pass condition to switch them on

if(encoder>=array1[i] && encoder<=(array1[i]+tolerance) && testRange)

will never return true because testRange stays false. Add

testRange=true; testRange2=true;

at the end of the loop otherwise it does return into true for next loop()iteration. I guess question is if you want other functions to run while LEDs are on or not.

It is a bad habit to make unnecessary variables like i,j,k,l global as they waste memory even when not needed you can define them within structure itself Eg: for(int i=1;... this way as soon as for, while etc loop (brackets close)finishes you get your memory back and you can use variable again