Code only functioning during third and fourth iterations of loop

Hi,

A portion of this code doesn’t work during the first and second times through loop, does work during the third and fourth iterations, then never works again.

The portion I’m referring to are the digitalWrites that fall between the Serial.println(“Button x”) and Serial.println(“Button x x”). They should be turning a light white, and they do during the third and fourth iterations. The Serial.println commands were put in as a sanity check to ensure that all of the “if” conditions are being met. The print commands continue to work every iteration.

  if (button_1_active == 0 && button_1_white == 0 && loop_reset_time - attract_reset_time > button_1_attract_interval && loop_reset_time - attract_reset_time < button_2_attract_interval){

     Serial.println("Button 1");
     
     digitalWrite(22, LOW);
     digitalWrite(24, LOW);
     digitalWrite(26, LOW);

     Serial.println("Button 1 1");
     
     button_1_white = 1;
    }

  if (button_2_active == 0 && button_2_white == 0 && loop_reset_time - attract_reset_time > button_2_attract_interval && loop_reset_time - attract_reset_time < button_3_attract_interval){
     if (button_1_active == 0){
     
     digitalWrite(22, HIGH);
     digitalWrite(24, LOW);
     digitalWrite(26, HIGH);  

     button_1_white = 0;
     button_2_white = 1;
     
     }
  
     Serial.println("Button 2");   
       
     digitalWrite(28, LOW);
     digitalWrite(30, LOW);
     digitalWrite(32, LOW);

     Serial.println("Button 2 2");
    }

  if (button_3_active == 0 && button_3_white == 0 && loop_reset_time - attract_reset_time > button_3_attract_interval && loop_reset_time - attract_reset_time < button_4_attract_interval){
     if (button_2_active == 0){

     digitalWrite(28, HIGH);
     digitalWrite(30, LOW);
     digitalWrite(32, HIGH);

     button_2_white = 0;
     button_3_white = 1;
     
     }

     Serial.println("Button 3"); 
        
     digitalWrite(34, LOW);
     digitalWrite(36, LOW);
     digitalWrite(38, LOW);

     Serial.println("Button 3 3");
    }

  if (button_4_active == 0 && button_4_white == 0 && loop_reset_time - attract_reset_time > button_4_attract_interval){
     if (button_3_active == 0){
      
     digitalWrite(34, HIGH);
     digitalWrite(36, LOW);
     digitalWrite(38, HIGH);   

     button_3_white = 0;
     button_4_white = 1;
     
     }

     Serial.println("Button 4");
     
     digitalWrite(40, LOW);
     digitalWrite(42, LOW);
     digitalWrite(44, LOW);

     Serial.println("Button 4 4");
     
    }

  if (button_4_active == 0 && button_4_white == 1 && loop_reset_time - attract_reset_time > button_4_attract_interval + attract_interval){
     
     digitalWrite(40, HIGH);
     digitalWrite(42, LOW);
     digitalWrite(44, HIGH);   

     }
  
    if (loop_reset_time - attract_reset_time > attract_loop_time){

    Serial.println("Attract Reset");
     
    attract_reset_time = millis();
    
    Serial.println(attract_reset_time);
    
    button_4_white = 0;
    
    } 
}

My boss suggested that the conditions within the “if” functions are just too complicated for the processor, but if that is the case, why do the println commands continue to work? (The complicated condition sets result from having 4 different controls and an attract function that all have to work separately all the time, making delay() and “for” loops less useful.)

Thanks!

Chris

My boss suggested that the conditions within the "if" functions are just too complicated for the processor

Sounds like your boss doesn't have a clue. You need to post complete code that can compile and exhibits the problem if you want help.

For starters those if statements of yours will be executing inside loop() continuously regardless of whether your button is pressed or not.

And that makes it quite difficult to figure out what the hell is going wrong.

What you want is for the if statements to be executed ONLY when your button is pressed.

That's what interrupts are for!

Put your button on one of the interrupt pins and do the following.

volatile bool bButtonPressedFlag = false;
uint8_t nButtonPin = 2;


void buttonPressedISR()
{
   // No moe interrupts until we handle this one in loop()
   noInterrupts();

   // Trip the flag so loop() knows that an interrupt has been triggered
   bButtonPressedFlag = true;
}


void setup()
{
   .
   .
   .
   pinMode(nButtonPin, INPUT);
   attachInterrupt(digitalPinToInterrupt(nButtonPin), buttonPressedISR, RISING);
   .
   .
   .

}

void loop()
{
   if (bButonPressedFlag)
   {
       . Rest of your if statements but simplified
       . You won't need buttonActive or what ever it was
       .
       .
        // You want this stuff to be executed if and only if the button is pressed so clear the flag
       bButtonPressedFlag = false;

       // Interrupt handled so enable interrupts again.
       interrupts();
   }
}

No that's not what interrupts are for. Interrupts are for things way faster than button presses by humans. If you want those if statements to only run when the button is pressed then put them inside an if statement that checks to see if the button is pressed. There's no need for an interrupt for that.

Delta_G:
No that's not what interrupts are for. Interrupts are for things way faster than button presses by humans. If you want those if statements to only run when the button is pressed then put them inside an if statement that checks to see if the button is pressed. There's no need for an interrupt for that.

But surely, depending on how much code is in loop() and how slow or fast it might be executed, it is possible that this code might be executing when a user happens to press the button....so that the press gets missed.

