Money timer Nightmare =/

Hi all arduino lovers,good day to you!

The problem I have is pretty simple but the solution seem to be out of my reach still.

I am trying to fix a coin timer which had it's board burnt by replacing it with an arduino Duemilanove.

Wiring:
1 input switch that kinda bounce a bit and that is pushed for like 50ms when a quarter comes in-->Pin2
1 Output Relay (currently just a led) on pin 13

What i'm trying to do:
-When you insert a quarter,you increment the timer by 3 minutes
-As soon as there is time available,the LED output/relay must be activated for that duration
-(here's the tricky part) say you have 2 minutes left,you insert another quarter and the timer must again increment by 3 so we now have 5 minutes left.

It all seem pretty simple I know,1 switch and 1 led but I can't get it to work correctly let's say for a reliable time of 15min.1 quarter works great when the switch bouncing problem ain't in the way but arduino timing ain't good and interupt are kinda weird acting too.

Here's the code I've done so far;

const int buttonPin = 2;     // the number of the pushbutton pin
const int relayPin =  13;      // the number of the LED pin

// variables will change:
int buttonstate = 0;         // variable for reading the pushbutton status
long timefactor = 1800;
int relaystate = 0 ;
long timeleft= 0;



void setup() {
  timeleft=0;
  pinMode(relayPin, OUTPUT);      
  pinMode(buttonPin, INPUT);  
Serial.begin(9600);  
}

void loop(){
Serial.println(timeleft);
  // Output Relay
  if (relaystate==1){
    digitalWrite(relayPin, HIGH);
  }
  if (relaystate==0){
    digitalWrite(relayPin, LOW);
  }
  //Trigger pressé?
  if (digitalRead(buttonPin)==HIGH){
    do
    {
    buttonstate=(digitalRead(buttonPin));
    }
    while (buttonstate==HIGH);
    delay(200);
    timeleft=timeleft+timefactor;

  }
  delay(100);
  timeleft=timeleft-1;
  if (timeleft>1){
    relaystate=1;
  }
  if (timeleft<1){
    relaystate=0;
    timeleft=0;
  }
}

You can tear it apart if you want,I'd just want the simplest working solution.

I'm kinda short on time and coding ain't my best domain so if someone would give it a try I would greatly appreciate it.

thanks and have a nice day ;p

Greetz Lordzeppo! This seemed like a perfect use of a Interrupt Routine, and a Debounce Routine.

Tested and working code follows...
Consider yourself Hooked Up! Nightmare over.

(BTW, what's this for? MAME cabinet or something?)

/*
 *  CoinTimer.pde v1.1 - Interrupt Driven Debounced Coin Timer for Arduino
 *  02-13-2010
 *
 *  Copyright (c) 2010 Brett Walach <emailbrett @ gmail dot com>
 *  All rights reserved.
 *
 *
 *  LICENSE
 *  -------
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 3 of the License, or
 *  (at your option) any later version.
 *  
 *  This program is distributed in the hope that it will be useful,
 *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 *  GNU General Public License for more details.
 *  
 *  You should have received a copy of the GNU General Public License
 *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 *
 *  EXPLANATION
 *  -----------
 *  This code allows for a switch input and general purpose output (LED or other).
 *  An interrupt fires every 1ms, and a counter is incremented by one, tested, and a
 *  timer_tick flag is set if the timer has expired.  In doing it this way, it is
 *  very easy to increment to 5 or 10 for a 5ms or 10ms interrupt. The switch input 
 *  is sampled every 1ms and is debounced for a default of 25ms.
 
 *  If the switch input is LOW for more than 25ms, and released to the HIGH state,
 *  it is counted as a valid input and coinSwActive flag is set.
 *
 *  Supporting routines may monitor for coinSwActive flag and update timers and outputs
 *  accordingly.  In this case, coinSwActive is used to let us know when to update the
 *  timeRemaining counter.  The updateOutputs() routine sets our general purpose
 *  output HIGH if there is timeRemaining, and LOW if timeRemaining is zero.
 *
 */


#include <SoftwareSerial.h>
#include <avr/interrupt.h>
#include <avr/io.h>

//---------- Constants ----------
#define coinSwPin 2           // The number of the coin sw pin
#define ledPin 13             // The number of the LED pin
#define ADD_TIME_MS 180000    // (3 mins) The number of milliseconds we will increase when coinSwPin is active.
#define TIME_HIGH_MS 3600000  // (60 mins) The number of milliseconds we will set the output ON
#define TIME_LOW_MS 0         // Value to keep timeRemaining above

//---------- Status variables ----------
long timeRemaining = 0;  // Time remaining in milliseconds (don't let exceed 4,294,967,295)

//---------- Debounce variables ----------
byte newTime = 0;
boolean newSw, oldSw, coinSwActive, debounceEn = false;
#define DEBOUNCE_TIME_MS 25 // switch input must be low for this duration in milliseconds, or debounce timer resets

//---------- Interrupt Defs / Vars ----------
#define INIT_TIMER_COUNT 6  
#define RESET_TIMER2 TCNT2 = INIT_TIMER_COUNT  
#define TIMER_DELAY_MS 1  // Interrupt timer delay in milliseconds, timer_tick will be set every TIMER_DELAY_MS ms.
int int_counter = 0;  
volatile boolean timer_tick = false;


// INTERRUPT ROUTINE ----------
// Aruino runs at 16 Mhz, so we have 1000 Overflows per second...  
// 1/ ((16000000 / 64) / 256) = 1 / 1000 (once per millisecond)
ISR(TIMER2_OVF_vect) {  
  RESET_TIMER2;  
  int_counter += 1;  
  if (int_counter == TIMER_DELAY_MS) {  
    timer_tick = true;
    int_counter = 0;  
  }   
}; 

void setup()
{
  // SETUP INTERRUPT HANDLING
  TCCR2B |= (1<<CS22);                  // turn on CS22 bit  
  TCCR2B &= ~((1<<CS21) | (1<<CS20));    // turn off CS21 and CS20 bits     
  TCCR2A &= ~((1<<WGM21) | (1<<WGM20));   // turn off WGM21 and WGM20 bits   
  TCCR2B &= ~(1<<WGM22);                  // turn off WGM22  
  // Use internal clock - external clock not used in Arduino  
  ASSR |= (0<<AS2);  
  TIMSK2 |= (1<<TOIE2) | (0<<OCIE2A);        //Timer2 Overflow Interrupt Enable    
  RESET_TIMER2;                 
  sei();      //not needed, but works with it in... don't forget about cli(); to disable
  
  // Uncomment to debug
  //Serial.begin(9600); 
 
  timeRemaining = 0; // Initialize time remaining as zero
  pinMode(ledPin, OUTPUT);      
  pinMode(coinSwPin, INPUT);  
}

void loop()
{
  if (timer_tick) {  // Timer has expired, time to process some stuff once per timer_tick
                     // timer_tick fires every 1ms based on TIMER_DELAY_MS constant.  Must run
                     // this fast enough to oversample and catch the shortest switch input duration
                     
    timer_tick = false;  // Reset timer tick

    // DON'T TAKE UP MORE THAN 1MS BELOW OR YOU WILL SACRIFICE REAL-TIME
    updateTimers();  // Update the timers to know when to take action

    readInputs();    // Read the switch inputs and debounce them

    updateOutputs(); // Update our outputs based on our inputs
  }


  // MAIN LOOP
  // -- add code here that you want to run in between the timer ticks (i.e. FAST!)


} // MAIN LOOP END


//----------
// Update Timers
//
// Updates timers based on INTERRUPT routine
//----------
void updateTimers()
{
  if (coinSwActive) {  // Update counters if a debounced switch press was received
    timeRemaining += ADD_TIME_MS; // Increase timeRemaining with more time
    if(timeRemaining > TIME_HIGH_MS) timeRemaining = TIME_HIGH_MS; // Cap time remaining HIGH to prevent overflow
    coinSwActive = false;
    // Uncomment to debug
    //Serial.print("TIME REMAINING: ");
    //Serial.println((timeRemaining/1000),DEC);  //Displays timeRemaining in seconds
  }
  timeRemaining--; // Decrease by the amount of time between timer_ticks, in this case 1ms per
  if(timeRemaining < TIME_LOW_MS) timeRemaining = TIME_LOW_MS; // Cap time remaining LOW to prevent underflow
}


//----------
// Read Inputs
//
// Reads coin switch input and debounces
//----------
void readInputs()
{
  newSw = digitalRead(coinSwPin);   // read input value from the coinSwPin and store it into newSw
  coinSwActive = false;             // Assume switch is NOT pressed
  
  if (newSw == LOW && oldSw == HIGH)    // the button state has changed
  {
    newTime = 0;
    debounceEn = true;
  }
  if (debounceEn)
  {
    if (newSw == LOW)
    {
      newTime++;  // Count up each 1ms pass through the debounce loop
      if (newTime > DEBOUNCE_TIME_MS)
      {
        coinSwActive = true;   // Switch press is valid, let other routines know
        debounceEn = false;    // resets debounce for next press
      }
    }
  }
  oldSw = newSw;  
}


//----------
// Update Outputs
//
// Updates outputs based on debounced inputs
//----------
void updateOutputs()
{
  if (timeRemaining > 0) {  // If time remaining, set output high... otherwise... low
    digitalWrite(ledPin,HIGH);
  }
  else {
    digitalWrite(ledPin,LOW);
  }
}

Look for the SERIAL stuff to uncomment if you want to monitor the serial port and see the "TIME REMAINING: xxx" count displayed when you drop a coin.

A few comments about the code...

  • Overall it looks very good.
  • timer_tick needs to be declared volatile.
  • In setup, there's no point calling sei; interrupts are never disabled. (I believe some part of timer setup requires interrupts to be disabled so you may be missing a cli)
  • There is a race condition in loop between checking timer_tick and resetting it. Those two actions need to be performed together with interrupts disabled.
  • I realize it's a pittance but you're giving the human one free millisecond for each additional coin. In updateTimers, timeRemaining should always be decremented if it's greater than zero.

Thanks.

Yeah I had timer_tick as a volatile int... and in the process of changing it to boolean I lost it.

Yeah, sei(); commented out still lets the interrupts run... seems good to leave it though in case I wanna use cli() in the future.

The race condition would only happen if the processes within the if(timer_tick) routine took longer than 1ms. Fortunately running at 16MHz lets us execute 4000 instructions in that amount of time... so it's definitely winning the race. Something to look out for though, however... disabling interrupts and clearing won't create more real -time.... so you just have to be smart and count your instructions.

Yes, you are correct... sometimes there IS such a thing as a free lunch. Would be good to always decrement and bound.

UPDATED CODE ABOVE TO v1.1

The race condition would only happen if the processes within the if(timer_tick) routine took longer than 1ms. Fortunately running at 16MHz lets us execute 4000 instructions in that amount of time... so it's definitely winning the race.

Two things...

  • It is always better to get the code correct and then worry about optimization. The cost to protect the code is minimal (about four machine instructions). Are you willing to speed up the code by four cycles at the expense of having reliable correct code?
  • You assume there are no other sources of interrupts. How many machine instructions are executed when a character arrives on the serial port? How many by the timer0 code? What happens if the two interrupts occur simultaneously? In your analysis, have you determined what other interrupts could occur?

I haven't assume anything except that a race condition would only occur if I didn't get back to check timer_tick within 1ms of checking it last time.

Hey, don't sweat it... this puppy is going to count quarters and dish out time delays like Donkey Kong throws barrels, like mad.

Besides, I think you'd have to do it in assembly code if you really wanted to make sure it was done correctly and efficiently.

Greetz Brett! You are a Genius ^^

The Updated code is working perfectly,even though I don't understand it all =/ but it's working =D

The machine it's currently in is a shower timer for public shower.It activate a relay valve on the hot water pipe so people can't leave it open and empty the hot water tank which would burn it.My parents had two of those machines of a certain model and both died,they were worth 900$ and the new ones were 200$,so I tought a 30$ arduino could up to the job,and with your help,it is^^

Arduino comunity hear me,Brett is THE man ^^.

Wow, thanks for the kind words Lordzeppo! You're very welcome, and your application is more involved and interesting than I suspected. I'm glad I could help you save some money.

Since this is meant to protect a water heater as a background task to having a timer function, there is still a potential to empty the tank if it kept getting fed quarters. It might be a good idea to lower the TIME_HIGH_MS value to 1800000 which is 30 minutes, or 1200000 which is 20 minutes. You could still feed it quarters after some time has run down, but at least you'd have to be there doing it.

Also, you could add another button that resets the timer to shut off the water when you are done... in case you just don't want to stand there wasting water. Although, they paid for it, so they might anyway. If done, this extra button wouldn't really even have to be debounced since it's a reset function. Just a nice waterproof, rugged switch to ground, and a 10k ohm pull up resistor. Run that into a digital input that gets read in readInputs(); Save to boolean resetState; which would have to be initialized as false (high from pull up). Later in updateOutputs(); test resetState and zero out timerRemaining if resetState equals true. The true/false helps to make you understand that the State is happening or not, but because the input is LOW when the reset state is true, you have to read in the input and invert it... like this: resetState = !digitalRead(resetPin); The not operator '!' inverts the input. You could also invert the switch wiring, but for some reason I never do. I guess one way keeps it all straight for me.

Some other thoughts are, adding a relay test... to make sure the relay is not stuck closed.

And if there is a way to get status on water level remaining, that could be another automatic reset.

Lastly, you could figure out how fast the water drains and refills, and make an absolute timer that keeps track of "water level". The quarters and RUN TIMER would basically decrement (water draining) from the BACKGROUND TIMER, and whenever the RUN TIMER was zero (water is filling back up), the BACKGROUND TIMER would increment back up. This would all have to be characterized to make it fairly accurate. But then if the BACKGROUND TIMER was zero... you'd have some idea that someone keeps feeding it quarters for a REALLY long shower, and you could just cut the hot water off... they'd jump out right quick!

Never thought my code would control hot showers... awexome!

All your suggestions are indeed awesome brettz but we do not need to know or control the water level,what I was refering to is a kid coming in the shower and putting the hot water full throttle then leaving so the shower would run for all day long till the maintenance crew would come by and check it,the risk of the tank emptying with quarter is thereby inexistant since it would need something like 500$ and if the guy do put 500$ in it,replacing the burnt element at a price of 40$ is still profitable :wink: but yet,won't happen =P

Check your inbox vbrett,I've sent you a pm also.

Last thing,the TIME_HIGH_MS is the maximum time the timer will go up to right? so 10 quarters in,no more until some time has run down correct?

That works then! Cool. Didn't know the heater element was so cheap either.

Yeah TIME_HIGH_MS is the max time in milliseconds. Right now it's set for 60 minutes total, so you would have to let it come down to 57 before putting a quarter in would give you full credit of 3 minutes. It will take 20 quarters to max the timer though... that's a lot of quarters to be carrying around :slight_smile:

Hi lordzeppo

Are you protecting the input pin with a resistor? I can't see any pull-up eneblemant in the code.

If I may offer my 25 cent - I would have made the code like this:

// Money_timer
// Money tricker (buttonPin) is connected to pin 2. The pin is protected with the internal pull-up resistor
// There is a debounce time before the buttonPin accept another input.
// Due to problems around millis() overflow (if stopTime did a roll-over before millis() the
// relay would go off) the comparison betweeen millis() and stopTime is only made 2 sec
// before and 2 sec after they are equal thus minimising premeture turnoff to max 2 sec


byte buttonPin = 2;
byte relayPin = 13;
unsigned long relayTime = 180000; // Relay open time (in milli sec)
int debounceTime = 200; //ms

unsigned long stopTime, now;
boolean thisReading, lastReading;

void setup(){
  pinMode(relayPin, OUTPUT);
  pinMode(buttonPin, INPUT);
  digitalWrite(buttonPin, HIGH); // Enable pull-up - don't fry the pin!
}

void loop(){
  // Detecting coin input
  thisReading = digitalRead(buttonPin);
  delay(1);  
  if ((thisReading != lastReading) and thisReading == LOW){ // A chenge was made on buttonPin
    if (digitalRead(relayPin) == HIGH) stopTime = stopTime + relayTime; //We are running and the button is pressed
    if (digitalRead(relayPin) == LOW){ //Button is pressed for the first time
      stopTime = millis() + relayTime;
      digitalWrite(relayPin, HIGH); // Time to start
    }
  delay(debounceTime);
  }
  lastReading = thisReading;
  
  // Check to see if relay should be off
  now = millis();
  if (((stopTime - now < 2000) or (stopTime - now > 2000)) and (now > stopTime)){  // minimal Roll-over error
    digitalWrite(relayPin,LOW);
  }
}

3 min hot water for a quater - that sounds rather cheap. Maybe electricity is just too overcharged here in EU :wink:

-Fletcher

In general, a pull up is not necessary really to protect the input. Usually the inputs will be "fried" if they are taken over the supply rail by > (VCC + 0.7V), which pops the clamp diode in the micro. If you leave the input "floating", that could cause it to consume a bunch of extra current and if it toggles on/off too rapidly maybe it could fizzle out. Then general idea is that you bring a digital input to one of two states, and don't let it just wander about. So adding a pull-down resistor would also work, and the switch pulls the input up to 5V. I don't generally do it that way though, and the code works with an active low input.

To protect the pin from things like Electrostatic Discharge (ESD), you need a series resistor. 1k series usually does the trick, unless it's a product in LasVegas, NV... then you might need 10k or more. ESD is tricky though... all circuits have different requirements, and need to be analyzed properly before just throwing values in.

This application likely has a microswitch inside a waterproof resistant box, so no one is going to be be ESD'ing into the pin.

Your code is nice and compact. Although I thought you were going to use millis() for your 1ms interval. Because you've used a hard delay of 1ms, and the other code takes up at least 25 cycles or more, you will end up with about a 6 second error in 60 minutes. Not really bad, but it could be more accurate with a few more lines of code.

Also millis() or even the hard delay could be used for a proper debounce routine, with some counters. What you have there just doesn't look at the input again for 200ms, and only requires a 1ms glitch to acknowledge a quarter. I don't really know enough about the application signal to know if 1ms is necessarily bad, but without any counters you can't fine-tune it. I will say that because of the 200ms hard delay, you lose 200ms for every quarter... with balances out your previous error of 6 seconds and makes it more like 1.6 second error in 60 minutes.

You should bound the StopTime for good coding practices, not that it's really needed for the application.

The output should go HIGH when there is time on the timer, then you can drive an NPN transistor or darlington transistor which would drive the relay coil.

Again, nice and compact Fletcher.

Hi BrettW

Thanks for your input.

I made the comment about the pull-down resistor since no information was given about the switch in the coin box. In my head I I thought about it as a simpel switch: Quater is comming thougt => switch is closed, otherwise the switch is open.

With a simpel setup up (like the one I made on the breadboard) the closed switch would short cirkute the buttonPin and ground thuse drawing more then 40 mA and fry the pin.

I'm nut sure the:

delay(1);

is needen anymore - I had some stragne behaviour during testing and that line helped.

I have to disagree about the time glitch issue. The:

delay(debounceTime);

is within the first if sentence:

if ((thisReading != lastReading) and thisReading == LOW){

thefore delay is only performed if the switch is closed. But even if the delay was in the main loop I can't see that the timing would be inaccurate.

When I thought about this problem I had two ways to go:

  1. Using a SEC timer and count down.
  2. Using absolute time and compare with millis().

There were cons and pros for both ways. Way 1) would (most likely) be safe from roll-over error but I needed a correct SEC ticker and going by:

millis() % 1000 == 0;

was just a bit to dangerious (even for me.....)

So I chose way 2). That means I calcualte the stop time (in millisec) from the moment the quater switch is closed. If relayPin is HIGH I need to add time and if the pin is LOW I need to find the endpoint and turn on the realy.

The way I see it this use of absolute time would make the system accurate even with build in delays. Am I wrong in the asumption?

The downfall of this metode is the roll-over of millis(). If stopTime did a roll-over and millis() still was 2 min away the relay would go off. Putting in another quarter (HEY - stupid maching give me my hot water!!) would not help .. (GIVE me back my quaters!!) and you might have to buy a new quater box ;D

I thought about makeing a roll-over detection but ended up with a crude:

(((stopTime - now < 2000) or (stopTime - now > 2000))

what would work as long as the main loop is executing faster than 2000 millisec.

An internal counter like "secis()" (unsigned long) counting every SEC would be nice since the roll-over would be once 136 years ((2**32-1)/60/60/24/365 ~ 136).

Darn, this post became much longer than entended ::slight_smile:

-Fletcher