2 Potentiometers controlling a single LED

This code is designed to continuously blink an LED with Pot1 controlling timeOn and Pot2 controlling timeOff. When the button is pushed, the sequence starts and loops, however once running, the delays in the loop prevent the button from toggling off and keeping the LED low.

I know to make this work I need to use millis() so the code can loop freely without interfering with the button, but I'm having great difficulty controlling the single LED's on and off time with 2 Pots using millis().

Any and all help is greatly appreciated! Here's the reference code. It works only every few button pushes...

int led = 2;
int Button = 3;
int pot1 = A4;
int pot2 = A5;
boolean toggle = false;

void setup() {
pinMode(led, OUTPUT);
pinMode(Button, INPUT_PULLUP);
}

void loop() {
int B = digitalRead(Button);        
int timeOn = analogRead(pot1);
timeOn = map(timeOn, 0, 1023, 50, 500);
int timeOff = analogRead(pot2);
timeOff = map(timeOff, 0, 1023, 50, 500);  

if (B == LOW){toggle = !toggle; delay(200);}
if (toggle == true){
  digitalWrite(led, HIGH); 
  delay(timeOn); 
  digitalWrite(led, LOW); 
  delay(timeOff);}
if (toggle == false){digitalWrite(led, LOW);}
}

Try here

I tried posting there but the replies seemed to fall into some sour discussion about my intentions for the project.

Hello
I think the IDE example named BlinkWithOutDelay will help.
Arduino - BlinkWithoutDelay.

Start with File->Examples->02.Digital->BlinkWithoutDelay

When the LED switches from ON to OFF, take that opportunity to change the 'interval':

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
      interval = map(analogRead(pot1), 0, 1023, 50, 500);
    } else {
      ledState = LOW;
      interval = map(analogRead(pot2), 0, 1023, 50, 500);
    }

Of course, if you do that without changing anything else you will get an error: "assignment of read-only variable 'interval'". That means "Take the 'const' keyword off of the declaration of 'interval' if you want to be able to change it!"

From all of these suggestions (and more from another thread) I've cracked it.
Toggling an LED with ON & OFF interval potentiometers.

int timeOn = A3;
int timeOff = A7;
int Button = 4;
int led = 6;
boolean toggle = false;
unsigned long timeLedChanged = millis();
unsigned long period;
boolean ledOn = false;

void setup() {
  pinMode(led, OUTPUT);
  pinMode(Button, INPUT_PULLUP);
  digitalWrite(led, LOW);
}

void loop() {
int B = digitalRead(Button);        
int onVal = analogRead(timeOn);
onVal = map(onVal, 0, 1023, 50, 500);
int offVal = analogRead(timeOff);
offVal = map(offVal, 0, 1023, 50, 500);  
 
if (B == LOW){toggle = !toggle; delay(200);}  

if (toggle== true){ 
  if (millis() - timeLedChanged >= period){
    ledOn = !ledOn;
    timeLedChanged = millis(); 
    digitalWrite(led, ledOn); 
      if (ledOn){
      period = (onVal);}
      else{
      period = (offVal);}
     }  
     }
if (toggle == false){
ledOn = false; 
digitalWrite(led, LOW);}    

}

Thanks for all the help everyone!

If I understand the "intent" of your code correctly the button controls whether the LED is ON (and blinking based on potentiometers) or whether the LED is OFF all of the time. If you want the button to be responsive you will have to remove the delays and use millis().

That's the correct function yes. Each example of millis() I've seen uses variables based on the time between changes. I think my issue is storing 2 lots of data from the pots and having them act upon the LED simultaniously, which I find easiest with delays.

The following tutorials should help you out:

State Change Detection

Using millis() for timing

1 Like

That's a fantastic link, thankyou ToddL1962!

You can use a little state machine which will make the code very easy to manage.

@CHARH
If you want that the LED should bink only once when button is closed and released, then your sketch requires following modification that involes the resetting of the variable state after one pass. (Current sketch of Post-1 will continuously blink LED and it does when button is closed and released only for once.)

int LED = 2;
int Button = 4;
int timeOn = A3;
int timeOff = A4;

boolean state = false;

void setup() 
{
 pinMode(LED, OUTPUT);
 pinMode(Button, INPUT_PULLUP);
}

void loop() 
{
 int B = digitalRead(Button);
 int onVal = analogRead(timeOn);
 onVal = map(onVal, 0, 1023, 50, 500);
 int offVal = analogRead(timeOff);
 offVal = map(offVal, 0, 1023, 50, 500);

 if (B == LOW) 
 {
   state = ! state; //Button closed change state
   delay(200);
 }
 if (state == true) 
 {
   digitalWrite(LED, HIGH);
   delay(onVal);
   digitalWrite(LED, LOW);
   delay(offVal);
   state = false;
 }
 /*
 if (state == false) 
 {
   digitalWrite(LED, LOW);
 }*/
}

