Added FDRS Timekeeping

[aviateur17]

You can reject if you want to add this to a different branch in order to wait to include in the main release branch. This adds code in Gateway for NTP Time server queries via WiFi. Compiled in Arduino 1.8.19 with and without USE_WIFI defined.

[timmbogner]

I love that you did it all without the time.h library. That definitely is better than I could have made it. My (literally) only concern presently is the DNS config. Is there any way to do that automatically? We'll at least have to make the definition a string probably, to be beginner-friendly and match MQTT config. That's really my only feedback. Otherwise it's great. I'll probably just leave this open through the new release, and we can discuss further details like how to share the time in the discussion thread.

[aviateur17]

I love that you did it all without the time.h library.

Yeah, I wanted to write the code with as little dependencies as possible to simplify the compile process and size of the code

My (literally) only concern presently is the DNS config. Is there any way to do that automatically? We'll at least have to make the definition a string probably, to be beginner-friendly and match MQTT config. That's really my only feedback. Otherwise it's great.

We can default the DNS server to a public DNS server like Google or some other DNS server if you'd like. That way we minimize the chance that people would have issues but still let them set the DNS server if they'd like. If it's the format of the IP address with the commas I'll have to go back and look at how I can change it into a string. It's most likely possible if that is desired.

Kinda related to this, I'm thinking about adding optional Static IP address configuration as an advanced option to the default DHCP configuration.

I'll probably just leave this open through the new release, and we can discuss further details like how to share the time in the discussion thread.

Sounds good!

[aviateur17]

My (literally) only concern presently is the DNS config. Is there any way to do that automatically? We'll at least have to make the definition a string probably, to be beginner-friendly and match MQTT config. That's really my only feedback.

Fixed in commit https://github.com/timmbogner/Farm-Data-Relay-System/pull/142/commits/cce5fe57d4142d07621341645a2fa6789fba22a5

[aviateur17]

In my commit https://github.com/timmbogner/Farm-Data-Relay-System/pull/142/commits/dcda381ea743498fb69474efe74b21505c68aee1 in the gateway_time.h and node_time.h files there is a couple of loadFDRS() calls with the current time and slew rate. I've disabled those by default as I think casual users would not care about that information. But just FYI if you want to do something with those like only enable when DEBUG is enabled and disable otherwise?

My thoughts on next feature adds in this topic:

1. share time via ESP-NOW from gateway to nodes
2. Option to disable DST for areas that do not observe DST
3. Add timestamps to DataReadings while maintaining backwards compatibility for DataReadings that do not have timestamps

I'm not sure how soon I'll get to those features. I have a couple of others I want to work with such as enable OTA updates for WiFi enabled gateways and perhaps BLE UART communication. I'll try to add the ESP-NOW part in the next few days and then the others may be a week or two or so.

[aviateur17]

In https://github.com/timmbogner/Farm-Data-Relay-System/pull/142/commits/a27d34ccccb6e5852fcee30068fd93c50043af7e, I removed the DNS server because the WiFi.useDNS() function does not work the way I expected it. Also, we'll just use the DNS server that is provided by the DHCP server. We will just make the assumption that the DHCP server has a valid DNS server entered ;) Once the branch with OTA and static IP are merged with this one users will have the option to define their own DNS server along with static IP address.

[aviateur17]

@timmbogner, question on events/actions. Based on the repeater examples it seems that the convention is that ESPNOW1_ACT and LORA1_ACT are directed towards the nodes and, oppositely, ESPNOW2_ACT and LORA2_ACT are directed towards the front end? I'm thinking in context of periodically sending the time from the front-end to the nodes that the actions to be taken should be under ESPNOW1_ACT and LORA1_ACT but the documentation doesn't really specify any directionality but the examples seem to imply what I stated above. I think up to now we really haven't had much or any traffic in the front-end to node direction so it hasn't been important to clarify that. So I'm thinking that I'm going to use ESPNOW1_ACT and LORA1_ACT as events/actions to determine where to direct the time data. Let me know your thoughts please. Thanks! The other challenge is that those events are really meant to handle DataPackets and not SystemCommands so there will be that challenge to distinguish between what type of information is being sent.

[timmbogner]

