Limiting button presses in a coin acceptor Interrupt/if statement, advice needed

After research, and studying the C++ syntax I took a shot at writing a little bit of code. Starting with the lCD , then the coin acceptor and activating a few relays when a specific currency total is met. I want to limit the presses to a button when the conditions are met. The issue I'm having is the button code within an Interrupt/ if statement wont activate properly.

Example:

if (money == .50){
button active only for a single shot. (this part is where I get a little confused)
1 shot = activate pin 13 for a specific amount of time set using the delay(xxx); function.
}

I have the code activating the relays to show physical proof that I can update the LCD activate something in the real world.

The Question:

How can I limit button presses when conditions are met, i.e. when money==.50 only allow 1 button press, money == 1.00 only allow 3 button presses.

The button is attached to digital pin 12 Output is digital pin 13

Edit: Full code now attached.

    //Load Libraries
  #include <LCD.h>
  #include <LiquidCrystal_I2C.h>
  #include <Wire.h>


// this constant won't change:
const int  Button = 12;    // the pin that the pushbutton is attached to
const int  LED = 13;       // the pin that the LED is attached to
const int  Relay = 11;

   // Variables will change:
   int RelayState = LOW;
   int buttonState = 0;
   int lastButtonState = HIGH;
   int ButtonCount = 0;
   
   //LCD QTY update
   int Shots = 0;
   

   
  //Define PINS
  #define COIN_PIN 2  
  #define I2C_ADDR 0x27 // <<- Add your address here.
  #define Rs_pin   0
  #define Rw_pin   1
  #define En_pin   2
  #define BACKLIGHT_PIN 3
  #define D4_pin   4
  #define D5_pin   5
  #define D6_pin   6
  #define D7_pin   7



  LiquidCrystal_I2C lcd(I2C_ADDR,En_pin,Rw_pin,Rs_pin,D4_pin,D5_pin,D6_pin,D7_pin);

 

  void setup()
  {

  // initialize the inputs and outputs
  pinMode(Button, INPUT_PULLUP);
  pinMode(LED, OUTPUT);
  pinMode(Relay, OUTPUT);
  
  digitalWrite(LED, HIGH);

  
 
  
  // Debugging
  Serial.begin(9600); // set up the LCD's number of rows and columns:

  lcd.begin(20, 4);
  //Turn on lcd backlight
  lcd  .setBacklightPin(BACKLIGHT_PIN,POSITIVE); // turn on lcd LED
  lcd.setBacklight(HIGH);
  
  //System Loading 
 
  lcd.begin(20, 4);
  delay(100);
  lcd.setCursor(3,1);
  lcd.print("FATAL MENTALITY");
  lcd.setCursor(5,2);
  lcd.print("DESIGNS.COM");
  delay(1000);
  lcd.clear();
  lcd.print("SCREAM MACHINE 1.1");
  delay(1000);
  lcd.clear();
  lcd.setCursor(2,1);
  lcd.print("SYSTEM LOADING..");
  {
  delay(500);
  }
  lcd.home();
  lcd.setCursor(2,2);
  delay(500);
  lcd.clear();
  lcd.setCursor(5,1);
  lcd.print("SCREAM ON!");
  delay(1000);
  lcd.clear();
  
  //End of LCD system loading code

  Serial.println("Ready...");
  pinMode(COIN_PIN, INPUT);
  
  attachInterrupt(0, coinISR, RISING);  // COIN wire connected to D2;
  } 


  // total amount of money collected;
  float Money = 0.0;
  
  
  // gets incremented by the ISR;
  // gets reset when coin was recognized (after train of pulses ends);
  volatile int pulses = 0;
  volatile long timeLastPulse = 0;


  // executed for every pulse;
  void coinISR()
 {
  pulses++;
  timeLastPulse = millis();
 }


 void loop() {

  
  //LCD Information
  lcd.setCursor(0,0); 
  lcd.print("_.:SCREAM MACHINE:._"); 
  lcd.setCursor (1,1); 
  lcd.print("$1.00 = 3 $0.50 = 1");
  lcd.setCursor(0,1);
  lcd.setCursor (0,2);
  lcd.print("INSERT COINS: $");
  lcd.print(Money); 
  lcd.setCursor(0,3);
  lcd.print("TOTAL  SHOTS:");
  lcd.setCursor(13,3);
  lcd.print(Shots);


  
 //coin acceptor code  
buttonState = digitalRead(Button);
 
  long timeFromLastPulse = millis() - timeLastPulse;
  if (pulses > 0 && timeFromLastPulse > 200){
    // sequence of pulses stopped; determine the coin type;
    if (pulses == 10)
    {
      }
      Serial.println("Received Quarter (10 pulses)");
      Money += .25; 
      if (Money == .50){Shots +=1;
    
       digitalWrite(LED,LOW);
       delay(1000);
       digitalWrite(LED,HIGH);
       delay(500);
       digitalWrite(LED,LOW);
       delay(500);
       digitalWrite(LED,HIGH);
       delay(500);
       digitalWrite(LED,LOW);
       delay(1000);
       digitalWrite(LED,HIGH);}
      
  
    
    if (Money == 1.00) {Shots += 2;

    digitalWrite(LED,LOW);
    delay(1000);
    digitalWrite(LED,HIGH);
    
    }
    
    if (Money > 1.00) {Money = .25;}

    if (Shots > 3) {Shots = 1;}
     {
    pulses = 0;
     }
   } 
 }