Unless OP wants it to continuously blink until the button turns it off. That was the assumption of OP intent I made.

As @CHARH has not mentioned his intention clearly, any reasonable assumption is justified. However, the bahavior of the sketch of Post-1 as described in Post-7 has been found after uploading it into Arduino MEGA.

Sorry for the confusion folks, the code intends to be...

  1. button push to activate.
  2. continuous blink, with pot 1 controlling led HIGH time and pot 2 controlling led LOW time.
  3. button push to stop and keep the led LOW.

I understand millis to a degree, however it's the merging of these 2 parameters acting onto a single led using millis which has me a bit stumped.

GolamMostafa the issue with using delays is that the continuous running of the led sequence is interfering with my ability to use the button with a debounce time applied.

Here is a start. I did the button processing and created a skeleton for the state machine and timing.

const int LED = 2;
const int Button = 4;
const int timeOn = A3;
const int timeOff = A4;

enum StateEnum
{
  STATE_IDLE,
  STATE_LEDON,
  STATE_LEDOFF
};

StateEnum state = STATE_IDLE;

void setup() 
{
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT_PULLUP);
}

void loop() 
{
  static int lastButtonState = digitalRead(Button);
  static unsigned long prevMillis;
  
  int buttonState = digitalRead(Button);
  unsigned long onVal = analogRead(timeOn);
  onVal = map(onVal, 0, 1023, 50, 500);
  unsigned long offVal = analogRead(timeOff);
  offVal = map(offVal, 0, 1023, 50, 500);
  unsigned long currMillis = millis();

  if (buttonState != lastButtonState)
  {
    delay(50);
    if (buttonState == LOW)
    {
      if (state == STATE_IDLE)
      {
        // set up to start flashing
        prevMillis = millis();
        digitalWrite(LED, HIGH);
        state = STATE_LEDON;
      }
      else
      {
        // Turn off LED go to idle state
        digitalWrite(LED, LOW);
        state = STATE_IDLE;
      }
    }
    lastButtonState = buttonState;
  }

  switch (state)
  {
    case STATE_IDLE:
      break;

    case STATE_LEDON:
      // check to see if on time has expired.  If so then turn LED off and transition to STATE_LEDOFF
      break;

    case STATE_LEDOFF:
      // check to see if off time has expired.  If so then turn LED on and transition to STATE_LEDON
      break;
  }
}

@CHARH
Implementation using millis():

int LED = 2;
int Button = 4;
int timeOn = A3;
int timeOff = A4;
int B;

boolean state = false;

void setup()
{
  Serial.begin(9600);
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT_PULLUP);
}

void loop()
{
  B = digitalRead(Button);
  int onVal = analogRead(timeOn);
  onVal = map(onVal, 0, 1023, 50, 500);
  int offVal = analogRead(timeOff);
  offVal = map(offVal, 0, 1023, 50, 500);

  if (B == LOW)
  {
    state = ! state; //Button closed change state //true
    delay(200);
  }

  if (state == true)
  {
    digitalWrite(LED, HIGH);
    timeDelay(onVal);
    digitalWrite(LED, LOW);
    timeDelay(offVal);
  }
}

void timeDelay(unsigned long t)//500 000
{
  unsigned long prMillis = millis();
  while(millis() - prMillis <= t)
  {
    if(digitalRead(Button) == LOW)
    {
      digitalWrite(LED, LOW);
      state = !state;
    }
  }
}

However, leads to an infinite number of reasonable assumptions... otherwise known as a long, drawn out thread.

reasonAssump

From all of these suggestions (and more from another thread) I've cracked it.
Toggling an LED with ON & OFF interval potentiometers.

int timeOn = A3;
int timeOff = A7;
int Button = 4;
int led = 6;
boolean toggle = false;
unsigned long timeLedChanged = millis();
unsigned long period;
boolean ledOn = false;

void setup() {
  pinMode(led, OUTPUT);
  pinMode(Button, INPUT_PULLUP);
  digitalWrite(led, LOW);
}

void loop() {
int B = digitalRead(Button);        
int onVal = analogRead(timeOn);
onVal = map(onVal, 0, 1023, 50, 500);
int offVal = analogRead(timeOff);
offVal = map(offVal, 0, 1023, 50, 500);  
 
if (B == LOW){toggle = !toggle; delay(200);}  

if (toggle== true){ 
  if (millis() - timeLedChanged >= period){
    ledOn = !ledOn;
    timeLedChanged = millis(); 
    digitalWrite(led, ledOn); 
      if (ledOn){
      period = (onVal);}
      else{
      period = (offVal);}
     }  
     }
if (toggle == false){
ledOn = false; 
digitalWrite(led, LOW);}    

}

Thanks for all the help everyone!