Great question. It's actually the big question for me in terms of timekeeping. First, you're right. The convention I've been using is for LORA1 to be towards the front-end and LORA2 towards the nodes and repeaters. But I would like to keep it a loose convention, never forcing the user to set it up any given way. Eventually I think I might add more neighbors, in which case having a pre-determined role for any of them a little unclear to a new user.

So I reckon there are two paths to think down here...

1. We re-use the existing routing configuration for SystemPackets (or specifically just time info). Essentially building a SystemPacket alternative into sendSerial(), sendESPNowNbr(), etc. I've thought about this, but it's not my first choice.
2. We more or less flood all interfaces with the time when we get it, sending the time out to UART, ESP-NOW/LoRa neighbors, broadcast to LoRa nodes, send to ESP-NOW peers, just goin nuts. The exception being the interface in which the time was received. I think this would avoid loops. This would also enable a device with an RTC located remotely to send a valid time inward to nodes that are closer to the front-end.

Let me know what you think!

[aviateur17]

I totally agree with you that we should keep it as flexible as possible for any future expansion and your item 2 above is the way to go. I think for now I'll set up time updates for ESP-NOW to go to Neighbor 2 (acting as repeater) and peers (acting as gateway). For LoRA time updates, I will send to Neighbor 2 (acting as repeater) and broadcast (acting as gateway). If, in the future, you tweak how things work it can be easily changed as I will try to neatly put the code in functions.

On a slightly different topic, as the code matures I think we may need to revisit function names to match what LoRa is doing to what ESP-NOW is doing as closely as possible for clarity. We have functions for initialization, functions for looping, lower level functions that handle the nitty gritty data encapsulation details and higher level functions that call the lower level functions. as more functionality is added it might be good to standardize on the names. Something that came to my mind while reviewing the code and something to ponder for the future.

[timmbogner]

I think it should send to all neighbors, just in case. A fringe configuration that is currently possible is that the gateway connected to the internet has two different neighbors, each being a repeater going out in different directions. This is feasible with WiFi-to-LoRa gateways and Ethernet-to-ESP-NOW gateways.

We may need to better define a condition where a neighbor is not present in configs, which would actually be good. #define LORA_NEIGHBOR_1 0x00 is what I define a "null" neighbor as, since in current usage 0x00 should only be an MQTT gateway and thus not listening to any radios. That's not great though, and it would be better to be able to just disable it. I don't think a 'NULL' or '-1' value would be safe, but I haven't looked too far into it.

[timmbogner]

On a slightly different topic, as the code matures I think we may need to revisit function names to match what LoRa is doing to what ESP-NOW is doing as closely as possible for clarity. We have functions for initialization, functions for looping, lower level functions that handle the nitty gritty data encapsulation details and higher level functions that call the lower level functions. as more functionality is added it might be good to standardize on the names. Something that came to my mind while reviewing the code and something to ponder for the future.

I'll definitely ponder this. I noticed when making functions for the recent reshuffle that I had begun setting_names_like_this() instead of likeThis() as I had usually done starting out. There are certain conventions like '-FDRS' as a suffix (beginFDRS()) is used for "public" functions. I'm always trying to prepare to shift the library to being class-based eventually, and that's my attempt to emulate what might become for example: FDRS.begin();

[aviateur17]

I think it should send to all neighbors, just in case. A fringe configuration that is currently possible is that the gateway connected to the internet has two different neighbors, each being a repeater going out in different directions. This is feasible with WiFi-to-LoRa gateways and Ethernet-to-ESP-NOW gateways.

I'm just afraid that if we send data back in the other direction, towards the front end, that will cause a loop. In general, all of the data should be sent in one direction, but there is no directionality in the code, because up to now there was no reason to send information from the front end down to the node.

[timmbogner]

I'm just afraid that if we send data back in the other direction, towards the front end, that will cause a loop.

I'll admit, that possibility is why I'm not 100% sure of myself. Preventing the time data from being returned to its source is my means of preventing this. So if the time was received from ESPNOW1, it would never be sent back to that device, only ESPNOW2 and peers.

[timmbogner]

I should say though, you're welcome to keep it how you think is best for the time-being.

[aviateur17]