Where I assume I put the limit of times the program reacts to a switch. (@PaulS)

//coin acceptor code  
 
  long timeFromLastPulse = millis() - timeLastPulse;
  if (pulses > 0 && timeFromLastPulse > 200){
    // sequence of pulses stopped; determine the coin type;
    if (pulses == 10)
    {
      }
      Serial.println("Received Quarter (10 pulses)");
      Money += .25; 
      if (Money == .50){Shots +=1;
    
       digitalWrite(LED,LOW);
       delay(1000);
       digitalWrite(LED,HIGH);
       delay(500);
       digitalWrite(LED,LOW);
       delay(500);
       digitalWrite(LED,HIGH);
       delay(500);
       digitalWrite(LED,LOW);
       delay(1000);
       digitalWrite(LED,HIGH);}
      
  
    
    if (Money == 1.00) {Shots += 2;

    digitalWrite(LED,LOW);
    delay(1000);
    digitalWrite(LED,HIGH);
    
    }
    
    if (Money > 1.00) {Money = .25;}

    if (Shots > 3) {Shots = 1;}
     {
    pulses = 0;
     }
   } 
 }

Thanks In advance for any help,

-Eric G

Please post your entire code. It's hard to chase problems looking at snippets. The problem often isn't where you think it is.

Here's one potential issue:

