Removing bogus values placed in a millis calculation after startup [SOLVED]

Projects Programming
1

I have several elapsed time millis calculations in my sketch.
One monitors a run status input and calculates the average run time
per number of ON/OFF cycles.

Whenever I load the UNO with a new code version the value initially displayed
for this average runtime calculation is always "4294967295."

As soon as the input registers a change of state the correct elapsed time values will display.
I have tried to clear this nuisance with "if value is greater than X make value = 0."
But for some reason that interferes with a getting a subsequent correct calculation.

This is not a big deal and the code works fine as soon as the digital input gets its first change of state.

Any suggestions on a clean way to make sure at startup your millis calculations are zeroed out?

Coding_Badly 2
3 Likes
3

No code posted so hard to be specific. Have you initialized the variable to zero in the declaration?

Another source of error could be overwriting the memory for the variable (eg, move a string that is too long into the variable declared just before it).

4

I set all of my previous times to millis as the last step of my setup function.

5

It seems to be a bug in your code. The millis() counter is initialized with 0 at startup.
The value 4294967295 that you got is 0xFFFFFFFF in HEX = the maximum possible value of the millis(). The reason for this answer is most likely due to incorrect use of unsigned arithmetic

6

I will try this. :+1:

7

In using millis I normally assign 'unsigned long' when declaring them. Is 'signed long'
a better choice?

8

is the way to go, unless you really know what's going on.

unsigned is key, as is using it in your code correctly.

Which might be a hint. About posting your code. Maybe. :expressionless:

a7

9

In the constants.h file I set it up as:

unsigned long total_daily_run_Millis = 0;

Would the compiler check for that or is that strictly the programmer's responsibility?

10

Please don't call the snippet police on me. I have about 2000 lines of code
so I didn't include it in its entirety.

" avg_hrs_per_cycle" at the bottom is the animal under investigation and the one that gets the max millis value on startup.

unsigned long blr_ON_mismatch_Millis = 0;
unsigned long blr_run_cmd_Millis = 0;
unsigned long blr_ON_Millis = 0;
unsigned long latest_on_cycle = 0;
unsigned long blr_daily_run_Millis = 0;
unsigned long total_daily_run_Millis = 0;
unsigned long blr_OFF_Millis = 0;  // used to monitor time boiler is OFF until restarting
unsigned long blr_OFF_interval = 0;
unsigned long blr_OFF_min = 0;
unsigned long blr_OFF_hr = 0;
unsigned long blr_OFF_mins = 0;

unsigned long level_alm_Millis = 0;

int daily_hours_elapsed = 0;  
int blr_cycles_counter = 0;   
int last_blr_state = 0;

if (blr_ON_status == false) {
      blr_OFF_interval = current_Millis - blr_OFF_Millis;  // track OFF time until next ON cycle
      blr_OFF_min = blr_OFF_interval / 60000;
      blr_OFF_hr = blr_OFF_interval / 3600000;
      blr_OFF_mins = blr_OFF_min % 60;
      blr_OFF_timeHack = 1;
      blr_ON_timeHack = 0;
    }
    if (blr_ON_stat == 0 && blr_OFF_timeHack == 0) {
      blr_OFF_timestamp = actualTime;
      blr_OFF_Millis = current_Millis;  // track OFF time until next ON cycle
      blr_OFF_timeHack = 1;
    }
    //                                                **** On/Off Cycle Counter ****
    if (blr_ON_stat != last_blr_state) {  // compare the boiler run status to its previous state
      if (blr_ON_status == true)          // if the state has changed to ON increment the counter;
      {                                   // goal is to only increment counter ONLY when boiler comes ON
        blr_cycles_counter++;
      }
      last_blr_state = blr_ON_stat;  // save current state as last state, for next time thru loop
    }
    //                                            **** Runtime Accumulator ****
    if (blr_ON_status == true && blr_on_state == false) {  // calculate daily boiler runtime
      blr_ON_Millis = current_Millis;
      blr_on_state = true;  // capture blr_on_millis one time only at start of ON cycle
      blr_off_state = false;
    }
    if (blr_ON_status == false && blr_off_state == false) {
      blr_OFF_Millis = current_Millis;
      blr_on_state = false;
      blr_off_state = true;  // capture blr_off_millis one time only at stop of current 'ON' cycle
      Serial.print("UNO System Time: Hour = ");
      Serial.print(hour(actualTime));
      Serial.print("  / Minute = ");
      Serial.print(minute(actualTime));
      Serial.print("  / Second = ");
      Serial.println(second(actualTime));
      latest_on_cycle = (blr_OFF_Millis - blr_ON_Millis);  // A - B = C
      blr_daily_run_Millis = (blr_daily_run_Millis + latest_on_cycle);
    }
    last_cycle_seconds = latest_on_cycle / 1000;  // these definitions must remain in void(loop) main code body
    last_cycle_minutes = latest_on_cycle / 60000;
    last_cycle_hours = latest_on_cycle / 3600000;
    last_cycle_run_minutes = last_cycle_minutes % 60;  // modulus operator gives remainder/leftover after division
    last_cycle_run_seconds = last_cycle_seconds % 60;
    //                                      ****  Current Cycle Runtime ****
    if (blr_ON_status == true) {
      current_on_cycle = current_Millis - blr_ON_Millis;
      current_cycle_seconds = current_on_cycle / 1000;
      current_cycle_minutes = current_on_cycle / 60000;
      current_cycle_hours = current_on_cycle / 3600000;
      current_cycle_run_minutes = current_cycle_minutes % 60;
      current_cycle_run_seconds = current_cycle_seconds % 60;
      total_daily_run_Millis = blr_daily_run_Millis + current_on_cycle;
    } else {  //  clear current run data when boiler goes OFF
      current_cycle_seconds = 0;
      current_cycle_minutes = 0;
      current_cycle_hours = 0;  // unsigned long current_cycle_hours
      current_cycle_run_minutes = 0;
      current_cycle_run_seconds = 0;
    }
    //                                          **** Total Daily Runtime ****
   
    seconds = total_daily_run_Millis / 1000;
    minutes = total_daily_run_Millis / 60000;
    hours = total_daily_run_Millis / 3600000;
    run_minutes = minutes % 60;  // modulus operator provides remainder/leftover value after division
    run_seconds = seconds % 60;

    avg_mins_per_cycle = (total_daily_run_Millis / 60000) / blr_cycles_counter;
    avg_hrs_per_cycle = (total_daily_run_Millis / 3600000) / blr_cycles_counter;
    avg_min_per_cycle = avg_mins_per_cycle % 60;