Going through the code to implement timekeeping via ESP-NOW seems like there is quite a bit to be modified. I wonder if this would be better as a different PR. The issue is that this PR was started before the last major commit to main branch so there are things that were committed to main that are conflicting with this code. So I think the next related commit to this might be better committed against main and wait till this PR is also completed.

[timmbogner]

Dangit... holdupasec

[timmbogner]

Soo... I thought that first commit had resolved the conflicts, then it added the other when I pushed it. My damage control didn't help. I dunno 😣

Here is a branch without the extra stuff that I think should be okay? Sorry... I guess it should just be reverted. My bad. I honestly didn't think I was pushing it directly to your fork, I sorta thought there was an intermediate branch for this pull request.

[aviateur17]

No problem. I think I can reject the pr and push again against the new main branch. I'll look at it probably tomorrow.

On Mon, Mar 13, 2023, 7:28 PM Timm Bogner @.***> wrote:

Soo... I thought that first commit had resolved the conflicts, then it added the other when I pushed it. My damage control didn't help. I dunno 😣

Here is a branch https://github.com/timmbogner/Farm-Data-Relay-System/tree/timekeeping-fixed without the extra stuff that I think should be okay? Sorry... dunno what to do at this point. I honestly didn't think I was pushing it directly to your fork, I sorta thought there was an intermediate branch for this pull request.

— Reply to this email directly, view it on GitHub https://github.com/timmbogner/Farm-Data-Relay-System/pull/142#issuecomment-1467164405, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOTXENIOVEVOD775GGCU6WLW363SPANCNFSM6AAAAAAVI5QZ3Q . You are receiving this because you authored the thread.Message ID: @.***>

[aviateur17]

Okay, I think I fixed the conflicts but I need to go through and make sure stuff compiles and then add my latest round of ESP-NOW changes. So don't merge this in yet. In the future, if I see conflicts with my PRs I know how to fix them :)

I guess the issue now is that this PR should get merged against main branch and not the older release_2023_01 branch. Not sure if that can be changed or I just need to close this PR and open a new one. Probably the latter.

[Edit]: Guess I can just update the "base branch" and we're good to go. Learning all this good stuff.

[timmbogner]

In the future, if I see conflicts with my PRs I know how to fix them :)

:D I was so close to leaving it alone, but was convinced I'd figured out how to do it.

Guess I can just update the "base branch" and we're good to go.

Oh wow... so simple!

[aviateur17]

Update: Been pulling my hair out on a few issues the past several days. I think most of them are figured out. Will be submitting time propagation via serial and ESP-NOW today (I hope).

[aviateur17]

Sending 4 byte time as a float isn't going to work. The value is truncated from 32 bits to 25 bits, thus losing the last 7 bits of time data. So the only way to accurately send time across is via a the SystemPacket type packet. I tried converting the time to a float and dividing by 100 but that didn't seem to work. So maybe there is still a way but I already spent too much time on it and figuring out other issues.

[aviateur17]

Not done yet. Still need to correct a DST/Std time issue and do more testing with ESP-NOW and LoRa. It might be a few days until I come back to this.

[aviateur17]

@timmbogner, I was hoping to implement all of this without adding any libraries but I'm not sure how to "easily" calculate whether the current date is within daylight savings or not. I was hoping the tm struct element tm_isdst would work but I'm not sure how it knows what time zone is being used. Using ESP libraries aren't going to work because I think this is more of an Arduino centric project so I would propose using the "de facto" Arduino timezone library https://github.com/JChristensen/Timezone

Any comments on that or other ideas?

[timmbogner]

I am still getting caught up... There is one commit I need to make to fix custom SPI, then I plan to get up to speed on this PR and the others.

I really appreciate you taking this on though, because I clearly wouldn't have had time to start this timekeeping project for a good while.

[timmbogner]

I see the conundrum about DST.

I was hoping the tm struct element tm_isdst would work but I'm not sure how it knows what time zone is being used.

My loose understanding is that if tm_isdst is 1, then if you are observing daylight saving time, you would want to set the time ahead an hour. So we could have a CONFIG_OFFSET_HOURS -6 , then a config USE_DST. if tm_isdst is 1 and USE_DST is defined, then the time gets moved ahead.

I could be wrong about that.

