0

I have an arduino where I looking for an input event then do something. Unfortunately this input event can occur like 200 times and I only care about the first notification. Since I have a DS1302 timer handy in the circuit already for other purposes I was thinking using the minutes reading out of this to squelch this event.

Here is the code I come up with:

if (digitalRead(sensor_event)==HIGH)
  {

      if (RTC.read(tm)) {

              if(time_set)
                {
                       if (tm.Minute > time_pushed+5)
                           time_set=0; // Unlock
                 }
               else
                 {
                        if (tm.Minute >= 55)
                          time_pushed=0;
                        else
                          time_pushed=tm.Minute; // Set global volatile min value

                        print("Sensor event");  
                        time_set=1;
                 }  
      }
  }

About the seconds and hours I don't care all I need is if this event is detected then go ahead with a function and put it on a 5 minutes hold before checking it again.

The small issue here is the minute overflow. For example if the minute is just at 56 when this happens the time_pushed would be set to 56 then the time_pushed+5 would give 61 (which of course is never reached).

Is there a better way to code this? What is the name of this time issue I having to ignore an event until x minutes? What I found for this was mostly debounce but I'm sure this has to be handled by others sometimes too.

Thanks

5 Answers 5

1

Use the millis() function instead if you can accomodate a rollover after 50 days.

  • Take a reading of millis() at the start time.
  • time_pushed = millis() + 5 * 60 * 1000; // 5 minutes into future.

The rest of your code shouldn't require too much editing.

4
  • 1
    Assuming all three are unsigned values, if(current_time - start_time >= timeout), then rollover becomes irrelevant. Aug 13, 2016 at 19:00
  • This is exactly what I looking for so the rollover never cause a problem, yes the device will run over 50 days. But in your example let's say the timeout is 1 minute 10000 mS. If current_time eg 9991512 and the start time 9990512 and lets say it overflows back from 9991512 -> 0000000 how is this helps me?
    – catkillr
    Aug 13, 2016 at 19:13
  • @catkillr: Arithmetic on unsigned long is performed modulo 2^32, so the difference current_time - start_time is always correct modulo 2^32 even if there is an overflow. Aug 13, 2016 at 20:16
  • See gammon.com.au/millis for a detailed explanation of why this works.
    – Nick Gammon
    Aug 14, 2016 at 0:13
0

Calculate target time, rollover if necessary and check if time is already passed that point. As long as you are checking the time multiple times per minute you shouldn't run into any issues.

if (digitalRead(sensor_event)==HIGH)
{

  if (RTC.read(tm)) {

          if(time_set)
            {
                   if (tm.Minute >= targettime)
                       time_set=0; // Unlock
             }
           else
             {
                    targettime=tm.Minute+5; // Set global volatile min value
                    targettime=targettime % 60;

                    print("Sensor event");  
                    time_set=1;
             }  
  }
}
0

The proper way to do that depends on whether the event is guaranteed to happen at least once every 49.7 days. If this is the case then, as Tom Carpenter suggested in a comment, you can just test for now - last_event >= hold_time:

void loop()
{
    uint32_t now = millis();
    static uint32_t last_event = -hold_time;

    // Detect the event if it did not happen in the last five minutes.
    if (now - last_event >= hold_time
            && digitalRead(sensor_event) == HIGH) {
        Serial.println("Sensor event");
        last_event = now;
    }
}

If you can have 49.7 days without a single event, then this approach fails because last_event is ambiguous. The simplest solution is probably to implement a state machine with only two states, as the event detection is either “on hold” or it is not:

void loop()
{
    uint32_t now = millis();
    static bool on_hold;
    static uint32_t last_event;

    // Remove the hold after five minutes.
    if (on_hold && now - last_event >= hold_time) {
        on_hold = false;
    }

    // Detect the event.
    if (!on_hold && digitalRead(sensor_event) == HIGH) {
        Serial.println("Sensor event");
        on_hold = true;
        last_event = now;
    }
}

Now I would want to insist on a specific point: in either approach the millis() rollover is not an issue. The choice between the first and the second approach depends solely on the maximum time between the events, not on how long the program is meant to run.

0

Just to save a lot of headaches for everybody who wonders here and still have a problem with this code:

uint32_t hold_time = 1*60*1000; // 1 minute will never work

uint32_t hold_time = 60000; // 1 minute will work

You must define the value as an EXACT number, not putting in multiplication and rely on the compiler to calculate it for you.

Probably because it's unsigned long and screws up something with the multiplication but with the first example you will be put on hold for days, maybe forever.

1
-1

Don't bother with the RTC, just use millis(). Or just convert the time in hours/minutes/seconds to a single number in seconds (i.e. UNIX timestamp) and use that. Then use these time comparison macros:

#define time_after(unknown, known) ((long)(known) - (long)(unknown) < 0) 
#define time_before(unknown, known) ((long)(unknown) - (long)(known) < 0)
#define time_after_eq(unknown, known) ((long)(unknown) - (long)(known) >= 0) 
#define time_before_eq(unknown, known) ((long)(known) - (long)(unknown) >= 0)

Then do something like this in your main loop, with a variable unsigned long sample_timer that's initialized to 0 at the start of your program:

if (time_after_eq(millis(), sample_timer))
{
    if (event)
    {
        sample_timer = millis() + 5*60*1000;
        // handle event...
    }
}

These time comparison macros will properly handle overflow if you use unsigned long for your timestamps.

6
  • −1 because this can lead to signed integer overflow, which is undefined behavior (i.e. a bug). But you can fix the macros by doing the subtraction before the cast. Aug 13, 2016 at 20:10
  • Really? Then give the Linux kernel your down vote because that's where these macros came from: makelinux.net/books/lkd2/ch10lev1sec3 Aug 13, 2016 at 20:15
  • You found a bug in the Linux kernel. Congrats. But I would advise that, before reporting it, you make sure it hasn't already been fixed. Aug 13, 2016 at 20:20
  • Update: I checked the current Linux kernel and, as I suspected, the bug has already been addressed. The fix, jiffies: Avoid undefined behavior from signed overflow, was committed by Paul E. McKenney three years ago. I urge you to read the commit message, as it explains in more detail what I wrote in my first comment. And notice that McKenney’s fix goes exactly along my suggestion: subtract before cast. Also, when you know you have posted a buggy answer, it is good practice to either fix it or delete it. Aug 25, 2016 at 9:45
  • It's also good practice to comment before down voting. Aug 25, 2016 at 12:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.