if (money == .50){

You should never compare floating point values for exact equality. Floating point values are by their very nature an imprecise way to hold a number. There are a number of values that they simply cannot represent exactly. Sort of like how in base 10 we can't write one-third exactly, we always have to round it at some point. The result of all of this is that sometimes 0.50 is really 0.49999999 and sometimes it's really 50.000001.

If I were you, I wouldn't use a floating point variable to hold an amount of money. Instead of thinking in terms of dollars, think in terms of pennies. You won't be taking any fractional pennies so by counting money in pennies instead of dollars you can use integer type variables. This will make all of your calculations and if testing work a whole lot smoother and will save you from a few nasty hidden bugs and pitfalls involved with using floats.

1 shot = activate pin 13 for a specific amount of time set using the delay(xxx); function.

Not in an interrupt service routine.

Reading the coin acceptor pulses in an ISR makes sense. Reading the "dispense a product" switch in an ISR does not. Actually dispensing the product in an ISR makes no sense.

How can I limit button presses when conditions are met, i.e. when money==.50 only allow 1 button press, money == 1.00 only allow 3 button presses.

You don't limit the number of times the switch can be pressed. You limit the number of times you react to a switch press.

I have a strong suspicion that you should remove ALL the delay()s from your code. They must make it almost impossible to detect a button press. There should be no need to use an interrupt - humans are very slow compared to an Arduino.

From your description I think you need to introduce some variables to keep track of the state of the system - for example whether a button press is due or not.

And, as @Delta_G says, post your complete program.

You may find some useful stuff in Planning and Implementing a Program. Among other things it uses millis() to manage timing rather than delay().

...R

Delta_G:
Please post your entire code. It's hard to chase problems looking at snippets. The problem often isn't where you think it is.

Here's one potential issue:

if (money == .50){

You should never compare floating point values for exact equality.

Untrue - you can always compare to zero, and to integral values within the
representation range of the floating point format (and here to 0.5 which is
represented exactly in IEEE floating point format)

You can compare to a constant, since the same constant will always have
the same representation.

What you shouldn't do is compare two floating point variables for equality
when they are the result of calculations, as rounding errors mean there is
no guarantee of the same presentation even if they are mathematically the
same when the program is treated as algebra.

For instance you can't compare 10.0/3.0 to 3.333333 or sin (PI/2) to 1.0

Robin2:
I have a strong suspicion that you should remove ALL the delay()s from your code. They must make it almost impossible to detect a button press. There should be no need to use an interrupt - humans are very slow compared to an Arduino.

From your description I think you need to introduce some variables to keep track of the state of the system - for example whether a button press is due or not.

And, as @Delta_G says, post your complete program.

You may find some useful stuff in Planning and Implementing a Program. Among other things it uses millis() to manage timing rather than delay().

...R

Delta_G:
Please post your entire code. It's hard to chase problems looking at snippets. The problem often isn't where you think it is.

Here's one potential issue:

if (money == .50){

You should never compare floating point values for exact equality. Floating point values are by their very nature an imprecise way to hold a number. There are a number of values that they simply cannot represent exactly. Sort of like how in base 10 we can't write one-third exactly, we always have to round it at some point. The result of all of this is that sometimes 0.50 is really 0.49999999 and sometimes it's really 50.000001.

If I were you, I wouldn't use a floating point variable to hold an amount of money. Instead of thinking in terms of dollars, think in terms of pennies. You won't be taking any fractional pennies so by counting money in pennies instead of dollars you can use integer type variables. This will make all of your calculations and if testing work a whole lot smoother and will save you from a few nasty hidden bugs and pitfalls involved with using floats.

That's also what I had read via other tutorials and write-ups, it seems to work fine updating the LCD for now. I posted the code in the original thread for you guys to review I plan on cleaning the code up after I solve this button puzzle. This is my first ambitious project with the Arduino platform please bare with me!

Again thank you for the response,

-Eric G

MarkT:
Untrue - you can always compare to zero, and to integral values within the
representation range of the floating point format (and here to 0.5 which is
represented exactly in IEEE floating point format)

You can compare to a constant, since the same constant will always have
the same representation.

What you shouldn't do is compare two floating point variables for equality
when they are the result of calculations, as rounding errors mean there is
no guarantee of the same presentation even if they are mathematically the
same when the program is treated as algebra.

For instance you can't compare 10.0/3.0 to 3.333333 or sin (PI/2) to 1.0

That's all well and good if you know how the float variable Money was calculated. 0.50 may be exactly representable, but you don't know what other intermediate values Money went through. What if the fifty cents was from 5 dimes? 0.10 isn't exactly representable so there will be rounding error. Do addition of that rounded value 5 times in a row (user inserted 5 dimes) and you multiply that error by 5. Suddenly five dimes doesn't add up to fifty cents anymore.

But the whole thing can be avoided, not to mention the code will run faster and take less space, if you ditch the float entirely and work in pennies. Unless you think someone is going to put more than 655 dollars in your machine at once, a regular 16 bit integer will work well for holding the Money amount.

eric_gee:
please bare with me!

I will skip that bit, if you don't mind :slight_smile:

I plan on cleaning the code up after I solve this button puzzle

Some of the things that have been suggested go beyond "cleaning the code". They are (or seem to be) fundamental to solving the problem.

My suggestion would be to modify your code to take in all the suggestions that have been made so far (assuming they don't conflict with each other) and see where that gets you.

...R

The coin mech gives you pulses when money is put, right?
Ones I've seen give 1 pulse for every 5 cents (no pennies!).

You would do best keeping the money value -as- pulse count. $0.50 == 10 pulses.

You want to have separate chunks of code in loop() do separate but related tasks.

  1. Input: watches the coin mech for a pulse and increments a pulse counter when each comes.
    -- with each pulse, save millis() as the time of the last pulse read.
    // One coin may generate a quick string of pulses, you want to count them all.
    // The pulses will not come back to back without loop() cycling many times in between.
    // Yes, really! Without delay() or other holdup this loop() will run > 10,000x per second.
    // Honest-up, you will need to code on pin change to watch pulses because....
    // just watching HIGH or LOW you will get multiple reads per pulse. Arduino is fast.

  2. Input: watches the button (and may need to debounce the button) and how long it has
    been since the last pulse to be sure all of the pulse train that the last coin coin generates
    are caught.
    if ( millis() - time last pulse read >= a bit longer than multiple pulses arrive from each other)
    then set a flag meaning button pressed T/F.

  3. Process: this only runs when the button pressed flag is set.
    -- If there are >= 20 pulses, add 3 to a variable holding paidItems and subtract 20 from pulses.
    -- Else if there are >= 10 pulses, add 1 to a variable holding paidItems and subtract 10.
    then
    -- if paidItems > 0, add one to a variable giveItems and subtract 1 from paidItems.

  4. Output: this only runs when the giveItems is > 0.
    -- check if the last item given is finished being given, if not then back to loop()
    -- give the item
    -- subtract 1 from giveItems.

This above will always count the money, see the button and try to give the items no matter what order the coins are entered or button pushed.
Worst case, someone pushes and holds the button then adds 4 quarters, as soon as it sees 10 pulses it will give one item and 10 pulses later another instead of 3 items total. Say huh? Why?
Because the time between inserting one quarter after another will probably be more than the time between the 5 pulses that a quarter generates.
You could make a long delay of perhaps 1/2 second or more that mormally comes between one coin and the next sorted and inserted in the mech but you'd better have a very strong button as some users will bang the button hard if they notice any delay (perhaps 1/5 seconds) at all.

I would suggest a sign that says Add coins THEN press button.

Every step above runs every time loop() runs though all any might do is check conditions and nothing else because there is no pulse or button press or other "run trigger condition".
They all run as if they are separate sketches inside the same loop().
Each can only tell another when to run, maybe how many times to run, but never ever does one section make another stop or miss a beat.

Think of this as a virtual machine with 4 freely moving parts. What can't you build?

Robin2:
I will skip that bit, if you don't mind :slight_smile:
Some of the things that have been suggested go beyond "cleaning the code". They are (or seem to be) fundamental to solving the problem.

My suggestion would be to modify your code to take in all the suggestions that have been made so far (assuming they don't conflict with each other) and see where that gets you.

...R

Haha oops I ment bear with me. :b

Thanks for the help, I will look into it more. All these suggestions have helped a ton!