Browse Source

fix: GPS update when counter wraps

See #2878.

Introduces a new function `millisHasPassed` that correctly handles when
either millis() or the checked number overflow.

Also changes long to explicit uint32_t so that tests pass on non-32-bit
platforms, too.
pull/2882/head
Christoph Koehler 4 weeks ago
parent
commit
037fb9f592
Failed to extract signature
  1. 14
      examples/companion_radio/ui-new/UITask.cpp
  2. 3
      examples/companion_radio/ui-new/UITask.h
  3. 5
      examples/companion_radio/ui-orig/UITask.cpp
  4. 3
      examples/companion_radio/ui-tiny/ScrollingStatusBar.h
  5. 16
      examples/companion_radio/ui-tiny/UITask.cpp
  6. 3
      examples/companion_radio/ui-tiny/UITask.h
  7. 5
      examples/simple_repeater/UITask.cpp
  8. 7
      examples/simple_room_server/UITask.cpp
  9. 7
      examples/simple_sensor/UITask.cpp
  10. 1
      src/helpers/ArduinoHelpers.h
  11. 17
      src/helpers/ArduinoTimer.h
  12. 7
      src/helpers/esp32/SerialBLEInterface.cpp
  13. 5
      src/helpers/sensors/EnvironmentSensorManager.cpp
  14. 5
      src/helpers/sensors/MicroNMEALocationProvider.h
  15. 40
      test/test_millis/test_millis.cpp
  16. 4
      variants/heltec_mesh_solar/target.cpp
  17. 4
      variants/heltec_tracker/target.cpp
  18. 4
      variants/meshadventurer/target.cpp
  19. 4
      variants/nano_g2_ultra/target.cpp
  20. 4
      variants/t1000-e/target.cpp
  21. 6
      variants/thinknode_m1/target.cpp

14
examples/companion_radio/ui-new/UITask.cpp

@ -77,7 +77,7 @@ public:
}
void poll() override {
if (millis() >= dismiss_after) {
if (millisHasPassed(dismiss_after)) {
_task->gotoHomeScreen();
}
}
@ -153,10 +153,10 @@ class HomeScreen : public UIScreen {
int sensors_nb = 0;
bool sensors_scroll = false;
int sensors_scroll_offset = 0;
int next_sensors_refresh = 0;
uint32_t next_sensors_refresh = 0;
void refresh_sensors() {
if (millis() > next_sensors_refresh) {
if (millisHasPassed(next_sensors_refresh)) {
sensors_lpp.reset();
sensors_nb = 0;
sensors_lpp.addVoltage(TELEM_CHANNEL_SELF, (float)board.getBattMilliVolts() / 1000.0f);
@ -773,7 +773,7 @@ void UITask::loop() {
}
#endif
#if defined(BACKLIGHT_BTN)
if (millis() > next_backlight_btn_check) {
if (millisHasPassed(next_backlight_btn_check)) {
bool touch_state = digitalRead(PIN_BUTTON2);
#if defined(DISP_BACKLIGHT)
digitalWrite(DISP_BACKLIGHT, !touch_state);
@ -799,7 +799,7 @@ void UITask::loop() {
if (curr) curr->poll();
if (_display != NULL && _display->isOn()) {
if (millis() >= _next_refresh && curr) {
if (millisHasPassed(_next_refresh) && curr) {
_display->startFrame();
int delay_millis = curr->render(*_display);
if (millis() < _alert_expiry) { // render alert popup
@ -827,7 +827,7 @@ void UITask::loop() {
_auto_off = millis() + AUTO_OFF_MILLIS;
}
#endif
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
#endif
@ -838,7 +838,7 @@ void UITask::loop() {
#endif
#ifdef AUTO_SHUTDOWN_MILLIVOLTS
if (millis() > next_batt_chck) {
if (millisHasPassed(next_batt_chck)) {
uint16_t milliVolts = getBattMilliVolts();
if (milliVolts > 0 && milliVolts < AUTO_SHUTDOWN_MILLIVOLTS) {
if(!board.isExternalPowered()) {

3
examples/companion_radio/ui-new/UITask.h

@ -6,6 +6,7 @@
#include <helpers/SensorManager.h>
#include <helpers/BaseSerialInterface.h>
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/sensors/LPPDataHelpers.h>
#ifndef LED_STATE_ON
@ -37,7 +38,7 @@ class UITask : public AbstractUITask {
unsigned long _alert_expiry;
int _msgcount;
unsigned long ui_started_at, next_batt_chck;
int next_backlight_btn_check = 0;
uint32_t next_backlight_btn_check = 0;
#ifdef PIN_STATUS_LED
int led_state = 0;
int next_led_change = 0;

5
examples/companion_radio/ui-orig/UITask.cpp

@ -1,5 +1,6 @@
#include "UITask.h"
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/TxtDataHelpers.h>
#include "../MyMesh.h"
@ -335,7 +336,7 @@ void UITask::loop() {
_need_refresh = true;
_firstBoot = false;
}
if (millis() >= _next_refresh && _need_refresh) {
if (millisHasPassed(_next_refresh) && _need_refresh) {
_display->startFrame();
renderCurrScreen();
_display->endFrame();
@ -351,7 +352,7 @@ void UITask::loop() {
_auto_off = millis() + AUTO_OFF_MILLIS;
}
#endif
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
}

3
examples/companion_radio/ui-tiny/ScrollingStatusBar.h

@ -1,6 +1,7 @@
#pragma once
#include <helpers/ui/DisplayDriver.h>
#include <helpers/ArduinoTimer.h>
#ifndef STATUS_BAR_SCROLL_MS
@ -93,7 +94,7 @@ public:
// Returns true if the status bar needs a redraw this frame.
bool needsRedraw() {
if (_text_width <= _display_width) return _needs_redraw; // static, no scrolling
return millis() >= _next_scroll;
return millisHasPassed(_next_scroll);
}
// Render the status bar via DisplayDriver.

16
examples/companion_radio/ui-tiny/UITask.cpp

@ -81,7 +81,7 @@ public:
}
void poll() override {
if (millis() >= dismiss_after) {
if (millisHasPassed(dismiss_after)) {
_task->gotoHomeScreen();
}
}
@ -116,10 +116,10 @@ class HomeScreen : public UIScreen {
int sensors_nb = 0;
bool sensors_scroll = false;
int sensors_scroll_offset = 0;
int next_sensors_refresh = 0;
uint32_t next_sensors_refresh = 0;
void refresh_sensors() {
if (millis() > next_sensors_refresh) {
if (millisHasPassed(next_sensors_refresh)) {
sensors_lpp.reset();
sensors_nb = 0;
sensors_lpp.addVoltage(TELEM_CHANNEL_SELF, (float)board.getBattMilliVolts() / 1000.0f);
@ -632,7 +632,7 @@ void UITask::loop() {
}
#endif
#if defined(BACKLIGHT_BTN)
if (millis() > next_backlight_btn_check) {
if (millisHasPassed(next_backlight_btn_check)) {
bool touch_state = digitalRead(PIN_BUTTON2);
#if defined(DISP_BACKLIGHT)
digitalWrite(DISP_BACKLIGHT, !touch_state);
@ -675,7 +675,7 @@ void UITask::loop() {
isSerialEnabled());
bool status_dirty = _statusBar.needsRedraw();
bool content_dirty = (millis() >= _next_refresh && curr);
bool content_dirty = (millisHasPassed(_next_refresh) && curr);
if (status_dirty || content_dirty) {
_display->startFrame();
@ -712,7 +712,7 @@ void UITask::loop() {
_auto_off = millis() + AUTO_OFF_MILLIS;
}
#endif
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
#endif
@ -723,7 +723,7 @@ void UITask::loop() {
#endif
#ifdef AUTO_SHUTDOWN_MILLIVOLTS
if (millis() > next_batt_chck) {
if (millisHasPassed(next_batt_chck)) {
_cached_batt_mv = getBattMilliVolts();
if (_cached_batt_mv > 0 && _cached_batt_mv < AUTO_SHUTDOWN_MILLIVOLTS) {
if(!board.isExternalPowered()) {
@ -742,7 +742,7 @@ void UITask::loop() {
next_batt_chck = millis() + 8000;
}
#else
if (_display != NULL && _display->isOn() && millis() >= next_batt_chck) {
if (_display != NULL && _display->isOn() && millisHasPassed(next_batt_chck)) {
_cached_batt_mv = getBattMilliVolts();
next_batt_chck = millis() + 8000;
}

3
examples/companion_radio/ui-tiny/UITask.h

@ -6,6 +6,7 @@
#include <helpers/SensorManager.h>
#include <helpers/BaseSerialInterface.h>
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/sensors/LPPDataHelpers.h>
#include "ScrollingStatusBar.h"
@ -41,7 +42,7 @@ class UITask : public AbstractUITask {
unsigned long _alert_expiry;
int _msgcount;
unsigned long ui_started_at, next_batt_chck;
int next_backlight_btn_check = 0;
uint32_t next_backlight_btn_check = 0;
uint16_t _cached_batt_mv;
#ifdef PIN_STATUS_LED
int led_state = 0;

5
examples/simple_repeater/UITask.cpp

@ -1,6 +1,7 @@
#include "UITask.h"
#include "target.h"
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/CommonCLI.h>
#ifndef USER_BTN_PRESSED
@ -129,14 +130,14 @@ void UITask::loop() {
#endif
if (_display->isOn()) {
if (millis() >= _next_refresh) {
if (millisHasPassed(_next_refresh)) {
_display->startFrame();
renderCurrScreen();
_display->endFrame();
_next_refresh = millis() + 1000; // refresh every second
}
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
}

7
examples/simple_room_server/UITask.cpp

@ -1,5 +1,6 @@
#include "UITask.h"
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/CommonCLI.h>
#ifndef USER_BTN_PRESSED
@ -95,7 +96,7 @@ void UITask::renderCurrScreen() {
void UITask::loop() {
#ifdef PIN_USER_BTN
if (millis() >= _next_read) {
if (millisHasPassed(_next_read)) {
int btnState = digitalRead(PIN_USER_BTN);
if (btnState != _prevBtnState) {
if (btnState == USER_BTN_PRESSED) { // pressed?
@ -113,14 +114,14 @@ void UITask::loop() {
#endif
if (_display->isOn()) {
if (millis() >= _next_refresh) {
if (millisHasPassed(_next_refresh)) {
_display->startFrame();
renderCurrScreen();
_display->endFrame();
_next_refresh = millis() + 1000; // refresh every second
}
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
}

7
examples/simple_sensor/UITask.cpp

@ -1,5 +1,6 @@
#include "UITask.h"
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
#include <helpers/CommonCLI.h>
#ifndef USER_BTN_PRESSED
@ -95,7 +96,7 @@ void UITask::renderCurrScreen() {
void UITask::loop() {
#ifdef PIN_USER_BTN
if (millis() >= _next_read) {
if (millisHasPassed(_next_read)) {
int btnState = digitalRead(PIN_USER_BTN);
if (btnState != _prevBtnState) {
if (btnState == USER_BTN_PRESSED) { // pressed?
@ -113,14 +114,14 @@ void UITask::loop() {
#endif
if (_display->isOn()) {
if (millis() >= _next_refresh) {
if (millisHasPassed(_next_refresh)) {
_display->startFrame();
renderCurrScreen();
_display->endFrame();
_next_refresh = millis() + 1000; // refresh every second
}
if (millis() > _auto_off) {
if (millisHasPassed(_auto_off)) {
_display->turnOff();
}
}

1
src/helpers/ArduinoHelpers.h

@ -2,6 +2,7 @@
#include <Mesh.h>
#include <Arduino.h>
#include <helpers/ArduinoTimer.h>
class VolatileRTCClock : public mesh::RTCClock {
uint32_t base_time;

17
src/helpers/ArduinoTimer.h

@ -0,0 +1,17 @@
#pragma once
#include <stdint.h>
// Wrap-safe check: has `now` reached or passed timestamp `t`?
// Handles uint32_t rollover correctly for intervals up to ~24 days.
inline bool millisHasPassed(uint32_t now, uint32_t t) {
return (int32_t)(now - t) >= 0;
}
#ifdef ARDUINO
#include <Arduino.h>
// Convenience wrapper using Arduino's millis().
inline bool millisHasPassed(uint32_t t) {
return millisHasPassed((uint32_t)millis(), t);
}
#endif

7
src/helpers/esp32/SerialBLEInterface.cpp

@ -1,4 +1,5 @@
#include "SerialBLEInterface.h"
#include <helpers/ArduinoTimer.h>
#include "esp_mac.h"
// See the following for generating UUIDs:
@ -183,12 +184,12 @@ size_t SerialBLEInterface::writeFrame(const uint8_t src[], size_t len) {
#define BLE_WRITE_MIN_INTERVAL 60
bool SerialBLEInterface::isWriteBusy() const {
return millis() < _last_write + BLE_WRITE_MIN_INTERVAL; // still too soon to start another write?
return !millisHasPassed(_last_write + BLE_WRITE_MIN_INTERVAL); // still too soon to start another write?
}
size_t SerialBLEInterface::checkRecvFrame(uint8_t dest[]) {
if (send_queue_len > 0 // first, check send queue
&& millis() >= _last_write + BLE_WRITE_MIN_INTERVAL // space the writes apart
&& millisHasPassed(_last_write + BLE_WRITE_MIN_INTERVAL) // space the writes apart
) {
_last_write = millis();
pTxCharacteristic->setValue(send_queue[0].buf, send_queue[0].len);
@ -238,7 +239,7 @@ size_t SerialBLEInterface::checkRecvFrame(uint8_t dest[]) {
oldDeviceConnected = deviceConnected;
}
if (adv_restart_time && millis() >= adv_restart_time) {
if (adv_restart_time && millisHasPassed(adv_restart_time)) {
if (pServer->getConnectedCount() == 0) {
BLE_DEBUG_PRINTLN("SerialBLEInterface -> re-starting advertising");
pServer->getAdvertising()->start(); // re-Start advertising

5
src/helpers/sensors/EnvironmentSensorManager.cpp

@ -1,4 +1,5 @@
#include "EnvironmentSensorManager.h"
#include <helpers/ArduinoTimer.h>
#include <Wire.h>
@ -896,11 +897,11 @@ void EnvironmentSensorManager::stop_gps() {
void EnvironmentSensorManager::loop() {
#if ENV_INCLUDE_GPS
static unsigned long next_gps_update = 0;
static uint32_t next_gps_update = 0;
if (gps_active) {
_location->loop();
}
if ((long)(millis() - next_gps_update) > 0) {
if (millisHasPassed(next_gps_update)) {
if(gps_active){
#ifdef RAK_WISBLOCK_GPS

5
src/helpers/sensors/MicroNMEALocationProvider.h

@ -4,6 +4,7 @@
#include <MicroNMEA.h>
#include <RTClib.h>
#include <helpers/RefCountedDigitalPin.h>
#include <helpers/ArduinoTimer.h>
#ifndef GPS_EN
#ifdef PIN_GPS_EN
@ -42,7 +43,7 @@ class MicroNMEALocationProvider : public LocationProvider {
int8_t _claims = 0;
int _pin_reset;
int _pin_en;
unsigned long next_check = 0;
uint32_t next_check = 0;
long time_valid = 0;
unsigned long _last_time_sync = 0;
static const unsigned long TIME_SYNC_INTERVAL = 1800000; // Re-sync every 30 minutes
@ -141,7 +142,7 @@ public :
if (!isValid()) time_valid = 0;
if ((long)(millis() - next_check) > 0) {
if (millisHasPassed(next_check)) {
next_check = millis() + 1000;
// Re-enable time sync periodically when GPS has valid fix
if (!_time_sync_needed && _clock != NULL && (millis() - _last_time_sync) > TIME_SYNC_INTERVAL) {

40
test/test_millis/test_millis.cpp

@ -0,0 +1,40 @@
#include <gtest/gtest.h>
#include <helpers/ArduinoTimer.h>
#include <stdint.h>
static constexpr uint32_t MAX = UINT32_MAX;
TEST(MillisHasPassed, ReturnsTrueWhenNowExceedsTarget) {
EXPECT_TRUE(millisHasPassed(1000, 500));
}
TEST(MillisHasPassed, ReturnsTrueWhenNowEqualsTarget) {
EXPECT_TRUE(millisHasPassed(1000, 1000));
}
TEST(MillisHasPassed, ReturnsFalseWhenNowIsBeforeTarget) {
EXPECT_FALSE(millisHasPassed(500, 1000));
}
TEST(MillisHasPassed, ReturnsTrueAfterRollover) {
// Target was set when millis was near MAX: next_update = (MAX - 500) + 1000
// which wraps to 499. Now millis has rolled over to 500.
uint32_t target = MAX - 500 + 1000; // == 499 via wrapping
EXPECT_TRUE(millisHasPassed(500, target));
}
TEST(MillisHasPassed, ReturnsFalseJustBeforeRolloverFires) {
// Target is 499 (wrapped), but now is only 498 — not there yet.
uint32_t target = MAX - 500 + 1000; // == 499
EXPECT_FALSE(millisHasPassed(498, target));
}
TEST(MillisHasPassed, ReturnsFalseWhenNowIsHighAndTargetIsHigherStill) {
// millis near MAX but hasn't reached target yet
EXPECT_FALSE(millisHasPassed(MAX - 500, MAX - 100));
}
int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

4
variants/heltec_mesh_solar/target.cpp

@ -66,11 +66,11 @@ bool SolarSensorManager::querySensors(uint8_t requester_permissions, CayenneLPP&
}
void SolarSensorManager::loop() {
static long next_gps_update = 0;
static uint32_t next_gps_update = 0;
_location->loop();
if (millis() > next_gps_update) {
if (millisHasPassed(next_gps_update)) {
if (_location->isValid()) {
node_lat = ((double)_location->getLatitude())/1000000.;
node_lon = ((double)_location->getLongitude())/1000000.;

4
variants/heltec_tracker/target.cpp

@ -71,11 +71,11 @@ bool HWTSensorManager::querySensors(uint8_t requester_permissions, CayenneLPP& t
}
void HWTSensorManager::loop() {
static long next_gps_update = 0;
static uint32_t next_gps_update = 0;
_location->loop();
if (millis() > next_gps_update) {
if (millisHasPassed(next_gps_update)) {
if (gps_active && _location->isValid()) {
node_lat = ((double)_location->getLatitude())/1000000.;
node_lon = ((double)_location->getLongitude())/1000000.;

4
variants/meshadventurer/target.cpp

@ -65,9 +65,9 @@ bool MASensorManager::querySensors(uint8_t requester_permissions, CayenneLPP& te
}
void MASensorManager::loop() {
static long next_gps_update = 0;
static uint32_t next_gps_update = 0;
_location->loop();
if(millis() > next_gps_update && gps_active) {
if (millisHasPassed(next_gps_update) && gps_active) {
if(_location->isValid()) {
node_lat = ((double)_location->getLatitude()) / 1000000.;
node_lon = ((double)_location->getLongitude()) / 1000000.;

4
variants/nano_g2_ultra/target.cpp

@ -73,7 +73,7 @@ bool NanoG2UltraSensorManager::querySensors(uint8_t requester_permissions, Cayen
}
void NanoG2UltraSensorManager::loop() {
static long next_gps_update = 0;
static uint32_t next_gps_update = 0;
if (!gps_active) {
return; // GPS is not active, skip further processing
@ -81,7 +81,7 @@ void NanoG2UltraSensorManager::loop() {
_location->loop();
if (millis() > next_gps_update) {
if (millisHasPassed(next_gps_update)) {
if (_location->isValid()) {
node_lat = ((double)_location->getLatitude()) / 1000000.;
node_lon = ((double)_location->getLongitude()) / 1000000.;

4
variants/t1000-e/target.cpp

@ -147,11 +147,11 @@ bool T1000SensorManager::querySensors(uint8_t requester_permissions, CayenneLPP&
}
void T1000SensorManager::loop() {
static long next_gps_update = 0;
static uint32_t next_gps_update = 0;
_nmea->loop();
if (millis() > next_gps_update) {
if (millisHasPassed(next_gps_update)) {
if (gps_active && _nmea->isValid()) {
node_lat = ((double)_nmea->getLatitude())/1000000.;
node_lon = ((double)_nmea->getLongitude())/1000000.;

6
variants/thinknode_m1/target.cpp

@ -69,8 +69,8 @@ bool ThinkNodeM1SensorManager::querySensors(uint8_t requester_permissions, Cayen
}
void ThinkNodeM1SensorManager::loop() {
static long next_gps_update = 0;
static long last_switch_check = 0;
static uint32_t next_gps_update = 0;
static uint32_t last_switch_check = 0;
// Check GPS switch state every second
if (millis() - last_switch_check > 1000) {
@ -98,7 +98,7 @@ void ThinkNodeM1SensorManager::loop() {
_location->loop();
if (millis() > next_gps_update) {
if (millisHasPassed(next_gps_update)) {
if (_location->isValid()) {
node_lat = ((double)_location->getLatitude())/1000000.;
node_lon = ((double)_location->getLongitude())/1000000.;

Loading…
Cancel
Save