I went over this once and will do so a couple more times to get a good feel for what you're up to. It all looks cool! The only concern I have so far is that sendESPNow() was documented and actually was used commonly in old configurations. I'm leaning towards the more portable neighbor functions now, but I still think we'll need to make sure that void sendESPNow(uint8_t address) will be available for backwards compatibility.

[timmbogner]

I just remembered that sendESPNow actually was sendESPNOW for most of its existence, so it's not a big deal to change it again. It could change to sendESPNowDirect() maybe...

[aviateur17]

I am still getting caught up... There is one commit I need to make to fix custom SPI, then I plan to get up to speed on this PR and the others.

Now I'm having issues with the ESP32 hanging while loading LoRa, I think it's related to SPI interface as when I remove LoRa it boots just fine. I'm trying to figure that out. The main branch works just fine so it's some change that has been made since main.

[aviateur17]

My loose understanding is that if tm_isdst is 1, then if you are observing daylight saving time, you would want to set the time ahead an hour. So we could have a CONFIG_OFFSET_HOURS -6 , then a config USE_DST. if tm_isdst is 1 and USE_DST is defined, then the time gets moved ahead.

The problem is that I don't see a way to set a time zone such that tm_isdst knows when to change from DST to STD time. Ideally we shouldn't have to download the controller to make this transition. Without a ton of coding I'm not sure how to do this so I'd look at adding the library mentioned earlier.

I went over this once and will do so a couple more times to get a good feel for what you're up to. It all looks cool! The only concern I have so far is that sendESPNow() was documented and actually was used commonly in old configurations. I'm leaning towards the more portable neighbor functions now, but I still think we'll need to make sure that void sendESPNow(uint8_t address) will be available for backwards compatibility.

I see sendESPNow as a "lower level function" that should not be called directly but only referred to by other "higher level" functions. It basically encapsulates the data type handling needed by higher level functions and handles calls to the Espressif API calls. I could be missing something though, where would changing the signature or parameters of the function affect the backward compatibility?

[Edit] I see how that the function was used in earlier versions in the actions. So I guess what I'll have to do is keep that existing one for backwards compatibility. I'll make some comments on which functions need to be kept for backwards compatibility. Thanks for seeing that and bringing it up.

[timmbogner]

Now I'm having issues with the ESP32 hanging while loading LoRa, I think it's related to SPI interface as when I remove LoRa it boots just fine. I'm trying to figure that out. The main branch works just fine so it's some change that has been made since main.

I've made a couple SPI fixes recently so check main's recent history for clues.

such that tm_isdst knows when to change from DST to STD time.

My understanding is that tm_isdst will be known and set by the time server, essentially it should always be 1 during the summer. The boolean could be translated like "are we within the time period when clocks are generally set forward? 1 if yes, 0 if no". It doesn't care where we are. I'm assuming the "spring forward/fall back" date is known by the server and it will change the value on that date. I'm curious though, if this is indeed the case, how RTCs know. Would they have it pre-coded? To be fair : the tutorial I learned that from was for Python, and the accompanying example didn't make sense to me. I could be totally off base.

So I guess what I'll have to do is keep that existing one for backwards compatibility.

Admittedly, I forgot that I didn't really care when I changed it from sendESPNOW to sendESPNow, so it doesn't really matter now. What if you changed the low-level version to transmitESPNow? That would have the benefit of matching LoRa a little bit.

[aviateur17]

My understanding is that tm_isdst will be known and set by the time server, essentially it should always be 1 during the summer. The boolean could be translated like "are we within the time period when clocks are generally set forward? 1 if yes, 0 if no". It doesn't care where we are. I'm assuming the "spring forward/fall back" date is known by the server and it will change the value on that date. I'm curious though, if this is indeed the case, how RTCs know. Would they have it pre-coded? To be fair : the tutorial I learned that from was for Python, and the accompanying example didn't make sense to me. I could be totally off base.

All the time server sends is the number of seconds since 1900 in UTC time. There's no timezone or DST information so it's up to the application to determine all of that.

[edit] Perhaps there is more information in the data that is just discarded. I'll see if I can dig into that and see if there is other data sent.

Admittedly, I forgot that I didn't really care when I changed it from sendESPNOW to sendESPNow, so it doesn't really matter now. What if you changed the low-level version to transmitESPNow? That would have the benefit of matching LoRa a little bit.

Good thought on the rename. I had a similar thought.

[timmbogner]

All the time server sends is the number of seconds since 1900 in UTC time. There's no timezone or DST information so it's up to the application to determine all of that.

Yeah brain-fart about it coming from the time server. I also was under the impression that most countries used the same date, but I see even US and EU are different, so my theory is sort of falling apart. I'll keep reading.

[aviateur17]

Looks like DST information isn't in the NTP standard UDP packet data: https://www.meinbergglobal.com/english/info/ntp-packet.htm

[timmbogner]

Looks like DST information isn't in the NTP standard UDP packet data

Yeah makes sense now that I think about it. I was forgetting how it all worked 🙄 Most of what I said here was pretty wrong as it turns out, so I'ma go back to researching. If we need the library then so be it, but I'd rather avoid dependencies of course.

[timmbogner]

Spitballing: It looks like DST is divided between two major observation periods. What if we had CONFIG_OFFSET_HOURS and then the options of USE_DST_US or USE_DST_EU. We can code in the logic for either one's start/end dates (unless this is too laborious) and then be able to figure it all out ourselves. This leaves out Egypt, Paraguay, Chile, NZ, and parts of Australia of course, so a bit messy.

[aviateur17]

Yeah, that was going to be my approach. So use the NTP time to calculate current date and see if it is in DST or STD time. The problem is the time change is not a specific date but like the second Sunday in March and the first Sunday in November. So the date of change is different in every year. So then you'd have to calculate the date for the second Sunday in March in the current year. So to calculate that you feed what you can into the timeInfo struct. I'm not sure that it will take in a day of week and a month and a year and give you a specific date. I think that is where I got stuck.

[timmbogner]

problem is the time change is not a specific date

Yeah that's the step I thought could be too laborious. At the least you could just use the Timezone library for now and hope to come up with something else later. Not the end of the world if we need to use the library.

[aviateur17]

I'm sure ChatGPT knows what to do but it's too busy right now to answer my question. :)

[aviateur17]

Could plug in a date like March 10th and then get the day of week. Then add or subtract the number of days from the current day of week to get to Sunday to get the date needed. Then calculate the difference between today's date and the known date to see if we are in DST or not. I'll see if I can get that to work.

[timmbogner]

I'm sure ChatGPT knows what to do but it's too busy right now to answer my question. :)

It's escaped! The hunt is on!

[aviateur17]

US DST testing output: US DST Start - 2nd Sunday in March - 02:00 local time US DST End - 1st Sunday in November - 02:00 local time

The current local date/time is: Wed Mar  1 17:33:39 2023 STD

The current local date/time is: Sun Mar 12 01:59:55 2023 STD
Time change from STD -> DST
The current local date/time is: Sun Mar 12 03:00:05 2023 DST

The current local date/time is: Sun Nov  5 01:59:55 2023 DST
Time change from DST -> STD
The current local date/time is: Sun Nov  5 01:00:05 2023 STD
EndTest

EU DST Testing output: (local time = UTC +2) EU DST Start - last Sunday in March - 01:00 UTC EU DST End - last Sunday in October - 01:00 UTC

The current UTC date/time is: Wed Mar  1 23:33:39 2023
The current local date/time is: Thu Mar  2 01:33:39 2023 STD

The current UTC date/time is: Sun Mar 26 00:59:54 2023
The current local date/time is: Sun Mar 26 02:59:55 2023 STD
Time change from STD -> DST
The current UTC date/time is: Sun Mar 26 01:00:04 2023
The current local date/time is: Sun Mar 26 04:00:05 2023 DST

The current UTC date/time is: Sun Oct 29 00:59:55 2023
The current local date/time is: Sun Oct 29 03:59:55 2023 DST
Time change from DST -> STD
The current UTC date/time is: Sun Oct 29 01:00:05 2023
The current local date/time is: Sun Oct 29 03:00:05 2023 STD
EndTest

Code:

void setup() {
beginFDRS();
delay(10000);
now = 1677713619; // 3/1/2023 UTC
setTime(0);
updateTime();
printTime();
delay(5000);

// now = 1678607970; // 3/12/2023 STD -> DST  US
now = 1679792370; // 3/26/2023 std -> dst EU
setTime(0);
updateTime();
printTime();
delay(25000);
updateTime();
printTime();
delay(10000);
updateTime();
printTime();

// now = 1699167570; // 11/5/2023 DST -> STD US
now = 1698541170; // 10/29/2023 DST -> STD EU
setTime(0);
updateTime();
printTime();
delay(25000);
updateTime();
printTime();
delay(10000);
updateTime();
printTime();

DBG("EndTest");
}

[aviateur17]

Was having issues with MQTT messages not sending - just generating the error. Figured out that the Arduino PubSubClient default size is 256 bytes and need to increase the default buffer by using setBufferSize.

https://pubsubclient.knolleary.net/api#setBufferSize

[timmbogner]

Was having issues with MQTT messages not sending - just generating the error. Figured out that the Arduino PubSubClient default size is 256 bytes and need to increase the default buffer by using setBufferSize.

Interesting! Does that mean, essentially, that sending >256 bytes of JSON wasn't ever working? Yikes 😬 I thought I had sent big packets via MQTT before, but thinking about it... not sure when. I'm not able to test it presently, but I'll look at that ASAP.

[aviateur17]

I think everything is working now so I'm going to get the last changes into the commit and then make sure everything compiles in Arduino and then should be done on this.

[aviateur17]

Of course my return type esp_err_t isn't valid for ESP8266 and the return types between ESP32 and ESP8266 for some functions are not consistent so I think I'm going to have to make another custom type.

[timmbogner]

Hm... that sounds familiar now that you mention it. One of several mysterious 32 vs 8266 incongruencies. Can you copy the declaration (or whatever) from ESP32 into _fdrsdatatypes.h and wrap it with #ifdef ESP8266?

I'm planning to devote some upcoming rainy days to testing all of this new stuff out, including the other PRs.

[aviateur17]

Hm... that sounds familiar now that you mention it. One of several mysterious 32 vs 8266 incongruencies. Can you copy the declaration (or whatever) from ESP32 into _fdrsdatatypes.h and wrap it with #ifdef ESP8266?

Very good idea, I was thinking exactly the same. I'm going to be on vacation for most of this week so I won't be able to finish this PR until late this week at the earliest. I think it's all complete except to make sure everything compiles properly in Arduino.

[timmbogner]

I just jumped into testing this and am working towards ethernet compatibility. At first, if(WiFi.status() == WL_CONNECTED) { never became true here. I added || eth_connected and it made it to the next step, but times out getting NTP response. Any ideas?

[timmbogner]

Scratch that! I just looked over and it was displaying the time. Must have been a one-off error.

[aviateur17]

Timm, while compiling this branch for Adafruit Feather M0 (SAMD21 micro) it seems that the settimeofday() function does not work for that microcontroller. The compilation succeeds but the linking step at the end fails. I'm not sure without that function if the time will be set in the SAMD21 micro. Do I just put some #ifdefs around that function call and we can have someone test the functionality on the SAMD21? I know you would like to be as microcontroller agnostic as possible. It seems like there would be another library needed to use the RTC functionality in the SAMD21 to keep time in that micro.

[timmbogner]

I'm not sure yet. This gave me the idea to test compilation for Pico, and predictably "esp_err_t" isn't defined for it (for starters). I think that it would be okay to use #ifdefs to switch between chip-specific RTC routines. This might come up again when adding support for varying external RTC modules. If there is a library needed, it would be okay to require it for timekeeping, but my hangup is having it required to install for users who may not need timekeeping.

Looks like AVR doesn't have "sys/time.h" either.

For now, should we stipulate that timekeeping is only for Espressif chips? Have you confirmed it on anything else?

I have been playing with the gateway timekeeping exclusively so far and everything seems fine on 8266, 32, and S3. My only thought right now:

I have an MQTT gateway with an ESP-NOW repeater. The MQTT gateway gets the accurate time and gives it to the repeater. However, the repeater will then end up giving its skewed time back to the MQTT gateway. What if instead of just sharing the time periodically, it was restricted to only happening when the gateway is given a verified-accurate time? So devices would only share the time when they receive a "fresh" time, either by NTP or a gateway sending one. This applies for sharing amongst gateways... nodes would be different and I haven't gotten to them.