Timer in a for loop

Projects Programming
1

Hi,

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;
         }
      }
  }
  
  
  }
2

i don't oldTIme - newTime can be > 0 (which is greater)

and set oldTime = newTime once at the end of loop(). there's no need to do it multiple time in a loop.

3

You are correct, I've changed it now. But it still doesn't work. It still behaves in the same way

4

i don't understand your code.

i usually figure out how to do something and then expand it to do more. I don't see how you could have done this with the error i described - 1 LED, 1 condition to test and basic timing

I don't understand why you have nested for loops to control each LED. while there may be multiple conditions for setting or clearing an LED, I would evaluate all those conditions completely and then decide how to set the LED.

why do you set both LEDs LOW near the top of loop()?

5

This is just a prototyping program, the nested for loops are necessary, and LEDs are set to LOW in the beginning so that they don't remain HIGH the whole time, before I added those two lines they wouldn't turn off at all.

6

aren't all program prototypes until they are finished.

it's not obvious why the for loops are needed. Not saying they aren't needed.

i am suggesting you test your logic for controlling the LEDs without the loops. Shouldn't your logic turn the LEDs off after "dt".

if you turn the LEDs off each loop() cycle, why would you expect them to remain on for some period of time?

7

In trying to understand your code I decomposed it. By that point I had rewritten it. This compiles however I can't test it right now but it should work, or be close to it, because it is simple. The idea is that is every iteration of loop() tests the encoder to see if it is within range for each element of the two arrays you provided. If so it turns on the appropriate LED, sets the old time, and exits the for loop. The way this code is written the LED that is turned on will stay on until the encoder goes out of range + 2 seconds. However, it could easily be modified to control the LED to go off after 2 seconds and ignore the subsequent readings.

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

const int led1 = 14;
const int led2 = 15;
int array1[] = {100, 200, 300, 400, 500, 600};
int array2[] = {150, 250, 420};
int brV = sizeof(array1)/sizeof(array1[0]);
int brM = sizeof(array2)/sizeof(array2[0]);
const int tolerance = 10;
unsigned long oldTime;
unsigned long dt = 2000; // 2 seconds

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


void setup() {
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  digitalWrite(led1, LOW);
  digitalWrite(led2, LOW);
  lcd1.init();
  lcd1.backlight();
  oldTime = millis();
}

void loop() {
  unsigned long encoder;
  unsigned long prev_value;
  bool ledOn = false;

  lcd1.setCursor(0, 0);
  encoder = myEnc.read() * 446 / 400;
  if (encoder != prev_value) {
    lcd1.clear();
    delay(10);
    prev_value = encoder;
  }
  lcd1.print(encoder);

  for (int i = 0; i < brV; i++) {
    if (encoder >= array1[i] && encoder <= (array1[i] + tolerance)) {
      oldTime = millis();
      digitalWrite(led1, HIGH);
      ledOn = true;
      break;
    }
  }
  
  for (int j = 0; j < brM; j++) {
    if (encoder >= array2[j] && encoder <= (array2[j] + tolerance)) {
      oldTime = millis();
      digitalWrite(led2, HIGH);
      ledOn = true;
      break;
    }
  }

  if (ledOn && millis() - oldTime >= dt) {
    ledOn = false;
    digitalWrite(led2, LOW);
    digitalWrite(led1, LOW);
  }
}
8

Thank you for the reply and for your effort. It looks like this will work perfectly!