Millis counter question

I have a relatively simple program where I need to count input to 24, once at 24, keep output on for 1.5 to 3 seconds (I'll have to test it on a machine to know for sure). During he time the output is on, the counter resets to 0 and starts counting up to 24 again, once at 24 it repeats.

at first I used delay() which prevented counter to restart and so I lost accuracy.
then i used 2 UNO's one that would count and the second would just keep the output on and then turn off, much like Blink example.

millis() is the way to go. I spent quite some time and came up with this:

const int output = 13; //output pin on the board. 13 has an LED attached for ease of troubleshooting
int outputstate = LOW; //variable that will determine output state of the pin. It is off by default
const int input = 11; //input pin where an input from machine is added
//const int reset = 8; // input to use for counter reset uncoment to use
int inputstate = 0; //check if input is on or not
int inputcounter = 0; //counter for the input
int lastinputstate = 0; // referece for the last state of input
unsigned long previousmillis = 0; //variable for the timer
long onTime = 250; //timer variable1
long offTime = 1500;// timer variable2

//basic function that sets up the hardware
void setup()
{
  //sett up the pins and turn serial monitor on for troubleshooting
  pinMode(output,OUTPUT);
  pinMode(input, INPUT);
  Serial.begin(9600);
}
//main function of the program that loops constantly
void loop()
{
//read the input and store the reading in inputstate variable
inputstate = digitalRead(input);

  // compare the inputstate to its previous state
  if (inputstate != lastinputstate) {
    // if the state has changed, increment the counter
    if (inputstate == HIGH) {
      // if the current state is HIGH then the button
      // wetd from off to on and add one to the counter
      inputcounter++;
      //wite on the serial that input turned on
      Serial.println("product under photoeye");
      //write the first part for the totals on the serial
      Serial.print("products counted:  ");
      //add the count number at the end of the previous line
      Serial.println(inputcounter);
    }
       else {
      // if the current state is LOW then the button
      // went from on to off and write it on the serial
      Serial.println("no product");
      //adding a slight delay to prevent input bounce
      delay(30);
    }
// set last state of input to become current input state
    lastinputstate = inputstate;
  //check if the counter reached 24
  if (inputcounter == 24){
    
  unsigned long currentmillis = millis();


  if ((outputstate == LOW) && (currentmillis - previousmillis >= onTime))
    
  {
    /* if the above check is true then the program does the following:*/
    outputstate = HIGH; //set output state to be on
    previousmillis = currentmillis; //set the value for previousmillis to be the current time captured
    digitalWrite(output, outputstate); //set the output to be what the value in output state is
    
  }
          
  inputcounter = 0;//rest the input counter back to 0



}
else if (inputcounter !=24) { //check the input counter if it is not (!=) equal to 24 then
  unsigned long currentmillis = millis(); //make currentmillis to be current time with millis()
  //this is so that once we do get to 24, we have current time figure to work with

  
  if ((outputstate == HIGH) && (currentmillis - previousmillis >=offTime))
        {
          outputstate = LOW;
          previousmillis = currentmillis;
          digitalWrite(output, outputstate);
          
        }
  
  

}
  //else if (reset == HIGH) {inputcounter =0;} uncoment to have a reset for the counter on input 8
}
}

it works great, it counts to 24 and keeps the output on for the time. However, if I stop during the time the output is on, the state does not change until I trigger the input again.

I plan to attach this to a stacking machine that separates stacks into 24, the output moves a conveyor belt at a double speed for the duration it is on. since the machine occasionally stops, I cannot guarantee that it will not stop while the output is high.

I have spent many hours and tried while(); case change and moving currentmillis declaration around and I'm not getting anywhere.

What am I doing wrong?

Best,
Edge

Secret pro tip to figuring out issues with nested if statements.

I copied your code into the Arduino IDE, pressed Control-T (Autoformat) and now it looks like this:

const int output = 13; //output pin on the board. 13 has an LED attached for ease of troubleshooting
int outputstate = LOW; //variable that will determine output state of the pin. It is off by default
const int input = 11; //input pin where an input from machine is added
//const int reset = 8; // input to use for counter reset uncoment to use
int inputstate = 0; //check if input is on or not
int inputcounter = 0; //counter for the input
int lastinputstate = 0; // referece for the last state of input
unsigned long previousmillis = 0; //variable for the timer
long onTime = 250; //timer variable1
long offTime = 1500;// timer variable2

//basic function that sets up the hardware
void setup()
{
  //sett up the pins and turn serial monitor on for troubleshooting
  pinMode(output, OUTPUT);
  pinMode(input, INPUT);
  Serial.begin(9600);
}
//main function of the program that loops constantly
void loop()
{
  //read the input and store the reading in inputstate variable
  inputstate = digitalRead(input);

  // compare the inputstate to its previous state
  if (inputstate != lastinputstate) {
    // if the state has changed, increment the counter
    if (inputstate == HIGH) {
      // if the current state is HIGH then the button
      // wetd from off to on and add one to the counter
      inputcounter++;
      //wite on the serial that input turned on
      Serial.println("product under photoeye");
      //write the first part for the totals on the serial
      Serial.print("products counted:  ");
      //add the count number at the end of the previous line
      Serial.println(inputcounter);
    }
    else {
      // if the current state is LOW then the button
      // went from on to off and write it on the serial
      Serial.println("no product");
      //adding a slight delay to prevent input bounce
      delay(30);
    }
    // set last state of input to become current input state
    lastinputstate = inputstate;
    //check if the counter reached 24
    if (inputcounter == 24) {

      unsigned long currentmillis = millis();


      if ((outputstate == LOW) && (currentmillis - previousmillis >= onTime))

      {
        /* if the above check is true then the program does the following:*/
        outputstate = HIGH; //set output state to be on
        previousmillis = currentmillis; //set the value for previousmillis to be the current time captured
        digitalWrite(output, outputstate); //set the output to be what the value in output state is

      }

      inputcounter = 0;//rest the input counter back to 0



    }
    else if (inputcounter != 24) { //check the input counter if it is not (!=) equal to 24 then
      unsigned long currentmillis = millis(); //make currentmillis to be current time with millis()
      //this is so that once we do get to 24, we have current time figure to work with


      if ((outputstate == HIGH) && (currentmillis - previousmillis >= offTime))
      {
        outputstate = LOW;
        previousmillis = currentmillis;
        digitalWrite(output, outputstate);

      }



    }
    //else if (reset == HIGH) {inputcounter =0;} uncoment to have a reset for the counter on input 8
  }
}

See the problem now? The part that is checking millis is inside the part that checks if the input has changed. So it can only notice that the interval has passed and turn off the pin if the input changes to get it into that part. So if the machine stops toggling the input, it never notices that the time has passed.

This really sounds like it should be coded like a state machine.

const byte output  = 13;
byte outputstate   = LOW;

const byte input   = 11;   

int inputstate     = 0;    
int inputcounter   = 0;    

int lastinputstate = 0;    

unsigned long previousmillis = 0; 
unsigned long onTime = 250;       
unsigned long offTime = 1500;     


//**************************************************************************
void setup()
{
  Serial.begin(9600);

  pinMode(output, OUTPUT);

  pinMode(input, INPUT_PULLUP);
  lastinputstate = digitalRead(input);

} //END of setup()


//**************************************************************************
void loop()
{
  static unsigned long lastMillis = 0;

  //*************************************
  //time to check the input?
  if (millis() - lastMillis >= 50)
  {
    checkInput();
  }
  //*************************************


} //END of loop()


//**************************************************************************
void checkInput()
{
  inputstate = digitalRead(input);

  //*************************************
  if (lastinputstate != inputstate)
  {
    lastinputstate = inputstate;

    //***************
    if (inputstate == HIGH)
    {
      inputcounter++;
      Serial.println("Product under photoeye");
      Serial.print("Products counted: ");
      Serial.println(inputcounter);
    }

  }

  if (inputcounter == 24)
  {
    inputcounter = 0;
    outputstate = HIGH;
    previousmillis = millis();
    digitalWrite(output, outputstate);
    
  }

  else
  {
    if (outputstate == HIGH && millis() - previousmillis >= offTime)
    {
      outputstate = LOW;
      previousmillis = millis();
      digitalWrite(output, outputstate);
    }
    
  }

} //END of checkInput()

wow
Delta_G thanks for the tip. I’ll check it out.

larryd thanks for the updated code version, I just loaded it and it works like magic. I will go through and check my code with Delta_G tip. I want to find where I went off and then compare it with your code to see where i need to improve.

Thanks a lot!
Edge

Added a missing line of code.

const byte output     = 13;
byte outputstate      = LOW;
const byte input      = 11;
int inputstate        = 0;
int inputcounter      = 0;
int lastinputstate    = 0;
unsigned long onTime  = 250;
unsigned long offTime = 1500;
unsigned long lastMillis;
unsigned long previousmillis;


//**************************************************************************
void setup()
{
  Serial.begin(9600);

  pinMode(output, OUTPUT);

  pinMode(input, INPUT_PULLUP);
  lastinputstate = digitalRead(input);

} //END of setup()


//**************************************************************************
void loop()
{
  //*************************************
  //time to check the input?
  if (millis() - lastMillis >= 50)
  {
    //restart timer
    lastMillis = millis();  // <------<<<<< Correction, added this line
    //check the input
    checkInput();
  }
  //*************************************


} //END of loop()


//**************************************************************************
void checkInput()
{
  inputstate = digitalRead(input);

  //*************************************
  if (lastinputstate != inputstate)
  {
    lastinputstate = inputstate;

    //***************
    if (inputstate == HIGH)
    {
      inputcounter++;
      Serial.println("Product under photoeye");
      Serial.print("Products counted: ");
      Serial.println(inputcounter);
    }

  }

  if (inputcounter == 24)
  {
    inputcounter = 0;

    outputstate = HIGH;
    //time the LED turned on
    previousmillis = millis();
    //turn on LED
    digitalWrite(output, outputstate);

  }

  else
  {
    //if the LED is ON, has the timer interval expired?
    if (outputstate == HIGH && millis() - previousmillis >= offTime)
    {
      outputstate = LOW;
      digitalWrite(output, outputstate);
    }

  }

} //END of checkInput()

All right, i have it installed into the machine and it works like a charm!