Add a 64bit version of the millis() function

[jgstroud]

millis() will rollover after 50 days leading to some interesting bugs. Create a 64bit version.

[jgstroud]

Fixes #261

[dkerr64]

Looks like the commit removed code related to syslog port. Was that intentional?

[jgstroud]

Nope. Something must not have merged properly. I'll rebase and update.

[jgstroud]

@dkerr64 good catch. fixed now. that was strange. its like part of a single commit was missing from my branch. rebased again and now I think it looks right.
I'm going to go ahead and keep running with this until I verify the rollover works before I merge it.

[mitchjs]

out of curiosity, what "interesting bugs" did a 59day rollover create?
as rollover of millis() is not usually problem (as long as everything is unsigned longs and subtraction used)

thanks
Mitch

[jgstroud]

In particular I saw my device go into recovery mode when I clicked the wall button. We have a timeout value that should have reset but the millis rolled and the timeout ended up being way off in the future

[dkerr64]

@dkerr64 good catch. fixed now. that was strange. its like part of a single commit was missing from my branch. rebased again and now I think it looks right. I'm going to go ahead and keep running with this until I verify the rollover works before I merge it.

Thanks, looks good now.

[dkerr64]

@jgstroud I added this for the ratgdo32 version, you can see the commit here.

There were quite a few unsigned long variables that I found needed to be change to long long (or uint64_t) which are missed in your changes here. I suggest a grep for unsigned long and/or a look at the changes I made for ratgdo32.

[jgstroud]

I'll take a look but I made sure it compiled without warning. I would have expected type mismatch warnings.

[dkerr64]

I'll take a look but I made sure it compiled without warning. I would have expected type mismatch warnings.

I googled and apparently it is a feature of the C compilers to accept assignment to different types, and it will truncate if necessary. You can have it warn with a -Wconversion flag. So I tried that... bad idea, thousands of warnings from just about every source file (not just ours).

[jgstroud]

Yeah, it's definitely legal in C. I really thought it would throw a warning if you didnt explicitly cast the data type, but I think I'm wrong there. I also forgot that a long is 32 bits on this device. Pushed new code that hopefully catches the rest of the missed variables

[dkerr64]

Yeah, it's definitely legal in C. I really thought it would throw a warning if you didnt explicitly cast the data type, but I think I'm wrong there. I also forgot that a long is 32 bits on this device. Pushed new code that hopefully catches the rest of the missed variables

Yes, I think you got them all now. Thanks.

[dkerr64]

Yeah, it's definitely legal in C. I really thought it would throw a warning if you didnt explicitly cast the data type, but I think I'm wrong there. I also forgot that a long is 32 bits on this device. Pushed new code that hopefully catches the rest of the missed variables

Yes, I think you got them all now. Thanks.

Spoke too soon. I found another one... lastDoorUpdateAt in web.cpp needs to be declared 64-bit.

[jgstroud]

I had 2 crashes after 10 days. I haven't analyzed the crash dumps yet to see if it is related to this patch, but leading up to that I had a storm of HK log messages asking for the firmware version. hundreds per second. so maybe unrelated.

[jgstroud]

Spoke too soon. I found another one... lastDoorUpdateAt in web.cpp needs to be declared 64-bit.

I thought I got that one 🤔

[jgstroud]

@dkerr64 pretty sure this change has introduced some instabilities. It needs some rethinking before merging.

[dkerr64]

ok, that is concerning. I would not have expected a change from storing a value as 32bit to storing at 64bit to make any difference.

[jgstroud]

There are only a few places where I think the rollover isn't handled properly. So, I think I'll fix that and keep with the 32bit version. I think we should only use the 64bit for the uptime. The ESP32 probably better handles 64bit. But if you are seeing crashes, I wouldn't rule this out.