Newb old guy trying to set time on LED with button

Hello;
I'm brand new to programming and electronics. I've been reading for weeks and run several starter kit projects on my UNO.
Now I'm trying to write a sketch to control a "Shoot - Move - Shoot" motor on a camera slider rail.
The idea is to use one button to select a time value, toggling up by hundreds from 200 - 1000 milliseconds. This value would be used to light the LED on pin 13 and activate the motor on the same pin through a relay.
The second button would select the off time the same way but toggling in thousands from 5000 - 30000.
With these values looping, the motor will pulse on and off and advance the camera down the rail.

I have the relay working and some basic parts of the sketch. Where I need help is I don't know how to send the buttonPresses to a timer to turn the LED on and off. I'm obviously doing that line incorrectly. Any help is greatly appreciated.

Here is my code so far.

// the LED
const int ledPin = 13;
int ledState = 0;

// the Button
const int buttonPin1 = 7;

// Arbitrary LED function
int val;
int LEDfunction = 0;
int buttonState;
int buttonPresses = 0;

void setup()
{
  pinMode(ledPin,OUTPUT);  
  pinMode(buttonPin1,INPUT);
  Serial.begin(9600);
  buttonState = digitalRead(buttonPin1);
}

void loop() {
  
  val = digitalRead(buttonPin1);
  
  if (val != buttonState) {          // the button state has changed!
    if (val == LOW) {                // check if the button is pressed
      buttonPresses++;               // increment the buttonPresses variable
      Serial.print("Incrament ");
      Serial.println(buttonPresses * 100);
      digitalWrite(ledPin,HIGH);
      unsigned long millis(buttonPresses * 100); // ???timer reads button presses x 100
      digitalWrite(ledPin,LOW);
    }
  }
  buttonState = val;                 // save the new state in our variable
}

The code dealing with the timer doesn't make sense, but if you take lines 31 - 33 out then the button reading the button state looks OK. You aren't doing any debounce so you might find you have problems with the code seeing some spurious button presses, but you could deal with that if/when it happens.

I assume that you want to have some maximum count value and go back to zero so you can just 'go round again' if you overshoot. If so, the way to do that would be to reset buttonPresses back to zero if it exceeds some value.

The code you have doing the button handling is well designed and can be run repeatedly - it only does anything when the button input changes state. This means you can use it together with some non-blocking timing code to turn LEDs on and off, and the two parts of your code can work concurrently. The Blink without Delay' example sketch shows how to blink an LED using non-blocking code. You can take the code for that and put it below your existing code in loop. I suggest to start with you just take the blink code as it is, so you are using a fixed delay. Once that is working you can replace the constant blink duration with a value that you work out from your buttonPresses variable. The way I'd do that is to put the duration values in an array and use buttonPresses as an index into the array.

Thanks so much PeterH for your quick help! I now have the buttonPresses resetting at 10 and the example Blink Without Delay is inserted and working. I am now trying to get my head around Arrays as you suggested. Will post new code when I get to the next level or stuck again. Cheers.

I have updated the code with two millis timers and two arrays. The first array (increment) and timer are linked to buttonPin on digital 7. It works now for choosing a time duration for the LED on pin 13.
The second timer is not yet connected to buttonPin1. I am able to select values from the second array (increment1) using the button and print them. This is intended to select time values for the LED off portion of the cycle. In other words the loop should be: increment (on), increment1 (off), increment (on), etc.
I know the IF ELSE LED sequence creates the symmetrical on/off blinking. So that has to be replaced with something else. I've tried several things and I think its time for some help. I know the buttons need debouncing. That can wait.
Please have a look at what I have so far. Thanks to all.

//Sketch to create an asymetric blink pattern on an LED and relay.
//ON and OFF values selected from two arrays via two buttons.

const int ledPin = 13; // relay connected here
int ledState;
long previousMillis = 0;     // will store last time LED was updated
long previousMillis1 = 0;
const int buttonPin = 7;
const int buttonPin1 = 6;
int increment[] = {100, 200, 300, 400, 500, 600, 700, 800, 900, 1000}; // LED on times
int increment1[] = {1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, 10000}; // LED off times
int val; // saves the button state
int val1;
int buttonState;
int buttonState1;
int buttonPresses = 4;
int buttonPresses1 = 4;

void setup() {
  pinMode(ledPin,OUTPUT);  
  pinMode(buttonPin,INPUT);
  pinMode(buttonPin1,INPUT);

  Serial.begin(9600);
  buttonState = digitalRead(buttonPin);
  buttonState1 = digitalRead(buttonPin1);
  ledState = ledPin,HIGH; // start off with the LED on
  
}