Perhaps unlikely but never the less possible.

An interrupt ensures that a button click is picked up because the currently executing code is 'interrupted'.

And if you do too much polling, with digitalRead(....) on 16MHz processor, in loop() then it is generally regarded as bad thing I would have thought.

boylesg:
But surely, depending on how much code is in loop() and how slow or fast it might be executed, it is possible that this code might be executing when a user happens to press the button....so that the press gets missed.

Perhaps unlikely but never the less possible.

If your loop code is blocking for hundreds or thousands of milliseconds at a time then you need to look at refactoring your loop function, not covering up the problem by bringing in an interrupt to read the button.

boylesg:
An interrupt ensures that a button click is picked up because the currently executing code is 'interrupted'.

And if you do too much polling, with digitalRead(....) on 16MHz processor, in loop() then it is generally regarded as bad thing I would have thought.

Why would polling a button in loop be considered a bad thing? Where are you getting that?

Think about it this way, even with your interrupt method you are only reacting to the button at the point where you read that flag. So you're not actually responding to the button any faster or slower with either method. All you've done with the interrupt is add an extra layer of complexity to reading the button.

Delta_G:
If your loop code is blocking for hundreds or thousands of milliseconds at a time then you need to look at refactoring your loop function, not covering up the problem by bringing in an interrupt to read the button.

Why would polling a button in loop be considered a bad thing? Where are you getting that?

Think about it this way, even with your interrupt method you are only reacting to the button at the point where you read that flag. So you're not actually responding to the button any faster or slower with either method. All you've done with the interrupt is add an extra layer of complexity to reading the button.

But is digitalRead(...) not considered to be slow? Particularly when it comes to continuous polling in loop()?

I can't pin point when or by whom, but I seem to recall being told this in here some time ago.

I guess you could use the digitalfastwrite library - it might improve matters.

But an interrupt seems the way to go from my perspective.

And is not testing the value of a 8 bit variable somewhat faster than a function call more generally?

digitalRead( ) is slow.compared to a direct pin read. It's not a problem for human interfacing.
Executing a large number of digitalRead()s, especially in a loop can cause a significant delay, Not the case here.
Interrupts are not a solution.

boolrules:
digitalRead( ) is slow.compared to a direct pin read. It's not a problem for human interfacing.
Executing a large number of digitalRead()s, especially in a loop can cause a significant delay, Not the case here.
Interrupts are not a solution.

If it is one or two buttons and the buttons do not implement core functionality of my sketch, then I always implement them as interrupts.

Might be different if your sketch was implementing a calculator, or something similar, where the core functionality of the sketch is obtaining user input.

Not enough interrupts pins anyway.

Hi,
Welcome to the forum.

Please read the first post in any forum entitled how to use this forum.
http://forum.arduino.cc/index.php/topic,148850.0.html then look down to item #7 about how to post your code.
It will be formatted in a scrolling window that makes it easier to read.

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

How have you got your buttons wired?

Please explain what it is supposed to do, then what it is doing?

Thanks.. Tom.. :slight_smile:

boylesg:
If it is one or two buttons and the buttons do not implement core functionality of my sketch, then I always implement them as interrupts.

OK, but there's no reason to encourage others to follow bad practices as well.

boylesg:
But is digitalRead(...) not considered to be slow? Particularly when it comes to continuous polling in loop()?

What makes you say that? Do you know anyone who can actually press and release a button in under a microsecond? Or even a millisecond? Or even 10 milliseconds? Your loop function should be able to run over and over and call about ten thousand digitalRead lines before you are anywhere close to even thinking about missing a button press because of the speed of digitalRead.

If your code is supposed to interact with a human and you're not getting through your control loop in less time than it takes to press a button then you need to refactor your code.

And anyways, none of this has any bearing on the OPs problem which is likely related to some uninitialized variable or something that is mucking things up for the first two turns of his loop.

Delta_G:
And anyways, none of this has any bearing on the OPs problem which is likely related to some uninitialized variable or something that is mucking things up for the first two turns of his loop.

I wonder if he is waiting for the forums "reliable" email notification system.. :o :o :o :slight_smile:

Hi all,

I'm very grateful to everyone who read and took a crack at answering this post. Most people who are answering in good faith are requesting the entire sketch, a schematic, and a full description of the functionality of the project. These are reasonable requests from someone trying to help troubleshoot, but as this is a work for hire, I would be committing multiple contract violations by publishing that information.

This was my first post. I'll be certain not to bother folks in the future by posting without all of the relevant information.

Thanks and apologies.

Hi,
How are you wiring your buttons?
Are you switching the digital input to +5V or gnd.
Do you have a pullup or pulldown 10K resistor to pull the input when the button is open?

Tom.... :slight_smile:

Hi,

 if (button_3_active == 0 && button_3_white == 0 && loop_reset_time - attract_reset_time > button_3_attract_interval && loop_reset_time - attract_reset_time < button_4_attract_interval)

Why the huge if statement?
I think you need to group some terms to;

 if (button_3_active == 0 && button_3_white == 0 && ((loop_reset_time - attract_reset_time) > button_3_attract_interval) && ((loop_reset_time - attract_reset_time) < button_4_attract_interval))

Likewise with the other long if statements.
Tom… :slight_smile: