-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Periodic BMS reset now at a specific time with the help of NTP #847
base: main
Are you sure you want to change the base?
Periodic BMS reset now at a specific time with the help of NTP #847
Conversation
… in the next 24 hours
…fall back to just resetting 24 hours after emulator starts if NTP fails or user doesnt set BMS-RESET-AT
@@ -44,6 +44,10 @@ | |||
#include "src/devboard/mqtt/mqtt.h" | |||
#endif // MQTT | |||
#endif // WIFI | |||
#ifdef PERIODIC_BMS_RESET_AT | |||
#include "src/devboard/utils/ntp_time.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see ntp being a useful optional add on in general, it would allot events and debug logging to show the date and time of the event instead of just the uptime figure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was thinking behind not doing NTP sync on the emulator and instead always counting from 0 millis. Did you not want a board that was reliant on internet connection ? According to the espressif docs there are robust enough timers in the esp32 for keeping time although the recommendation is to sync hourly. Also you would have to think about the other hardware that is supported and implications for that. My thinking was to keep it simple for this feature :)
@@ -124,6 +127,9 @@ void setup() { | |||
|
|||
xTaskCreatePinnedToCore((TaskFunction_t)&core_loop, "core_loop", 4096, &core_task_time_us, TASK_CORE_PRIO, | |||
&main_loop_task, CORE_FUNCTION_CORE); | |||
#ifdef PERIODIC_BMS_RESET_AT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If wifi is slow to connect could this fail to set the time? It would be good to periodically sync time to avoid drift and moving this code to a loop would avoid the wifi connection issue at startup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'm calculating an offset I wanted to do it as early as possible. If I had it in the main loop and constantly retrying I might end calculating an unaccurate off-set. e.g Maybe the internet doesn't work for first 30 mins then I calculate it.. A v2 of this would just have the main timer synced to NTP and the reset would be done on reaching a set time but that would involve refactoring lots of other things I don't know much about. I'm time poor :)
@@ -249,7 +249,7 @@ void handle_BMSpower() { | |||
|
|||
#ifdef PERIODIC_BMS_RESET | |||
// Check if 24 hours have passed since the last power removal | |||
if (currentTime - lastPowerRemovalTime >= powerRemovalInterval) { | |||
if ((currentTime + bmsResetTimeOffset) - lastPowerRemovalTime >= powerRemovalInterval) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be impacted by daylight savings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only does the time sync once on start up. If the clocks change its going to be out.
…ery 24 hours there after
…ery 24 hours there after
What
This PR implements the ability to reset the BMS at a specific time.
Also fixes the bug where the bms power is restored during a bms reset event but the bms is not given time to startup and CAN overrun messages appear in logs.
Why
To add more flexiblity to periodic reset. Currently it resets every 24 hours from when the emulator is turned on.
How
How does it do it?
As the Emulator board has no concept of time. The emulator can now sync with an NTP server (Using espressif Time library) at startup and work out the number of ms between now and a time in the future. That time difference is used as an offset when performing the BMS Reset.
Timezone, NTP servers and BMS Reset time are all configurable parameters.
<img width="673" alt="Screenshot 2025-02-03 at 14 29 23" src="https://github.com/user-attachments/assets/f047583
1-e0f5-4039-99c7-1970f2797d81" />