void loop() {
  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();
   if(currentMillis - previousMillis > increment[buttonPresses]) 
   {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;
     // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;
 // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
   }
   
  val = digitalRead(buttonPin);
  if (val != buttonState) {     // the button state has changed!
    if (val == LOW) {                // check if the button is pressed
      buttonPresses++;               // increment the buttonPresses variable
      Serial.print("Increment ");
      Serial.println((buttonPresses + 1)*100); // print LED on time in milliseconds.
  
  if (buttonPresses >= 10) // reset buttonpresses counter at 10 to 0
  {
    buttonPresses = 0; 
  }    
     }
  }
  buttonState = val;
    // save the new state in the variable
 unsigned long currentMillis1 = millis();
   if(currentMillis1 - previousMillis1 > increment[buttonPresses1]) //save the last LED off time
   {
    previousMillis1 = currentMillis1;
   }
   
  val1 = digitalRead(buttonPin1);
  if (val1 != buttonState1) {     // the button state has changed!
    if (val1 == LOW) {                // check if the button is pressed
      buttonPresses1++;               // increment the buttonPresses variable
      Serial.print("Interval ");
      Serial.println((buttonPresses1 + 1)*1000); // print LED off time in milliseconds
  
  if (buttonPresses1 >= 10) // reset buttonPresses1 at 10 to 0
  {
    buttonPresses1 = 0;
  }    
     }
  }
  buttonState1 = val1; // save the new state of buttonPin1
  }

If you want the blinking to be asymmetrical, more on than off or vice versa, then set the required interval each time you change the LED state.

int increment[] = {100, 200, 300, 400, 500, 600, 700, 800, 900, 1000}; // LED on times
int increment1[] = {1000, 2000, 3000, 4000, 5000, 6000, 7000, 8000, 9000, 10000}; // LED off times

The fact that you need to use comments to explain what the variables are for suggests that the names are not descriptive enough. Something like ledOnTimes and ledOffTimes might be clearer.

There is an obvious relationship between the two arrays. If you really just want the off time to be 10x the on time then it would make more sense to just calculate the off time from the on time and get rid of the second array.

If you want the blinking to be asymmetrical, more on than off or vice versa, then set the required interval each time you change the LED state.

Yes I guess that seems obvious but thanks to you pointing it out, I've changed the order of the operations and got it to work. Thanks.

The fact that you need to use comments to explain what the variables are for suggests that the names are not descriptive enough. Something like ledOnTimes and ledOffTimes might be clearer.

There is an obvious relationship between the two arrays. If you really just want the off time to be 10x the on time then it would make more sense to just calculate the off time from the on time and get rid of the second array.

The quotes were more for other readers than for me, but you're right. The names could be more obvious.
The values of the two arrays are not linked one to one. I need to pick them independently. That is working now. Thanks for you suggestions.

Three things still to resolve.

  1. I intend to replace the serial print commands with LCD commands when I get the LCD wired up. But right now, the sketch prints the array[0] values wrong on both arrays. Is this something to do with my initial setting of 4? Or is the reset to 0 part? I want the program to boot up on that setting as a default.

  2. debounce the buttons. Hoping I can work this out myself.

  3. I don't think the timing is accurate. The cycle needs to repeat in exact multiples of 1 second.

Here's the new code:

//Sketch to create an asymetric blink pattern on an LED and relay.
//ON and OFF values selected from two arrays via two buttons.

//Still printing wrong for 0 value
// printing is off in several ways
// timing may be inaccurate

const int ledPin = 13; // relay connected here
long previousMillis = 0;     // will store last time LED was updated
long previousMillis1 = 0;
const int buttonPin = 7;
const int buttonPin1 = 6;
int increment[] = {100, 200, 300, 400, 500, 600, 700, 800, 1000}; // LED on times
int increment1[] = {4000, 5000, 6000, 7000, 8000, 9000, 10000, 11000, 12000}; // LED off times
int val; // saves the button state
int val1;
int buttonState;
int buttonState1;
int buttonPresses = 4;
int buttonPresses1 = 4;
void setup() {
  pinMode(ledPin,OUTPUT);  
  pinMode(buttonPin,INPUT);
  pinMode(buttonPin1,INPUT);
  Serial.begin(9600);
  buttonState = digitalRead(buttonPin);
  buttonState1 = digitalRead(buttonPin1);
  digitalWrite(ledPin,HIGH); // start off with the LED on
  
}

void loop() {
  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();
   if(currentMillis - previousMillis > increment[buttonPresses]) 
   {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;
    
   digitalWrite(ledPin, LOW);
     unsigned long currentMillis1 = millis();
   if(currentMillis1 - previousMillis1 > (increment1[buttonPresses1] - increment[buttonPresses])) //save the last LED off time
   {
    previousMillis1 = currentMillis1;
    digitalWrite(ledPin, HIGH);
 }
   }
   
  val = digitalRead(buttonPin);
  if (val != buttonState) {     // the button state has changed!
    if (val == LOW) {                // check if the button is pressed
      buttonPresses++;               // increment the buttonPresses variable
      Serial.print("Increment ");
      Serial.println(increment[buttonPresses]); // print LED on time in milliseconds.
  
  if (buttonPresses > 8) // reset buttonpresses counter at 10 to 0
  {
    buttonPresses = 0; 
  }    
     }
  }
  buttonState = val;
    // save the new state in the variable
   
  val1 = digitalRead(buttonPin1);
  if (val1 != buttonState1) {     // the button state has changed!
    if (val1 == LOW) {                // check if the button is pressed
      buttonPresses1++;               // increment the buttonPresses variable
      Serial.print("Interval ");
      Serial.println(increment1[buttonPresses1]); // print LED off time in milliseconds
  
  if (buttonPresses1 > 8) // reset buttonPresses1 at 10 to 0
  {
    buttonPresses1 = 0;
  }    
     }
  }
  buttonState1 = val1; // save the new state of buttonPin1
  }
      buttonPresses++;               // increment the buttonPresses variable
      Serial.print("Increment ");
      Serial.println(increment[buttonPresses]); // print LED on time in milliseconds.

      if (buttonPresses > 8) // reset buttonpresses counter at 10 to 0
      {
        buttonPresses = 0; 
      }

