From 4e35e81fb490977d9089790ed5519fa985e40480 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 16 Sep 2026 13:44:29 -0700 Subject: [PATCH] Treat the GPS enable pin as a pin number, not a truth value gpsResetPin was uint32_t and assigned from a truth test: static uint32_t gpsResetPin = 0; ... if(PIN_GPS_EN){ gpsResetPin = PIN_GPS_EN; } Two defects fall out of that. PIN_GPS_EN is -1 on boards with no enable pin, which passes the truth test and stores 4294967295 in an unsigned pin variable; start_gps() and stop_gps() then call pinMode() and digitalWrite() with it on every gps on/off. And 0 is a legitimate pin number that the same test rejects, so a board wiring the enable line to pin 0 would silently never store it. The first case is harmless today only because the core discards it: cores/nRF5/wiring_digital.c checks 'if (ulPin >= PINS_COUNT) return;' before use. Relying on that is fragile, and it leaves start_gps() and stop_gps() silently doing nothing on RAK4631 with no indication why. Make gpsResetPin signed with -1 meaning 'no enable pin', test the macro with >= 0 so -1 is excluded and 0 is accepted, and guard both toggles so the no-pin case is explicit and logged rather than resting on a bounds check further down. Behaviour is unchanged on every board in tree. rak4631 is the only variant defining PIN_GPS_EN=-1, and it goes from an out-of-range write the core drops to an explicit skip. The gat562 boards define 33 and are unaffected, rak3401 is excluded by the existing ifndef, and no variant uses pin 0 today, so that part is a latent fix. Deliberately not changed: the serial branch does not fall back to the ioPin that woke the receiver, the way the I2C branch does. On RAK4631 that pin is WB_IO2, the shared 3V3_S slot rail, so reusing it would make stop_gps() power down the RTC and every other slot module, and stop_gps() runs at boot on stock firmware. Whether a GPS toggle should drive a shared rail is a design question, not something to settle here. Verified on RAK4631 + RAK12002: 'gps off' and 'gps on' log the new no-enable-pin line, the CLI reply is unchanged, and the clock keeps running across both, confirming the slot rail is untouched. Build checked on RAK_4631_repeater and GAT562_Mesh_Tracker_Pro_companion_radio_usb, the two RAK_WISBLOCK_GPS board families that differ in this code. --- .../sensors/EnvironmentSensorManager.cpp | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/helpers/sensors/EnvironmentSensorManager.cpp b/src/helpers/sensors/EnvironmentSensorManager.cpp index 6f4607751c..7444391c61 100644 --- a/src/helpers/sensors/EnvironmentSensorManager.cpp +++ b/src/helpers/sensors/EnvironmentSensorManager.cpp @@ -165,7 +165,10 @@ static RAK12035_SoilMoisture RAK12035; #endif #ifdef RAK_WISBLOCK_GPS -static uint32_t gpsResetPin = 0; +// The GPS enable pin, or -1 when the board has none. Signed, so the "no pin" +// case is representable: it was uint32_t, which turned PIN_GPS_EN == -1 into +// 4294967295 and left pinMode() to discard it as out of range. +static int gpsResetPin = -1; static bool i2cGPSFlag = false; static bool serialGPSFlag = false; #ifndef TELEM_RAK12500_ADDRESS @@ -865,7 +868,15 @@ bool EnvironmentSensorManager::gpsIsAwake(uint8_t ioPin){ } else if (Serial1.available()) { MESH_DEBUG_PRINTLN("Serial GPS init correctly and is turned on"); #ifdef PIN_GPS_EN - if(PIN_GPS_EN){ + // A range test, not a truth test: PIN_GPS_EN is -1 on boards with no + // enable pin, which passed `if(PIN_GPS_EN)` and stored a bogus pin, and 0 + // is a legitimate pin number that would have failed it. + // + // Note this deliberately does not fall back to ioPin the way the I2C + // branch above does. On RAK4631 the pin that woke the receiver is WB_IO2, + // the shared 3V3_S slot rail, so reusing it here would make stop_gps() + // power down every other module in the slots as well. + if (PIN_GPS_EN >= 0) { gpsResetPin = PIN_GPS_EN; } #endif @@ -887,8 +898,12 @@ void EnvironmentSensorManager::start_gps() { gps_active = true; #ifdef RAK_WISBLOCK_GPS #ifndef RAK_3401 - pinMode(gpsResetPin, OUTPUT); - digitalWrite(gpsResetPin, HIGH); + if (gpsResetPin >= 0) { + pinMode(gpsResetPin, OUTPUT); + digitalWrite(gpsResetPin, HIGH); + } else { + MESH_DEBUG_PRINTLN("GPS: no enable pin configured, power state unchanged"); + } #endif return; #endif @@ -905,8 +920,12 @@ void EnvironmentSensorManager::stop_gps() { gps_active = false; #ifdef RAK_WISBLOCK_GPS #ifndef RAK_3401 // rak3401 shouldn't turn off WB_IO2 as it powers the PA - pinMode(gpsResetPin, OUTPUT); - digitalWrite(gpsResetPin, LOW); + if (gpsResetPin >= 0) { + pinMode(gpsResetPin, OUTPUT); + digitalWrite(gpsResetPin, LOW); + } else { + MESH_DEBUG_PRINTLN("GPS: no enable pin configured, power state unchanged"); + } #endif return; #endif