11

You could write a small example that compiles (!) and exhibits the behaviour.

12

More likely, forum members will just move on to another thread, where the user chooses to provide the required information.

13

Division by zero. You are responsible for avoiding it.

14

All I could glean from what you did post produces nothing but 0.

int blr_cycles_counter = 0;   
unsigned long total_daily_run_Millis = 0;
int avg_hrs_per_cycle = (total_daily_run_Millis / 3600000) / blr_cycles_counter;

void setup() {
  Serial.begin(115200);
  Serial.println("Ed the Wino!\n");

  Serial.println(avg_hrs_per_cycle);
}

void loop() {
}

I post it only to show you what we mean by reducing to a minimum and posting a complete working sketch which shows the issue.

I tried different types for avg_hrs_per_cycle. And confirmed it was not a few other things I won't mention since it was none of them.

a7

15

The division by zero (undefined behaviour) could also produce a zero. The outcome is compiler / platform / weather / solar flare / etcetera dependent. In other words, it would be handy for @edthewino to also indicate which board(s) they are using.

17

Ok, I took your suggestion and made a new sketch with just the code related to the
runtime average calculation.

I was not able to reproduce the same "4294967295" being placed in the Hrs. data field
when the processor first initialized.
But what I did discover is two particular lines of code if left in would cause the processor to halt/hangup. Commenting them out allowed the UNO to keep looping.
They are:

 avg_mins_per_cycle = (total_daily_run_Millis / 60000) / blr_cycles_counter;
  avg_hrs_per_cycle = (total_daily_run_Millis / 3600000) / blr_cycles_counter;

The line initializing 'blr_cycles_counter' to '0' is the culprit. As soon as I made it a '1'
the processor would continue running. So I think this is what's causing the bogus
number on initialization: divide by zero; not good I am thinking. As soon as the boiler goes through a cycle the phantom number goes away.
I need to have it start out with a zero but not sure what a workaround would be.
I did change all the unsigned long declarations to 'int' as I was troubleshooting the code.
Is it ok in this application to leave them as int's or stay with unsigned integer?

int blr_cycles_counter = 0;