Ignoring the fact that the comment about resetting does not match the code, aren't you resetting it too late in the code ? If it is already 9 when you get the data from the array you will be reading the tenth element of a nine element array.

Thanks very much. I'll work on that when I 'm back home in a couple of days. Thanks for pointing out the outdated comment too. As I say, I'm very new to programming and I appreciate the help.

Well I've come a long way with this project. I have the timers working independently in sequence, responsive to button input and reading out to an LCD. The millis() timers were subject to processing lag and losing about 1.3 milliseconds per cycle. I read everything I could find on this. The timer.h library and interrupts were possible solutions but I didn't really understand how to implement them.
So I came up with setting a constant "peg" pegged to the millis clock and then used that as a reference for the lag.
So TIME = millis() - ((millis() - peg) % 100). I used 100 because all of my time variables are divisible evenly by 100. This subtracted the error on every cycle.
It works great but what I'm getting now in my project is : with two millis timers back to back, both corrected like this, I get a hickup every 3 minutes or so. The led flash is very short and then everything continues normally.
MY QUESTION: Is this because the timer0 in the Arduino is correcting millis every so often and generating a number that doesn't work with my formula? Or is it some interaction between the two timers?

Here's my code:

#include <LiquidCrystal.h>

//Sketch to create an adjustable asymetric blink pattern on an LED and relay.
//ON and OFF values selected from two arrays via two buttons.

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
const int ledPin = 13; // also relay connected here
unsigned long previousMillis;     // will store last time LED was updated
unsigned long previousMillis1;
unsigned long peg;
const int buttonPin = 7;
const int buttonPin1 = 6;
int increment[] = {100, 200, 300, 400, 500, 600, 700, 800, 1000}; // LED on times
int increment1[] = {4000, 5000, 6000, 7000, 8000, 9000, 10000, 11000, 12000}; // LED off times
int val; // saves the button state
int val1;
int buttonState;
int buttonState1;
int buttonPresses = 4;
int buttonPresses1 = 4;

void setup() {
  pinMode(ledPin,OUTPUT);  
  pinMode(buttonPin,INPUT_PULLUP);
  pinMode(buttonPin1,INPUT_PULLUP);
  Serial.begin(9600);
  lcd.begin(16,2);
  lcd.print("increment: ");
 lcd.setCursor(0,1);
 lcd.print("Interval: "); 
  buttonState = digitalRead(buttonPin);
  buttonState1 = digitalRead(buttonPin1);
  peg = millis();
  digitalWrite(ledPin,HIGH); // start off with the LED on
  }

void loop() {
  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  int val = digitalRead(buttonPin);
  if (buttonPresses >= 9) // reset buttonpresses counter at 9 to 0
  {
    buttonPresses = 0; }
    lcd.setCursor(11,0);
    lcd.print(increment[buttonPresses]);
    lcd.print(" ");
  if (val != buttonState) {     // the button state has changed!
    if (val == LOW) {           // check if the button is pressed
      buttonPresses++;          // increment the buttonPresses variable
     }
  }
  buttonState = val;    // save the new state in the variable
     
  int val1 = digitalRead(buttonPin1);
  if (buttonPresses1 >= 9) // reset buttonPresses1 at 10 to 0
  {
    buttonPresses1 = 0;  }
  lcd.setCursor(10,1);
  lcd.print(increment1[buttonPresses1]);
  lcd.print(" ");
  if (val1 != buttonState1) {     // the button state has changed!
    if (val1 == LOW) {            // check if the button is pressed
      buttonPresses1++;           // increment the buttonPresses variable
     }
  }
  buttonState1 = val1; // save the new state of buttonPin1
  
  unsigned long currentMillis = millis() - ((millis() - peg) % 100); 
   if((currentMillis - previousMillis) >= increment[buttonPresses]) 
   { 
    // save the last LED on time
     previousMillis = currentMillis;
    
   digitalWrite(ledPin, LOW);
 
     unsigned long currentMillis1 = millis() - ((millis() - peg) % 100);
   if((currentMillis1 - previousMillis1) >= (increment1[buttonPresses1]))
   {
    //save the last LED off time - processing delay
     previousMillis1 = currentMillis1;
    digitalWrite(ledPin, HIGH);
 Serial.println(currentMillis1);
   }
  }
 }