Long term operations. Hardware reset needed?

I am incorporating the Jaycar water flow meter into my project. This is using millis as a counter

void loop()
{ 
  if((millis() - oldTime) > 1000)    // Only process counters once per second
  { 
    // Disable the interrupt while calculating flow rate and sending the value to
    // the host
    detachInterrupt(sensorInterrupt);
    //lcd.setCursor(15, 0);
    //lcd.print("*");

    // Because this loop may not complete in exactly 1 second intervals we calculate
    // the number of milliseconds that have passed since the last execution and use
    // that to scale the output. We also apply the calibrationFactor to scale the output
    // based on the number of pulses per second per units of measure (litres/minute in
    // this case) coming from the sensor.
    flowRate = ((1000.0 / (millis() - oldTime)) * pulseCount) / calibrationFactor;

    // Note the time this processing pass was executed. Note that because we've
    // disabled interrupts the millis() function won't actually be incrementing right
    // at this point, but it will still return the value it was set to just before
    // interrupts went away.
    oldTime = millis();

  //tralala.............

    // Enable the interrupt again now that we've finished sending output
    attachInterrupt(sensorInterrupt, pulseCounter, FALLING);

i.e. millis runs constantly, is checked each time round the loop, and the calcs are made on that basis.

The project has to run unattended, freezing in the dark, for at least ninety days.

I understood that millis will roll over after about fifty days. Is this correct?

If so, I guess a software correction can be made, but it is a bit inelegant to have to poll every second for a condition you know will only happen once.

I am considering adding a hardware reset circuit comme ci

http://www.playwitharduino.com/?p=291&lang=en

This will operate once a day, which might be a wise precaution against unknown stuffups anyway.

Further to this exercise, I would need to store and retrieve two, maybe four, floating point numbers. I already have the SD in service but I wonder if it might be better to use EEPROM.

void loop()
{ 
  if((millis() - oldTime) > 1000)    // Only process counters once per second
  {

…is guaranteed to drift forward. Change the comparison to greater-than-or-equals-to.

    detachInterrupt(sensorInterrupt);

…is very unlikely to be necessary and will very likely result in missed events. Post your entire code for suggestions.

    flowRate = ((1000.0 / (millis() - oldTime)) * pulseCount) / calibrationFactor;

You should be calling millis exactly once at the top of loop and using the saved value. Calling it multiple times introduces an error.

    oldTime = millis();

…in most cases, this is a better choice…

    oldTime += 1000;

The project has to run unattended, freezing in the dark, for at least ninety days.

For testing, you can shorten the rollover point by using a 16 bit integer instead of a 32 bit integer.

I understood that millis will rollover after about fifty days. Is this correct?

Correct.

If so, I guess a software correction can be made…

The rollover can be handled without any correction.

I am considering adding a hardware reset circuit comme ci
http://www.playwitharduino.com/?p=291&lang=en

Unnecessary for two reasons: 1. The rollover can be handled without a reset. 2. The processor includes a watchdog. If you feel compelled to include a reset, use the watchdog (less hardware = less that can fail).

This will operate once a day, which might be a wise precaution against unknown stuffups anyway.

In my opinion, that is a foolish approach. Who’s to say that, even with the resets, your program will not get stuffed-up? The best course is to: completely and correctly solve the problem with as little code as possible #, verify when possible, thoroughly test when not possible.

Further to this exercise, I would need to store and retrieve two, maybe four, floating point numbers. I already have the SD in service but I wonder if it might be better to use EEPROM.

Is there a reason not to use the EEPROM?

# Einstein was a smart dude: “Everything should be kept as simple as possible, but no simpler.”

Thanks

I’m reasonably confident about the robustness of the code but this matter has come up on the Freetronics EtherTen Forum, and I have only just realised the potential issue with millis.

This is the code for the flow meter from Jaycar. At the moment, I am using it essentially unaltered.

/**
 * Water Flow Gauge
 *
 * Uses a hall-effect flow sensor to measure the rate of water flow and
 * output it via the serial connection once per second.
 * Copyright 2009 Jonathan Oxer <jon@oxer.com.au>
 * Copyright 2009 Hugh Blemings <hugh@blemings.org>
 *
 */

#include <LiquidCrystal.h>
LiquidCrystal lcd(8, 9, 16, 5, 6, 7);

byte sensorInterrupt = 0;  // 0 = pin 2; 1 = pin 3
byte sensorPin       = 2; //

float calibrationFactor = 6.7;

volatile byte pulseCount;  

float flowRate;
unsigned int flowMilliLitres;
unsigned long totalMilliLitres;
unsigned long oldTime;

void setup()
{
  lcd.begin(16, 2);
    lcd.clear();
      lcd.setCursor(0, 0);
      lcd.print(calibrationFactor);
      delay(2000);
  lcd.clear();
      lcd.setCursor(0, 0);
      lcd.print("Flow: ");
      lcd.setCursor(11, 0);   
      lcd.print("L/min");
      lcd.setCursor(0, 1);
      lcd.print("Total: ");
      lcd.setCursor(15, 1);   
      lcd.print("L");
  // Initialize a serial connection for reporting values to the host


  // Set up the pair of counter reset buttons and activate internal pull-up resistors

  pinMode(sensorPin, INPUT);
  digitalWrite(sensorPin, HIGH);

  pulseCount          = 0;
  flowRate            = 0.0;
  flowMilliLitres          = 0;
  totalMilliLitres         = 0;
  oldTime             = 0;

  // The Hall-effect sensor is connected to pin 17 (A3) which uses interrupt 0.
  // Configured to trigger on a FALLING state change (transition from HIGH
  // state to LOW state)
  attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
}

void loop()
{ 
  if((millis() - oldTime) > 1000)    // Only process counters once per second
  { 
    // Disable the interrupt while calculating flow rate and sending the value to
    // the host
    detachInterrupt(sensorInterrupt);
    //lcd.setCursor(15, 0);
    //lcd.print("*");

    // Because this loop may not complete in exactly 1 second intervals we calculate
    // the number of milliseconds that have passed since the last execution and use
    // that to scale the output. We also apply the calibrationFactor to scale the output
    // based on the number of pulses per second per units of measure (litres/minute in
    // this case) coming from the sensor.
    flowRate = ((1000.0 / (millis() - oldTime)) * pulseCount) / calibrationFactor;

    // Note the time this processing pass was executed. Note that because we've
    // disabled interrupts the millis() function won't actually be incrementing right
    // at this point, but it will still return the value it was set to just before
    // interrupts went away.
    oldTime = millis();

    // Divide the flow rate in litres/minute by 60 to determine how many litres have
    // passed through the sensor in this 1 second interval,
 flowMilliLitres = (flowRate / 60) * 1000;

    // Add the litres passed in this second to the cumulative total
    totalMilliLitres += flowMilliLitres;

    unsigned int frac;
     frac = (flowRate - int(flowRate)) * 10;

    lcd.setCursor(6, 0);

    if(int(flowRate) < 10)
    {
      lcd.print(" ");
    }
    lcd.print((int)flowRate);   // Print the integer part of the variable
    lcd.print('.');             // Print the decimal point
    lcd.print(frac, DEC) ;      // Print the fractional part of the variable


    lcd.setCursor(7, 1);

lcd.print(int(totalMilliLitres / 1000));


    // Reset the pulse counter so we can start incrementing again
    pulseCount = 0;

    // Enable the interrupt again now that we've finished sending output
    attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
  }
}

/**
 * Invoked by interrupt0 once per rotation of the hall-effect sensor. Interrupt
 * handlers should be kept as small as possible so they return quickly.
 */
void pulseCounter()
{
  // Increment the pulse counter
  pulseCount++;
}

I understand the on-board watchdog is not be relied upon to shut down the W5100.

The best course is to: completely and correctly solve the problem with as little code as possible, verify when possible, thoroughly test when not possible.

Agreed. I have month to get this installed and see the reset and any watchdog as a precaution.

Is there a reason not to use the EEPROM?

Yes, two. I don’t know how, and I find the (wealth of) blurb on this matter quite depressing to read. It appears to be written for the cognoscenti. The best stuff I have seen uses Arduino.h and I believe therefore out of date. I have not had time to actually try any of this, as I have been able to avoid EEPROM altogether up to yesterday.
I imagine the EEPROM will be a lot faster than the SD but I can do the deed when the flow meter won’t be working.

Nick_Pyner: The project has to run unattended, freezing in the dark, for at least ninety days.

Then I guess you won't be using an LCD. I suggest you make your sketch as simple as possible (but no simpler!) and that means getting rid of features that you don't need, such as the LCD hardware and related software.

The sample code you posted looks pretty mediocre and I'd suggest using it for inspiration rather than taking it as the starting point for your own code. The functionality you're describing sounds very simple and I would be looking for a sketch containing a couple of dozen lines of code, none of it doing anything complex, and all of it similar to examples that ship with the Arduino IDE.

[quote author=Coding Badly link=topic=164027.msg1224959#msg1224959 date=1367475175]

void loop()
{ 
  if((millis() - oldTime) > 1000)    // Only process counters once per second
  {

...is guaranteed to drift forward. Change the comparison to greater-than-or-equals-to.

[/quote]

I think I have to disagree with the part about cumulative error (drifting forward), though he should make the change you suggest to not start 1mS late. I'm thinking that as long as he fixes the oldTime variable to be bumped by exactly 1000 each time, it will be fine. Even though he will respond 1mS late each time, the timeout boundaries won't shift if they are increasing by exactly 1000 each time. He'll just spend an extra millisecond in the loop wasting time leaving a maximum of 999mS for his processing. To reiterate the OP should make the change but I don't think it will cause drift. Calling millis() again to compute the next roll time will cause drift though. Even trying to use the value obtained in the first call to adjust oldTime wouldn't be accurate. The only way is to prime oldTime once with a call to millis() and then base everything off that by bumping oldTime by 1000 each time.

He might consider only calling millis() once and sticking it in a variable since he needs the value a couple of times.

Yes the LCD will probably go. The data goes out to cosm, and possibly to a local display by 2.4 GHz. The data to cosm also goes to SD card.

???? I'm not aware of anything similar included with the IDE. I have another version from Seeedstudio which is about the same size. I haven't tried it yet as one station has to ready for the ski season, which is imminent, and I am running out of time. Seeedstudio seems much the same, and the current setup is going quite well, so far. I am more interested in flow rate but the total volume readings from three turbines are hovering around 99% to 100.9% accuracy for several days, which is better than expected.

afremont: He might consider only calling millis() once and sticking it in a variable since he needs the value a couple of times.

That sounds pretty good. Thanks. I haven't really come to grips with this code. I have only just started to use it.

I am considering adding a hardware reset circuit comme ci

http://www.playwitharduino.com/?p=291&lang=en

This will operate once a day, which might be a wise precaution against unknown stuffups anyway.

Back in 1994 or 1995, I built a car-alarm for my '94 van. (Different microprocessor, different timer- scheme). It's been running 24/7 for almost 20 years!!! It runs continuously... Even when it's "off" it's still running and just reading the state of an on/off input and executing a different part of the code. The only time it's reset is when the battery in the van dies every few years.

I think I have to disagree with the part about cumulative error (drifting forward)…

// Only process counters once per second

if((millis() - oldTime) > 1000)

By including the comment, what did the author intend to happen? Will the code execute as described? Will the code meet the author’s intentions?

Or does the code deviate from the author’s expectations? In what way does the code deviate?

No offense, I was just sayin'.... It will do exactly what the comments say, it will do the process precisely once per second, but it will start the first iteration with a 1mS delay because of the > (greater than sign). After that, the interval will be 1000mS, not 999mS or 1001mS. There is no cumulative drift, that's all I was saying.

afremont: No offense, I was just sayin'....

I'm not offended.

It will do exactly what the comments say...

Maybe this will get the lightbulb to come on...

oldTime = 0;
millis() --> 1000;
delta = 1000;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  // Condition not true; then-clause not executed

oldTime = 0;
millis() --> 1001;
delta = 1001;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  oldTime = millis();  // oldTime = 1001;
  // 1001 ms elapsed

oldTime = 1001;
millis() --> 2001;
delta = 1000;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  // Condition not true; then-clause not executed

oldTime = 1001;
millis() --> 2002;
delta = 1001;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  oldTime = millis();  // oldTime = 2002;
  // 1001 ms elapsed since the last execution of the then-clause
  // 2002 ms total elapsed

The flaw in your example is that you are not bumping oldTime by 1000, you are reloading it from millis(). That will never work right if timekeeping is the goal. If oldTime is incremented by 1000 each time, there will be no drift.

oldTime = 0;
millis() --> 1000;
delta = 1000;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  // Condition not true; then-clause not executed

oldTime = 0;
millis() --> 1001;
delta = 1001;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  oldTime = millis();  // oldTime = 1001;        <------  This is wrong, should be oldTime += 1000;
  // 1001 ms elapsed

oldTime = 1001;        <---- should be 1000;
millis() --> 2001;
delta = 1000;
if((millis() - oldTime) > 1000)    // Only process counters once per second
  // Condition not true; then-clause not executed   <----- Actually will be true since 2001 - 1000 > 1000

oldTime = 1001;      <----- Should be 1000
millis() --> 2002;     <----- will clear at 2001
delta = 1001;
if((millis() - oldTime) > 1000)    // Only process counters once per second      <----- 2001 - 1000 > 1000  TRUE
  oldTime = millis();  // oldTime = 2002;     <------  Again, this is wrong  oldTime += 1000;
  // 1000 ms elapsed since the last execution of the then-clause
  // 2001 ms total elapsed

See where I’m coming from?

EDIT: The progression, as I see it, will be:

millis() oldTime
1001 0
2001 1000
3001 2000
4001 3000

etc…

afremont: The flaw in your example...

My example (and claim of "drifting") is based on the code in the original post.

See where I'm coming from?

Yes. You've modified the code from the original to suit your argument.

Now, on to something more productive...

@Nick_Pyner, this should provide a better framework for what you are trying to do…

/**
 * Water Flow Gauge
 *
 * Uses a hall-effect flow sensor to measure the rate of water flow and
 * output it via the serial connection once per second.
 * Copyright 2009 Jonathan Oxer <jon@oxer.com.au>
 * Copyright 2009 Hugh Blemings <hugh@blemings.org>
 *
 */

const byte sensorInterrupt = 0;  // 0 = pin 2; 1 = pin 3
const byte sensorPin       = 2; //

const float calibrationFactor = 6.7;

const unsigned long UPDATE_INTERVAL = 1000UL;

volatile byte pulseCount;  
static byte OldPulseCount;
static byte CurrentPulseCount;

static float flowRate;
static unsigned int flowMilliLitres;
static unsigned long totalMilliLitres;
static unsigned long oldTime;

void setup()
{
  // Set up the pair of counter reset buttons and activate internal pull-up resistors

  pinMode(sensorPin, INPUT);
  digitalWrite(sensorPin, HIGH);

  pulseCount          = 0;
  flowRate            = 0.0;
  flowMilliLitres          = 0;
  totalMilliLitres         = 0;
  oldTime             = 0;

  // The Hall-effect sensor is connected to pin 17 (A3) which uses interrupt 0.
  // Configured to trigger on a FALLING state change (transition from HIGH
  // state to LOW state)
  attachInterrupt(sensorInterrupt, pulseCounter, FALLING);
}

void loop()
{ 
  unsigned long CurrentTime;

  CurrentTime = millis();

  if((CurrentTime - oldTime) >= UPDATE_INTERVAL)
  { 
    CurrentPulseCount = pulseCount;

    flowRate = ((1000.0 / (CurrentTime - oldTime)) * (byte)(CurrentPulseCount - OldPulseCount)) / calibrationFactor;

    flowMilliLitres = (flowRate / 60) * 1000;

    totalMilliLitres += flowMilliLitres;


    OldPulseCount = CurrentPulseCount;
    oldTime += UPDATE_INTERVAL;
  }
}

/**
 * Invoked by interrupt0 once per rotation of the hall-effect sensor. Interrupt
 * handlers should be kept as small as possible so they return quickly.
 */
void pulseCounter()
{
  // Increment the pulse counter
  pulseCount++;
}

Points of interest…

• Interrupts are never disabled so you will not lose pulses.

• As written, the pulse count handling requires the datatype to be byte (unsigned eight bits). If you need a larger datatype the assignment to CurrentPulseCount / read of pulseCount will have to be protected with a critical section.

My first post was to point out the flaw in the original code, I have not changed anything to suit my argument. You seem to be changing your argument. You said that leaving the = sign out would guarantee drift. I disagree. The only thing that was causing drift is the improper incrementing of oldTime, not the comparison. That would lead to drift regardless. The proof of that is in what happens when a 2mS delay occurs outside the loop().

EDIT: I wasn't trying to change any arguments, but I will add another. Trying to adjust oldTime by calling millis() rather than by a fixed increment is not going to work reliably in the long run. That's why I pointed it out right away and based my argument on something that would work. Outside delays or the fact that the return value will eventually differ between the two calls in the loop will result in long term drift. :)

EDIT: I put the smiley on the end to be friendly, but the Icon it prints is kind of repugnant and gives off an in-your-face 'tude. That's not what I intend by a long shot.

Thanks Gentlemen, I will pursue this. Right now I have three turbines under test here in Dee Why, using the original code. I will leave them for a few days and then explore this further.

I was concerned about losing pulses, as I want serial print at one second intervals, cosm at ten secs, etc.

I have yet to integrate the flow meter with the rest of the code.

My finding is that the millis() is not that great at keeping time correct, so I added a DS1307 RTC which takes about 2.5ms to read. I have to read it every minute to have the time in my home alarm centre correct.

But to avoid time skipping and also to avoid having an oldTime variable I did this:

  if ( ( millis() % 1000) < 500 ) {
    if (!doneThisSecond ) {
      doneThisSecond = true;  // Put this first
      lcdWriteTime(); // Slow
    }
  }
  else {
    doneThisSecond = false;
  }

teddyz:
My finding is that the millis() is not that great at keeping time correct, so I added a DS1307 RTC which takes about 2.5ms to read. I have to read it every minute to have the time in my home alarm centre correct.

But to avoid time skipping and also to avoid having an oldTime variable I did this:

  if ( ( millis() % 1000) < 500 ) {

if (!doneThisSecond ) {
      doneThisSecond = true;  // Put this first
      lcdWriteTime(); // Slow
    }
  }
  else {
    doneThisSecond = false;
  }

Bah, humbug. :wink: millis() can keep time just as good as your oscillator is capable. I have a clock project that does it and I’ve had it run for days and it loses about 30 seconds/day which is in agreement with the main oscillator accuracy. About the only thing I know of that will wreck timekeeping with it is SoftwareSerial because it runs with interrupts disabled while sending or receiving; at 9600 baud or less, interrupts will be disabled for more than a mS at a time. This causes lost interrupts on Timer0 which leads to lost time using millis().

teddyz: My finding is that the millis() is not that great at keeping time correct, so I added a DS1307 RTC which takes about 2.5ms to read. I have to read it every minute to have the time in my home alarm centre correct.

I think you have rather missed the point. The DS1307 delivers seconds, here we are talking about 1/1000 of a second. To that effect millis is a thousand times more accurate than a DS1307 and, as far as I'm aware, the only way to get a DS1307 to talk in milliseconds would be to combine it with millis. This would be a pretty pointless exercise since, if you extended your findings just a little further, you would find that the DS1307 isn't that flash in the accuracy department either. Indeed, your findings would lead you to exactly what you should have expected - that is, for timekeeping, a $2 DS1307 and $2's worth of millis components are both about as accurate as a $2 digital watch.

I completely fail to see why you have to read the clock every minute to keep a home alarm centre correct but, by that standard, your accuracy requirement is 1/60,000 that of this exercise.

Nick_Pyner: I think you have rather missed the point. The DS1307 delivers seconds, here we are talking about 1/1000 of a second.

Most important is that I am happy with my system. :) The drift of my system before the DS1307 was in the range of minutes per day, and that might be because my too large interrupts. My DS1307 is impressively accurate, with an average drift of a second per day. I was lucky (it is not adjustable), and the system runs in room temperature, which of course helps.

About precision, I still make use of the millis() for getting periods of one second. avoiding the oldTime variable gives me the advantage of catching up if I was a bit late the last second. So I have all precision I need.

Reading the DS1307 every minute makes my system add or remove a leap second if needed, which it does more than once per hour. I will know exactly which second the burglar emptied my home :)

Also, if the system reboot I still know what time it is. If you are going to log data each second for three months you will have 8 million lines of data if done in ascii format. Having a good date/time stamp would help, I guess.