```cpp

#include <math.h>

unsigned long blr_ON_mismatch_Millis = 0;
unsigned long blr_run_cmd_Millis = 0;
unsigned long blr_ON_Millis = 0;
int latest_on_cycle = 0;
unsigned long blr_daily_run_Millis = 0;
unsigned long total_daily_run_Millis = 0;
unsigned long blr_OFF_Millis = 0;  
int blr_OFF_interval = 0;
int blr_OFF_min = 0;
int blr_OFF_hr = 0;
int blr_OFF_mins = 0;
int daily_hours_elapsed = 0;
int blr_cycles_counter = 0;
int last_blr_state = 0;
int current_on_cycle = 0;
int current_cycle_seconds = 0;
int current_cycle_minutes = 0;
int current_cycle_hours = 0;
int current_cycle_run_minutes = 0;
int current_cycle_run_seconds = 0;

int last_cycle_seconds = 0;
int last_cycle_minutes = 0;
int last_cycle_hours = 0;
int last_cycle_run_minutes = 0;
int last_cycle_run_seconds = 0;

int seconds = 0;
int minutes = 0;
int hours = 0;
int run_minutes = 0;
int run_seconds = 0;

int avg_mins_per_cycle = 0;
int avg_hrs_per_cycle = 0;
int avg_min_per_cycle = 0;

unsigned long current_Millis = 0;;

boolean blr_ON_status = false;
boolean blr_on_state = false;   // used to capture millis() once when boiler comes on
boolean blr_off_state = false;  // used to capture millis() once when boiler goes off
boolean blr_on = false;

void setup() {
  Serial.begin(115200);
}

void loop() {
  current_Millis = millis();
  if (blr_ON_status == false && blr_off_state == false) {
    blr_OFF_Millis = current_Millis;
    blr_on_state = false;
    blr_off_state = true;  
    latest_on_cycle = (blr_OFF_Millis - blr_ON_Millis);  
    blr_daily_run_Millis = (blr_daily_run_Millis + latest_on_cycle);
  }
  last_cycle_seconds = latest_on_cycle / 1000;  
  last_cycle_minutes = latest_on_cycle / 60000;
  last_cycle_hours = latest_on_cycle / 3600000;
  last_cycle_run_minutes = last_cycle_minutes % 60;  
  last_cycle_run_seconds = last_cycle_seconds % 60;

  if (blr_ON_status == true) {
    current_on_cycle = current_Millis - blr_ON_Millis;
    current_cycle_seconds = current_on_cycle / 1000;
    current_cycle_minutes = current_on_cycle / 60000;
    current_cycle_hours = current_on_cycle / 3600000;
    current_cycle_run_minutes = current_cycle_minutes % 60;
    current_cycle_run_seconds = current_cycle_seconds % 60;
    total_daily_run_Millis = blr_daily_run_Millis + current_on_cycle;
  }
 
  seconds = total_daily_run_Millis / 1000;
  minutes = total_daily_run_Millis / 60000;
  hours = total_daily_run_Millis / 3600000;
  run_minutes = minutes % 60;  // modulus operator provides remainder/leftover value after division
  run_seconds = seconds % 60;

  avg_mins_per_cycle = (total_daily_run_Millis / 60000) / blr_cycles_counter;
  avg_hrs_per_cycle = (total_daily_run_Millis / 3600000) / blr_cycles_counter;
  avg_min_per_cycle = avg_mins_per_cycle % 60;

  Serial.println();
  Serial.print(" Avg runtime hour/min: ");
  Serial.print(avg_hrs_per_cycle);
  Serial.print(": ");
  Serial.print(avg_min_per_cycle);
  Serial.println();
  delay(1000);
 
}
18

See post 17 please. You were on the mark. :+1:

"Divide by zero" is something to avoid evidently.
My bad for not stating I have an Arduino UNO WiFi Rev2.

19

Use a flag (bool; seen_first_cycle seems a reasonable choice for the variable name). Set it (true) after the first cycle. Until the flag is set, don't go through any code that could divide by zero.

They are counts of events. It's impossible to have less than zero events. unsigned is the correct choice.

20

Because the Arduino Uno has a simple microcontroller, a division by zero will go wrong without warning. There is no error function that is called.
Sometimes a integer division by zero is runtime, I think that the result will be -1 or all FF (0xFFFFFFFF for a 32 bit variable).
Sometimes a integer division is already detected by the compiler, then some code might not even be generated. It is detected with all warnings turned on: warning: division by zero [-Wdiv-by-zero]

You have too many variables, too much if-else constructions, variables with almost identical names, and 2000 lines is still a small sketch for some of us.
There are programming techniques to create consistent code, or libraries that keep track of timing.
I think there are better ways to make your sketch easier to maintain.

A little improvement is how to use 'bool' variables. They are already 'true' or 'false', so you don't have to check them against 'true' or 'false':

bool umbrella;
bool raining;

if (!raining and umbrella)
{
  Serial.println("There was no need to bring an umbrella.");
}
23

If you have links to places I can get info on programming techniques or libraries I am all ears.

"A little improvement is how to use 'bool' variables. They are already 'true' or 'false', so you don't have to check them against 'true' or 'false':"

This was a eureka moment for me! Wow! I need that umbrella because I am soaking wet. :upside_down_face: