From c71f24fe6649a80f611fb763929a920b8a00a4c9 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Thu, 15 Oct 2026 23:07:37 -0700 Subject: [PATCH] Close GPS-owned UARTs before sleeping receivers --- .../sensors/EnvironmentSensorManager.cpp | 100 +++++++++--------- .../sensors/EnvironmentSensorManager.h | 3 + .../sensors/MicroNMEALocationProvider.h | 5 + test/fixtures/wio_tracker_l2_gps_power.cpp | 3 + test/test_companion_hardware_serial_gps.py | 17 ++- test/test_esp32_gpio_hold.py | 27 +++-- test/test_esp32_gps_discovery.py | 75 ++++++++++--- test/test_gps_upstream_adaptations.py | 37 ++++++- test/test_nrf52_variant_contracts.py | 2 +- test/test_rak_gps_power.py | 13 ++- test/test_wio_tracker_l2_gps_power.py | 3 + 11 files changed, 200 insertions(+), 85 deletions(-) diff --git a/src/helpers/sensors/EnvironmentSensorManager.cpp b/src/helpers/sensors/EnvironmentSensorManager.cpp index 5fcd3afa6..affda7897 100644 --- a/src/helpers/sensors/EnvironmentSensorManager.cpp +++ b/src/helpers/sensors/EnvironmentSensorManager.cpp @@ -353,14 +353,6 @@ static void setRakGpsControl(uint8_t pin, bool shared_power_rail, digitalWrite(pin, enabled ? active : static_cast(!active)); } -static void stopRakGpsUart() { - Serial1.end(); - // UARTE teardown disconnects PSEL but does not restore GPIO configuration. - // Remove output drive and pulls so serial pads cannot back-power an off GPS. - pinMode(PIN_GPS_TX, INPUT); - pinMode(PIN_GPS_RX, INPUT); -} - static bool serialHasValidGpsSentence(Stream& serial, uint32_t timeout_ms) { mesh::NmeaSentenceProbe probe; const uint32_t started = millis(); @@ -1218,16 +1210,42 @@ void EnvironmentSensorManager::setPowerSavingEnabled(bool enabled) { } #if ENV_INCLUDE_GPS +void EnvironmentSensorManager::startGpsSerialTransport() { + if (gps_serial_open) return; + Serial1.setPins(PIN_GPS_TX, PIN_GPS_RX); +#ifdef GPS_BAUD_RATE + Serial1.begin(GPS_BAUD_RATE); +#else + Serial1.begin(9600); +#endif + gps_serial_open = true; +} + +void EnvironmentSensorManager::stopGpsSerialTransport() { + // Logical reservation survives GPS-off and bridge handoff. Only this state + // proves that GPS opened the driver: nRF52 Uart::end() can hang unopened. + if (!gps_serial_open) return; + gps_serial_open = false; +#if defined(ESP32_PLATFORM) + // A board-specific provider may already have closed the ESP32 driver while + // switching its receiver rail. Its bool conversion reports that condition. + if (Serial1) +#endif + Serial1.end(); + // UARTE/ESP32 teardown need not restore GPIO configuration. Release BOTH + // pads before receiver power changes; several boards name them from GPS. +#if defined(PIN_GPS_TX) && PIN_GPS_TX >= 0 + pinMode(PIN_GPS_TX, INPUT); +#endif +#if defined(PIN_GPS_RX) && PIN_GPS_RX >= 0 + pinMode(PIN_GPS_RX, INPUT); +#endif +} + #if (defined(ESP32_PLATFORM) || defined(NRF52_PLATFORM)) && !defined(RAK_WISBLOCK_GPS) void EnvironmentSensorManager::finishBasicGpsDiscovery(bool found) { gps_discovery_pending = false; gps_detected = found; -#if defined(ESP32_PLATFORM) - // Relinquish the failed probe's UART while ownership is still known. Leaving - // its idle-high TX installed after the receiver rail is cut can back-power - // the receiver, especially when terminal deep sleep retains digital pads. - if (!found) shutdownGpsTransportForDeepSleep(); -#endif gps_serial_transport = found; if (found) { MESH_DEBUG_PRINTLN("GPS detected"); @@ -1245,6 +1263,7 @@ void EnvironmentSensorManager::finishBasicGpsDiscovery(bool found) { } else { MESH_DEBUG_PRINTLN("No GPS detected"); } + stopGpsSerialTransport(); _location->stop(); gps_active = false; if (found && isGpsTelemetryUserEnabled()) { @@ -1286,13 +1305,7 @@ void EnvironmentSensorManager::initBasicGPS() { gps_active = false; #endif - Serial1.setPins(PIN_GPS_TX, PIN_GPS_RX); - - #ifdef GPS_BAUD_RATE - Serial1.begin(GPS_BAUD_RATE); - #else - Serial1.begin(9600); - #endif + startGpsSerialTransport(); // Try to detect if GPS is physically connected to determine if we should expose the setting _location->begin(); @@ -1335,6 +1348,7 @@ void EnvironmentSensorManager::initBasicGPS() { } else { MESH_DEBUG_PRINTLN("No GPS detected"); } + stopGpsSerialTransport(); _location->stop(); gps_active = false; //Set GPS visibility off until setting is changed #endif @@ -1456,13 +1470,7 @@ void EnvironmentSensorManager::rakGPSInit() { gps_serial_transport_blocked = false; _location = _configured_location; - Serial1.setPins(PIN_GPS_TX, PIN_GPS_RX); - -#ifdef GPS_BAUD_RATE - Serial1.begin(GPS_BAUD_RATE); -#else - Serial1.begin(9600); -#endif + startGpsSerialTransport(); // A RAK-derived board with a real GPS-enable pin must use it. Stock WisBlock // bases have no dedicated GPS switch, so their fallback is shared WB_IO2. @@ -1478,12 +1486,12 @@ void EnvironmentSensorManager::rakGPSInit() { // WB_IO2 is not an independently owned GPS pin. A provider must never // lower it during reset/begin/stop behind the shared-power policy's back. _location->setPinEn(shared_power_rail ? -1 : gps_control_pin); - if (i2cGPSFlag) stopRakGpsUart(); + if (i2cGPSFlag) stopGpsSerialTransport(); } else { MESH_DEBUG_PRINTLN("No GPS found"); gps_active = false; gps_detected = false; - stopRakGpsUart(); + stopGpsSerialTransport(); return; } @@ -1620,12 +1628,7 @@ void EnvironmentSensorManager::start_gps() { } #endif if (gps_serial_transport) { - Serial1.setPins(PIN_GPS_TX, PIN_GPS_RX); -#ifdef GPS_BAUD_RATE - Serial1.begin(GPS_BAUD_RATE); -#else - Serial1.begin(9600); -#endif + startGpsSerialTransport(); } gps_active = true; #ifdef RAK_WISBLOCK_GPS @@ -1652,9 +1655,9 @@ void EnvironmentSensorManager::stop_gps() { _location->setNextWake(); } -#ifdef RAK_WISBLOCK_GPS // End the MCU UART before removing receiver power, never the reverse. - if (serialGPSFlag) stopRakGpsUart(); + stopGpsSerialTransport(); +#ifdef RAK_WISBLOCK_GPS _wisblock_power.setGpsActive(false); #if defined(PIN_GPS_EN) && PIN_GPS_EN >= 0 setRakGpsControl(PIN_GPS_EN, false, false); @@ -1689,18 +1692,10 @@ bool EnvironmentSensorManager::gpsUsesSerialUart(uint8_t uart) const { } void EnvironmentSensorManager::shutdownGpsTransportForDeepSleep() { -#if defined(ESP32_PLATFORM) && ENV_INCLUDE_GPS \ - && defined(PIN_GPS_TX) && PIN_GPS_TX >= 0 \ - && defined(PIN_GPS_RX) && PIN_GPS_RX >= 0 +#if ENV_INCLUDE_GPS // gpsUsesSerialUart stays true while a bridge temporarily owns the UART. // Do not end that owner's transport or touch an I2C/shared peripheral bus. - if (gps_serial_transport && !gps_serial_transport_blocked) { - if (Serial1) Serial1.end(); - // The Arduino pin names are from the receiver's perspective on several - // boards. Release BOTH pads rather than assuming which one drives TX. - pinMode(PIN_GPS_TX, INPUT); - pinMode(PIN_GPS_RX, INPUT); - } + if (!gps_serial_transport_blocked) stopGpsSerialTransport(); #endif } @@ -1755,12 +1750,13 @@ bool EnvironmentSensorManager::setGpsSerialTransportBlocked(uint8_t uart, setGpsTelemetryTransportAvailable(false); gps_serial_transport_blocked = true; #if (defined(ESP32_PLATFORM) || defined(NRF52_PLATFORM)) && !defined(RAK_WISBLOCK_GPS) - if (gps_discovery_pending) _location->stop(); + if (gps_discovery_pending) { + stopGpsSerialTransport(); + _location->stop(); + } #endif if (gps_active) stop_gps(); -#ifndef RAK_WISBLOCK_GPS - Serial1.end(); -#endif + stopGpsSerialTransport(); } else { gps_serial_transport_blocked = false; #if (defined(ESP32_PLATFORM) || defined(NRF52_PLATFORM)) && !defined(RAK_WISBLOCK_GPS) diff --git a/src/helpers/sensors/EnvironmentSensorManager.h b/src/helpers/sensors/EnvironmentSensorManager.h index 34119a22b..08a129354 100644 --- a/src/helpers/sensors/EnvironmentSensorManager.h +++ b/src/helpers/sensors/EnvironmentSensorManager.h @@ -27,6 +27,7 @@ protected: bool gps_active = false; bool gps_serial_transport = false; bool gps_serial_transport_blocked = false; + bool gps_serial_open = false; #if defined(RAK_BOARD) && !defined(RAK_WISMESH_TAG) mesh::gps::WisBlockSharedPowerPolicy _wisblock_power; bool _wisblock_discovery_completed = false; @@ -63,6 +64,8 @@ protected: LocationProvider* _location; LocationProvider* _configured_location; void armGpsPowerSavingCycle(); + void startGpsSerialTransport(); + void stopGpsSerialTransport(); void start_gps(); void stop_gps(); bool telemetryGpsDetected() const override { return gps_detected; } diff --git a/src/helpers/sensors/MicroNMEALocationProvider.h b/src/helpers/sensors/MicroNMEALocationProvider.h index 704f24f0b..69d8e7d2e 100644 --- a/src/helpers/sensors/MicroNMEALocationProvider.h +++ b/src/helpers/sensors/MicroNMEALocationProvider.h @@ -50,6 +50,7 @@ class MicroNMEALocationProvider : public LocationProvider { Stream* _gps_serial; RefCountedDigitalPin* _peripher_power; int8_t _claims = 0; + bool _started = false; int _pin_reset; int _pin_en; unsigned long next_check = 0; @@ -119,6 +120,8 @@ public : void begin() override { _begin_calls++; + if (_started) return; + _started = true; claim(); if (_pin_en != -1) { digitalWrite(_pin_en, GPS_EN_ACTIVE); @@ -143,6 +146,8 @@ public : void stop() override { _stop_calls++; + if (!_started) return; + _started = false; if (_pin_en != -1) { digitalWrite(_pin_en, !GPS_EN_ACTIVE); } diff --git a/test/fixtures/wio_tracker_l2_gps_power.cpp b/test/fixtures/wio_tracker_l2_gps_power.cpp index aee7c4528..243c2b979 100644 --- a/test/fixtures/wio_tracker_l2_gps_power.cpp +++ b/test/fixtures/wio_tracker_l2_gps_power.cpp @@ -22,6 +22,7 @@ class EnvironmentSensorManager : public SensorManager { public: LocationProvider* _location; bool gps_detected=false, gps_active=false, gps_serial_transport=false; + bool gps_serial_open=false; bool gps_serial_transport_blocked=false, gps_discovery_pending=false; bool gps_discovery_preference_known=false; uint32_t gps_discovery_started_at=0; @@ -36,6 +37,8 @@ public: void finishBasicGpsDiscovery(bool found); void serviceBasicGpsDiscovery(); void initBasicGPS(); + void startGpsSerialTransport(); + void stopGpsSerialTransport(); void armGpsPowerSavingCycle(); void start_gps(); void stop_gps(); diff --git a/test/test_companion_hardware_serial_gps.py b/test/test_companion_hardware_serial_gps.py index 186e3ddaa..2e88d25e4 100644 --- a/test/test_companion_hardware_serial_gps.py +++ b/test/test_companion_hardware_serial_gps.py @@ -56,7 +56,7 @@ public: Uart& state() const { return uarts[port]; } void setPins(int receive, int transmit) { state().rx = receive; state().tx = transmit; } void begin(unsigned speed) { state().baud = speed; state().installed = true; } - void end() { state().installed = false; } + void end() { assert(state().installed); state().installed = false; state().fifo.input.clear(); } int available() override { return state().fifo.available(); } int availableForWrite() override { return state().installed ? state().fifo.availableForWrite() : 0; @@ -88,6 +88,7 @@ struct Location { }; struct EnvironmentSensorManager { bool gps_serial_transport_blocked = false, gps_serial_transport = false; + bool gps_serial_open = false; bool gps_detected = false, gps_active = false, powersaving_enabled = false; bool telemetry_user_enabled = false; Location location; @@ -96,6 +97,8 @@ struct EnvironmentSensorManager { void setGpsTelemetryUserEnabled(bool value) { telemetry_user_enabled = value; } void armGpsPowerSavingCycle() {} void initBasicGPS(); + void startGpsSerialTransport(); + void stopGpsSerialTransport(); void start_gps(); void stop_gps(); }; @@ -154,7 +157,7 @@ int main() { sensors.initBasicGPS(); assertCompanionConfiguration(); assert(Serial1.state().baud == 9600); - assert(Serial1.available() == gps_bytes); + assert(Serial1.available() == 0); // Closing GPS discards only its own FIFO. companion_serial.state().fifo.write_capacity = 128; interface_manager.loop(); const std::vector expected = {'>', 1, 0, 0x00}; @@ -197,6 +200,12 @@ public: tx_owner.at(transmit) = int(port); } void begin(unsigned speed) { state().baud = speed; state().installed = true; } + void end() { + assert(state().installed); + state().installed = false; + state().fifo.input.clear(); + if (tx_owner.at(state().tx) == int(port)) tx_owner.at(state().tx) = -1; + } int available() override { return state().fifo.available(); } int availableForWrite() override { return state().installed ? state().fifo.availableForWrite() : 0; @@ -373,6 +382,8 @@ class CompanionHardwareSerialGpsTest(unittest.TestCase): source = source.replace("@GPS_FUNCTIONS@", "\n".join( extract_braced(gps, signature) for signature in ( "void EnvironmentSensorManager::initBasicGPS()", + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()", "void EnvironmentSensorManager::start_gps()", "void EnvironmentSensorManager::stop_gps()"))) scenarios = ( @@ -450,6 +461,8 @@ class CompanionHardwareSerialGpsTest(unittest.TestCase): source = source.replace("@GPS_FUNCTIONS@", "\n".join( extract_braced(gps, signature) for signature in ( "void EnvironmentSensorManager::initBasicGPS()", + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()", "void EnvironmentSensorManager::start_gps()", "void EnvironmentSensorManager::stop_gps()"))) scenarios = [] diff --git a/test/test_esp32_gpio_hold.py b/test/test_esp32_gpio_hold.py index facd07c37..13b264b4e 100644 --- a/test/test_esp32_gpio_hold.py +++ b/test/test_esp32_gpio_hold.py @@ -346,8 +346,10 @@ int main(){ using namespace mesh::esp32; extra_source=(variant/'LoRaFEMControl.cpp').read_text(),files=files) def test_terminal_gps_uart_release_respects_owner_and_leaves_i2c_alone(self): - hook=body((ROOT/'src/helpers/sensors/EnvironmentSensorManager.cpp').read_text(), + source=(ROOT/'src/helpers/sensors/EnvironmentSensorManager.cpp').read_text() + hook=body(source, 'void EnvironmentSensorManager::shutdownGpsTransportForDeepSleep()') + stop=body(source,'void EnvironmentSensorManager::stopGpsSerialTransport()') code=r''' #include #define INPUT 0 @@ -356,27 +358,34 @@ struct Port { bool installed=false;unsigned ends=0;explicit operator bool()const void end(){assert(installed);installed=false;++ends;} } Serial1; void pinMode(int pin,int mode){assert(!Serial1.installed && mode==INPUT);++released_pads;pads[pin].output=false;} class EnvironmentSensorManager {public:bool gps_serial_transport=false,gps_serial_transport_blocked=false; - void shutdownGpsTransportForDeepSleep();}; + bool gps_serial_open=false; + void shutdownGpsTransportForDeepSleep();void stopGpsSerialTransport();}; +#if ENV_INCLUDE_GPS +@STOP@ +#endif @HOOK@ int main(){ EnvironmentSensorManager sensors; // I2C/no-owned GPS: an unrelated serial user is untouched. Serial1.installed=true;sensors.shutdownGpsTransportForDeepSleep();assert(Serial1.installed && !Serial1.ends && !released_pads); sensors.gps_serial_transport=true;sensors.gps_serial_transport_blocked=true; sensors.shutdownGpsTransportForDeepSleep();assert(Serial1.installed && !Serial1.ends && !released_pads); - sensors.gps_serial_transport_blocked=false;sensors.shutdownGpsTransportForDeepSleep(); + sensors.gps_serial_transport_blocked=false;sensors.gps_serial_open=true; + sensors.shutdownGpsTransportForDeepSleep(); #if EXPECT_RELEASE - assert(!Serial1.installed && Serial1.ends==1 && released_pads==2); - sensors.shutdownGpsTransportForDeepSleep();assert(Serial1.ends==1 && released_pads==4); + assert(!Serial1.installed && Serial1.ends==1 && released_pads==EXPECT_PADS); + sensors.shutdownGpsTransportForDeepSleep();assert(Serial1.ends==1 && released_pads==EXPECT_PADS); #else assert(Serial1.installed && !Serial1.ends && !released_pads); #endif } -'''.replace('@HOOK@',hook) +'''.replace('@HOOK@',hook).replace('@STOP@',stop) for definitions in ( - ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=18','PIN_GPS_RX=17','EXPECT_RELEASE=1'), + ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=18','PIN_GPS_RX=17','EXPECT_RELEASE=1','EXPECT_PADS=2'), ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=0','PIN_GPS_TX=18','PIN_GPS_RX=17','EXPECT_RELEASE=0'), - ('NRF52_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=18','PIN_GPS_RX=17','EXPECT_RELEASE=0'), - ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=-1','PIN_GPS_RX=17','EXPECT_RELEASE=0'), + ('NRF52_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=18','PIN_GPS_RX=17','EXPECT_RELEASE=1','EXPECT_PADS=2'), + ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=1','PIN_GPS_TX=-1','PIN_GPS_RX=17','EXPECT_RELEASE=1','EXPECT_PADS=1'), + ('ESP32_PLATFORM=1','ENV_INCLUDE_GPS=1','EXPECT_RELEASE=1','EXPECT_PADS=0'), + ('NRF52_PLATFORM=1','ENV_INCLUDE_GPS=0','PIN_GPS_TX=-1','PIN_GPS_RX=-1','EXPECT_RELEASE=0'), ): self.compile_run(code,defines=definitions) diff --git a/test/test_esp32_gps_discovery.py b/test/test_esp32_gps_discovery.py index d46a442cb..78bce215e 100644 --- a/test/test_esp32_gps_discovery.py +++ b/test/test_esp32_gps_discovery.py @@ -38,8 +38,8 @@ struct SerialMock { bool installed=false; explicit operator bool() const { return installed; } void setPins(int first, int second) { first_pin=first; second_pin=second; } - void begin(unsigned speed) { installed=true; baud=speed; ++begins; gpio_modes[first_pin]=INPUT; gpio_modes[second_pin]=OUTPUT; } - void end() { installed=false; ++ends; } + void begin(unsigned speed) { assert(!installed); installed=true; baud=speed; ++begins; gpio_modes[first_pin]=INPUT; gpio_modes[second_pin]=OUTPUT; } + void end() { assert(installed); installed=false; ++ends; } int available() { ++polls; return bytes; } } Serial1; @@ -49,7 +49,11 @@ struct TestLocation : LocationProvider { bool powered=false; void begin() override { powered=true; ++begins; } void reset() override { ++resets; } - void stop() override { powered=false; ++stops; } + void stop() override { + assert(!Serial1.installed); + assert(gpio_modes[PIN_GPS_TX]==INPUT && gpio_modes[PIN_GPS_RX]==INPUT); + powered=false; ++stops; + } void loop() override { ++loops; } int getPinEn() override { return enable_pin; } bool isEnabled() override { return powered; } @@ -73,6 +77,8 @@ struct EnvironmentSensorManager : SensorManager { bool userEnabled() const { return isGpsTelemetryUserEnabled(); } bool transportAvailable() const { return isGpsTelemetryTransportAvailable(); } void initBasicGPS(); + void startGpsSerialTransport(); + void stopGpsSerialTransport(); void armGpsPowerSavingCycle(); void start_gps(); void stop_gps(); @@ -94,7 +100,8 @@ static void requireImmediateInit(EnvironmentSensorManager& sensors) { const uint32_t before=millis(); sensors.initBasicGPS(); assert(millis()==before && delay_count==0); - assert(Serial1.installed && Serial1.first_pin==PIN_GPS_TX && Serial1.second_pin==PIN_GPS_RX); + assert(Serial1.installed==(sensors.gps_discovery_pending || sensors.gps_active)); + assert(Serial1.first_pin==PIN_GPS_TX && Serial1.second_pin==PIN_GPS_RX); #ifdef GPS_BAUD_RATE assert(Serial1.baud==GPS_BAUD_RATE); #else @@ -115,12 +122,8 @@ static void absent() { now_ms=5100; sensors.loop(); assert(!sensors.gps_discovery_pending && !sensors.gps_detected && !sensors.gps_active); assert(!location.powered && location.stops==1 && !sensors.gpsUsesSerialUart(1)); -#ifdef ESP32_PLATFORM assert(!Serial1.installed && Serial1.ends==1); assert(gpio_modes[PIN_GPS_TX]==INPUT && gpio_modes[PIN_GPS_RX]==INPUT); -#else - assert(Serial1.installed && Serial1.ends==0); // nRF lifecycle is unchanged. -#endif const unsigned polls=Serial1.polls; Serial1.bytes=8; now_ms=6100; sensors.loop(); assert(Serial1.polls==polls && !sensors.gps_detected && location.stops==1); @@ -250,6 +253,43 @@ static void noEnablePin() { assert(!sensors.gps_discovery_pending && !sensors.gps_detected && !location.powered); } +static void offDutyAndShutdown() { + fresh(); TestLocation location; EnvironmentSensorManager sensors(location); + // Shutdown before the first GPS begin must never call the nRF UARTE end. + sensors.shutdownGpsTransportForDeepSleep(); + assert(Serial1.ends==0); + Serial1.bytes=8; sensors.initBasicGPS(); + assert(sensors.gps_detected && sensors.gpsUsesSerialUart(1)); + sensors.setPowerSavingEnabled(true); + assert(sensors.setSettingValue("gps", "1")); + assert(sensors.gps_active && sensors.gps_serial_open && Serial1.installed); + const unsigned ends=Serial1.ends, begins=Serial1.begins; + // Repeated discovery must reuse the driver that GPS already opened. + sensors.startGpsSerialTransport(); + assert(Serial1.begins==begins); + location.stopTimeSync(); location.setNextGPSOff(millis()+1); now_ms+=2; + sensors.loop(); + assert(!sensors.gps_active && !sensors.gps_serial_open && !Serial1.installed); + assert(Serial1.ends==ends+1 && sensors.gpsUsesSerialUart(1)); + const uint32_t wake=location.getNextGPSOn(); assert(wake!=0); + sensors.stop_gps(); sensors.shutdownGpsTransportForDeepSleep(); + sensors.shutdownGpsTransportForDeepSleep(); + assert(Serial1.ends==ends+1); + now_ms=wake; sensors.loop(); + assert(sensors.gps_active && Serial1.installed && Serial1.begins==begins+1); + assert(sensors.setSettingValue("gps", "0")); + assert(!location.powered && !Serial1.installed && Serial1.ends==ends+2); + // A bridge can acquire an already-off GPS. Its driver and pads then belong + // to that bridge even though GPS keeps its logical UART reservation. + assert(sensors.setGpsSerialTransportBlocked(1, true)); + Serial1.begin(115200); + const unsigned bridge_ends=Serial1.ends; + sensors.shutdownGpsTransportForDeepSleep(); sensors.stop_gps(); + sensors.loop(); sensors.initBasicGPS(); + assert(Serial1.installed && Serial1.baud==115200 && Serial1.ends==bridge_ends); + assert(gpio_modes[PIN_GPS_RX]==OUTPUT && !location.powered); +} + int main(int argc, char** argv) { assert(argc==2); if (!strcmp(argv[1], "absent")) absent(); @@ -259,6 +299,7 @@ int main(int argc, char** argv) { else if (!strcmp(argv[1], "wrap")) wrap(); else if (!strcmp(argv[1], "blocked")) blockedRelease(); else if (!strcmp(argv[1], "no_enable")) noEnablePin(); + else if (!strcmp(argv[1], "off_duty_shutdown")) offDutyAndShutdown(); else assert(false && "unknown scenario"); std::printf("PASS %s\n", argv[1]); } @@ -302,12 +343,14 @@ def harness(*, negative=None): " gps_discovery_started_at = millis();\n delay(5000);"), "blocked_poll": ("if (!gps_discovery_pending || gps_serial_transport_blocked) return;", "if (!gps_discovery_pending) return;"), - "cancellation": (" if (gps_discovery_pending) _location->stop();", - " // Negative control: leave the pending receiver powered."), + "cancellation": (" _location->stop();", + " // Negative control: leave the pending receiver powered."), "preference_loss": (" gps_discovery_preference_known = true;", " // Negative control: forget that an explicit preference was loaded."), - "failed_uart": (" if (!found) shutdownGpsTransportForDeepSleep();", - " // Negative control: abandon the failed probe with TX driven."), + "failed_uart": (" stopGpsSerialTransport();\n _location->stop();\n gps_active = false;\n if (found", + " _location->stop();\n gps_active = false;\n if (found"), + "unopened_end": (" if (!gps_serial_open) return;", + " // Negative control: end an unopened UART."), } if negative: before, after = mutations[negative] @@ -358,7 +401,7 @@ class CooperativeGpsDiscoveryTests(unittest.TestCase): self.assertIn("PASS " + case, result.stdout) def test_boot_is_cooperative_and_loop_finishes_cold_gps(self): - self.compile_run(("absent", "late", "latest", "awake", "wrap", "blocked", "no_enable"), + self.compile_run(("absent", "late", "latest", "awake", "wrap", "blocked", "no_enable", "off_duty_shutdown"), defines=("GPS_BAUD_RATE=38400",)) def test_persistent_default_cannot_replace_loaded_user_preference(self): @@ -385,7 +428,7 @@ class CooperativeGpsDiscoveryTests(unittest.TestCase): def test_nrf52_basic_gps_uses_production_guarded_state_and_methods(self): for defines in (("GPS_BAUD_RATE=38400",), ("PERSISTANT_GPS=1",), ("RAK_BOARD=1", "RAK_WISMESH_TAG=1")): - self.compile_run(("absent", "late", "latest", "awake", "wrap", "blocked", "no_enable"), + self.compile_run(("absent", "late", "latest", "awake", "wrap", "blocked", "no_enable", "off_duty_shutdown"), defines=defines, platform="NRF52_PLATFORM") for defines in (("ENV_SKIP_GPS_DETECT=1",), ("ENV_SKIP_GPS_DETECT=1", "PERSISTANT_GPS=1")): @@ -429,6 +472,10 @@ class CooperativeGpsDiscoveryTests(unittest.TestCase): self.compile_run(("absent",)) self.compile_run(("absent",), negative="failed_uart") + def test_nrf52_uart_stop_requires_an_open_driver(self): + self.compile_run(("off_duty_shutdown",), platform="NRF52_PLATFORM", + negative="unopened_end") + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/test/test_gps_upstream_adaptations.py b/test/test_gps_upstream_adaptations.py index 41f38d6ba..ce914e712 100644 --- a/test/test_gps_upstream_adaptations.py +++ b/test/test_gps_upstream_adaptations.py @@ -27,6 +27,7 @@ ARDUINO = r''' #define HIGH 1 #define LOW 0 #define OUTPUT 1 +#define INPUT 0 inline uint32_t now_ms=0; inline uint32_t delay_count=0; inline uint32_t millis(){return now_ms;} @@ -280,6 +281,7 @@ struct SerialMock { unsigned begins=0,polls=0; void setPins(int,int){} void begin(int){++begins;start=millis();} + void end(){} int available(){++polls;return arrival!=UINT32_MAX&&static_cast(millis()-start)>=arrival;} } Serial1; struct Location { @@ -289,12 +291,15 @@ struct Location { struct EnvironmentSensorManager { Location* _location; bool gps_serial_transport_blocked=false,gps_serial_transport=false; + bool gps_serial_open=false; bool gps_detected=false,gps_active=false,user_enabled=false; unsigned transport_resets=0; explicit EnvironmentSensorManager(Location& p):_location(&p){} void resetGpsTelemetryTransportState(){++transport_resets;} void setGpsTelemetryUserEnabled(bool enabled){user_enabled=enabled;} void initBasicGPS(); + void startGpsSerialTransport(); + void stopGpsSerialTransport(); }; @INIT@ int main(){ @@ -401,11 +406,11 @@ class GpsUpstreamAdaptationsTest(unittest.TestCase): if cls.compiler is None: raise unittest.SkipTest("a host C++17 compiler is required") - def compile_run(self, source, defines=(), extra_sources=()): + def compile_run(self, source, defines=(), extra_sources=(), *, arduino=ARDUINO): with tempfile.TemporaryDirectory(prefix="meshcore-gps-upstream-") as directory: path = Path(directory) for name, text in { - "Arduino.h": ARDUINO, "Mesh.h": MESH, "MicroNMEA.h": NMEA, + "Arduino.h": arduino, "Mesh.h": MESH, "MicroNMEA.h": NMEA, "RTClib.h": RTC, "CayenneLPP.h": CAYENNE, "Wire.h": "#pragma once\nclass TwoWire {};\n", }.items(): @@ -450,10 +455,38 @@ int main(){SensorManager sensors;assert(!sensors.requestGpsTelemetryTimeSync(UIN def test_bounded_discovery_fast_cold_absent_bypass_and_uart_owner(self): source = (ROOT / "src/helpers/sensors/EnvironmentSensorManager.cpp").read_text() init = extract_braced(source, "void EnvironmentSensorManager::initBasicGPS()") + init += "\n" + "\n".join(extract_braced(source, signature) for signature in ( + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()")) for defines in ((), ("ENV_SKIP_GPS_DETECT=1",), ("PERSISTANT_GPS=1",)): with self.subTest(defines=defines): self.compile_run(COLD.replace("@INIT@", init), defines) + def test_provider_repeated_begin_stop_preserves_shared_power_owners(self): + arduino = ARDUINO.replace( + "inline void digitalWrite(int,int){}\ninline int digitalRead(int){return HIGH;}", + "inline int pin_values[64]={};\n" + "inline void digitalWrite(int pin,int value){pin_values[pin]=value;}\n" + "inline int digitalRead(int pin){return pin_values[pin];}") + self.compile_run(r''' +#include +#include +void LocationProvider::sendSentence(const char*) {} +int main() { + Stream serial; RefCountedDigitalPin rail(5); rail.begin(); + MicroNMEALocationProvider gps(serial, nullptr, 7, 6, &rail); + gps.stop(); assert(digitalRead(5)==LOW); + for (unsigned cycle=0; cycle<300; ++cycle) { + rail.claim(); // Another peripheral owns the same supply. + gps.begin(); gps.begin(); + assert(digitalRead(5)==HIGH && digitalRead(6)==HIGH); + gps.stop(); gps.stop(); + assert(digitalRead(5)==HIGH && digitalRead(6)==LOW); + rail.release(); assert(digitalRead(5)==LOW); + } +} +''', arduino=arduino) + def run_rak_uart_probe(self, *, negative=False): source = (ROOT / "src/helpers/sensors/EnvironmentSensorManager.cpp").read_text() timeout = re.search(r"#ifndef RAK_UART_GPS_PROBE_TIMEOUT_MS\s*[\s\S]*?#endif", source).group() diff --git a/test/test_nrf52_variant_contracts.py b/test/test_nrf52_variant_contracts.py index 0bc417e80..b02ea7bd2 100644 --- a/test/test_nrf52_variant_contracts.py +++ b/test/test_nrf52_variant_contracts.py @@ -397,7 +397,7 @@ class Nrf52VariantContractsTest(unittest.TestCase): "setGpsTelemetryTransportAvailable(false)", "gps_serial_transport_blocked = true", "stop_gps()", - "Serial1.end()", + "stopGpsSerialTransport()", ): self.assertLess(refusal, block.index(mutation)) self.assertIn("setGpsTelemetryTransportAvailable(false)", sensors) diff --git a/test/test_rak_gps_power.py b/test/test_rak_gps_power.py index 2a89c148e..b2a820d5e 100644 --- a/test/test_rak_gps_power.py +++ b/test/test_rak_gps_power.py @@ -28,8 +28,8 @@ def lifecycle_harness(gps=True, negative=None): "_location->setPinEn(shared_power_rail ? -1 : gps_control_pin);\n" " setGpsTelemetryUserEnabled(true);"), "no_wake_settle": ("if (!was_powered) delay(500);", "// Negative control: use UART before cold receiver settles."), - "no_pad_release": ("pinMode(PIN_GPS_TX, INPUT);\n pinMode(PIN_GPS_RX, INPUT);", - "// Negative control: retain driven serial pads after receiver power-off."), + "no_pad_release": (" pinMode(PIN_GPS_TX, INPUT);", + " // Negative control: retain the serial pad after receiver power-off."), } if negative in mutations: before, after = mutations[negative] @@ -37,9 +37,9 @@ def lifecycle_harness(gps=True, negative=None): source = source.replace(before, after, 1) elif negative == "power_before_uart_end": before = extract_braced(source, "void EnvironmentSensorManager::stop_gps()") - after = before.replace("if (serialGPSFlag) stopRakGpsUart();", "// Negative control: release UART too late.") + after = before.replace("stopGpsSerialTransport();", "// Negative control: release UART too late.") assert after != before, "negative-control UART stop anchor changed" - after = after.replace("applyWisBlockRailPower();", "applyWisBlockRailPower();\n if (serialGPSFlag) stopRakGpsUart();", 1) + after = after.replace("applyWisBlockRailPower();", "applyWisBlockRailPower();\n stopGpsSerialTransport();", 1) source = source.replace(before, after, 1) elif negative is not None: raise ValueError("unknown lifecycle negative control") @@ -48,7 +48,6 @@ def lifecycle_harness(gps=True, negative=None): helpers = "\n".join(extract_braced(source, signature) for signature in ( "static uint8_t rakGpsControlActiveLevel(", "static void setRakGpsControl(", - "static void stopRakGpsUart(", "static bool serialHasValidGpsSentence(", "static bool configureRakUbloxReceiver(", )) if gps else "" @@ -72,6 +71,8 @@ def lifecycle_harness(gps=True, negative=None): "void EnvironmentSensorManager::rakGPSInit()", "bool EnvironmentSensorManager::gpsIsAwake(", "void EnvironmentSensorManager::armGpsPowerSavingCycle()", + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()", "void EnvironmentSensorManager::start_gps()", "void EnvironmentSensorManager::stop_gps()", "bool EnvironmentSensorManager::gpsUsesSerialUart(", @@ -86,6 +87,8 @@ def lifecycle_harness(gps=True, negative=None): "void EnvironmentSensorManager::rakGPSInit()", "bool EnvironmentSensorManager::gpsIsAwake(", "void EnvironmentSensorManager::armGpsPowerSavingCycle()", + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()", "void EnvironmentSensorManager::start_gps()", "void EnvironmentSensorManager::stop_gps()", "void EnvironmentSensorManager::loop()", diff --git a/test/test_wio_tracker_l2_gps_power.py b/test/test_wio_tracker_l2_gps_power.py index 863a529ac..b9018c295 100644 --- a/test/test_wio_tracker_l2_gps_power.py +++ b/test/test_wio_tracker_l2_gps_power.py @@ -58,6 +58,7 @@ public: class HardwareSerial : public Stream { public: bool installed=false; + explicit operator bool() const { return installed; } unsigned begins=0, ends=0; int rx=-1,tx=-1; std::deque incoming; @@ -149,6 +150,8 @@ def harness(mutation=None): "void EnvironmentSensorManager::serviceBasicGpsDiscovery()", "void EnvironmentSensorManager::initBasicGPS()", "void EnvironmentSensorManager::armGpsPowerSavingCycle()", + "void EnvironmentSensorManager::startGpsSerialTransport()", + "void EnvironmentSensorManager::stopGpsSerialTransport()", "void EnvironmentSensorManager::start_gps()", "void EnvironmentSensorManager::stop_gps()", "void EnvironmentSensorManager::loop()",