From 3af5ffe11923eff8da35a14baf505f207fc33d2d Mon Sep 17 00:00:00 2001 From: Naomi Smith Date: Fri, 3 Jul 2026 11:55:06 -0700 Subject: [PATCH] Fix GPS and LEDs on ThinkNode M3 GPS support was implemented incorrectly for the ThinkNode M3. The reset pin was being driven when it should be left floating for this unit. The enable pin also wasn't being picked up by `MicroNMEALocationProvider` because of a mismatch in constant naming conventions. I did a general cleanup of the GPS and ThinkNode M3 bring-up code so that constant names line up and "*_ACTIVE" constants are used consistently vs hardcoding `HIGH`/`LOW`. After making these changes, serial data immediately starts streaming in from the NMEA on boot and GPS detection just works. LED handling was also not quite right for the ThinkNode M3. The LoRa TX LED was being driven high to turn it on when it should actually be driven low. I changed the code to use the `LED_STATE_ON` constant and also added a little code to the shutdown path to properly make sure that all LEDs are turned off. Tested and confirmed working on real hardware. Resolves both #1864 and #2879. --- .../sensors/MicroNMEALocationProvider.h | 32 +++++++++++-------- variants/thinknode_m3/ThinkNodeM3Board.h | 12 +++---- variants/thinknode_m3/platformio.ini | 2 -- variants/thinknode_m3/variant.cpp | 8 +++-- variants/thinknode_m3/variant.h | 9 +++--- 5 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/helpers/sensors/MicroNMEALocationProvider.h b/src/helpers/sensors/MicroNMEALocationProvider.h index eec466d3aa..9d697fed31 100644 --- a/src/helpers/sensors/MicroNMEALocationProvider.h +++ b/src/helpers/sensors/MicroNMEALocationProvider.h @@ -13,8 +13,12 @@ #endif #endif -#ifndef PIN_GPS_EN_ACTIVE - #define PIN_GPS_EN_ACTIVE HIGH +#ifndef GPS_EN_ACTIVE + #ifdef PIN_GPS_EN_ACTIVE + #define GPS_EN_ACTIVE PIN_GPS_EN_ACTIVE + #else + #define GPS_EN_ACTIVE HIGH + #endif #endif #ifndef GPS_RESET @@ -25,11 +29,11 @@ #endif #endif -#ifndef GPS_RESET_FORCE +#ifndef GPS_RESET_ACTIVE #ifdef PIN_GPS_RESET_ACTIVE - #define GPS_RESET_FORCE PIN_GPS_RESET_ACTIVE + #define GPS_RESET_ACTIVE PIN_GPS_RESET_ACTIVE #else - #define GPS_RESET_FORCE LOW + #define GPS_RESET_ACTIVE LOW #endif #endif @@ -52,11 +56,11 @@ public : _gps_serial(&ser), nmea(_nmeaBuffer, sizeof(_nmeaBuffer)), _pin_reset(pin_reset), _pin_en(pin_en), _clock(clock), _peripher_power(peripher_power) { if (_pin_reset != -1) { pinMode(_pin_reset, OUTPUT); - digitalWrite(_pin_reset, GPS_RESET_FORCE); + digitalWrite(_pin_reset, GPS_RESET_ACTIVE); } if (_pin_en != -1) { pinMode(_pin_en, OUTPUT); - digitalWrite(_pin_en, LOW); + digitalWrite(_pin_en, !GPS_EN_ACTIVE); } } @@ -76,27 +80,27 @@ public : void begin() override { claim(); if (_pin_en != -1) { - digitalWrite(_pin_en, PIN_GPS_EN_ACTIVE); + digitalWrite(_pin_en, GPS_EN_ACTIVE); } if (_pin_reset != -1) { - digitalWrite(_pin_reset, !GPS_RESET_FORCE); + digitalWrite(_pin_reset, !GPS_RESET_ACTIVE); } } void reset() override { if (_pin_reset != -1) { - digitalWrite(_pin_reset, GPS_RESET_FORCE); + digitalWrite(_pin_reset, GPS_RESET_ACTIVE); delay(10); - digitalWrite(_pin_reset, !GPS_RESET_FORCE); + digitalWrite(_pin_reset, !GPS_RESET_ACTIVE); } } void stop() override { if (_pin_en != -1) { - digitalWrite(_pin_en, !PIN_GPS_EN_ACTIVE); + digitalWrite(_pin_en, !GPS_EN_ACTIVE); } if (_pin_reset != -1) { - digitalWrite(_pin_reset, GPS_RESET_FORCE); + digitalWrite(_pin_reset, GPS_RESET_ACTIVE); } release(); } @@ -105,7 +109,7 @@ public : // directly read the enable pin if present as gps can be // activated/deactivated outside of here ... if (_pin_en != -1) { - return digitalRead(_pin_en) == PIN_GPS_EN_ACTIVE; + return digitalRead(_pin_en) == GPS_EN_ACTIVE; } else { return true; // no enable so must be active } diff --git a/variants/thinknode_m3/ThinkNodeM3Board.h b/variants/thinknode_m3/ThinkNodeM3Board.h index 1435d31d47..63b141161d 100644 --- a/variants/thinknode_m3/ThinkNodeM3Board.h +++ b/variants/thinknode_m3/ThinkNodeM3Board.h @@ -18,12 +18,12 @@ class ThinkNodeM3Board : public NRF52BoardDCDC { void begin(); uint16_t getBattMilliVolts() override; -#if defined(P_LORA_TX_LED) +#ifdef P_LORA_TX_LED void onBeforeTransmit() override { - digitalWrite(P_LORA_TX_LED, HIGH); // turn TX LED on + digitalWrite(P_LORA_TX_LED, LED_STATE_ON); // turn TX LED on } void onAfterTransmit() override { - digitalWrite(P_LORA_TX_LED, LOW); // turn TX LED off + digitalWrite(P_LORA_TX_LED, !LED_STATE_ON); // turn TX LED off } #endif @@ -44,9 +44,9 @@ class ThinkNodeM3Board : public NRF52BoardDCDC { void powerOff() override { // turn off all leds, sd_power_system_off will not do this for us - #ifdef P_LORA_TX_LED - digitalWrite(P_LORA_TX_LED, LOW); - #endif + digitalWrite(PIN_LED_BLUE, !LED_STATE_ON); + digitalWrite(PIN_LED_GREEN, !LED_STATE_ON); + digitalWrite(PIN_LED_RED, !LED_STATE_ON); // power off board sd_power_system_off(); diff --git a/variants/thinknode_m3/platformio.ini b/variants/thinknode_m3/platformio.ini index 0a3d4eda92..0004f4c739 100644 --- a/variants/thinknode_m3/platformio.ini +++ b/variants/thinknode_m3/platformio.ini @@ -24,7 +24,6 @@ build_flags = ${nrf52_base.build_flags} -D P_LORA_MOSI=46 -D P_LORA_RESET=42 -D P_LORA_TX_LED=PIN_LED_BLUE - -D P_LORA_TX_LED_ON=LOW -D LR11X0_DIO_AS_RF_SWITCH=true -D LR11X0_DIO3_TCXO_VOLTAGE=3.3 -D MESH_DEBUG=1 @@ -105,7 +104,6 @@ build_flags = ${ThinkNode_M3.build_flags} -D BLE_TX_POWER=0 ; -D BLE_DEBUG_LOGGING=1 ; -D MESH_PACKET_LOGGING=1 - -D GPS_NMEA_DEBUG -D OFFLINE_QUEUE_SIZE=256 -D DISPLAY_CLASS=NullDisplayDriver -D PIN_BUZZER=23 diff --git a/variants/thinknode_m3/variant.cpp b/variants/thinknode_m3/variant.cpp index dad0f3f550..b47b83543d 100644 --- a/variants/thinknode_m3/variant.cpp +++ b/variants/thinknode_m3/variant.cpp @@ -80,16 +80,18 @@ void initVariant() digitalWrite(LED_POWER, HIGH); pinMode(PIN_LED_BLUE, OUTPUT); + digitalWrite(PIN_LED_BLUE, !LED_STATE_ON); pinMode(PIN_LED_GREEN, OUTPUT); + digitalWrite(PIN_LED_GREEN, !LED_STATE_ON); pinMode(PIN_LED_RED, OUTPUT); + digitalWrite(PIN_LED_RED, !LED_STATE_ON); pinMode(BUTTON_PIN, INPUT_PULLUP); pinMode(PIN_GPS_POWER, OUTPUT); pinMode(PIN_GPS_EN, OUTPUT); - pinMode(PIN_GPS_RESET, OUTPUT); // Power on gps but in standby - digitalWrite(PIN_GPS_EN, LOW); - digitalWrite(PIN_GPS_POWER, HIGH); + digitalWrite(PIN_GPS_EN, !GPS_EN_ACTIVE); + digitalWrite(PIN_GPS_POWER, GPS_POWER_ACTIVE); } diff --git a/variants/thinknode_m3/variant.h b/variants/thinknode_m3/variant.h index 02ed78a84f..78dfab85d9 100644 --- a/variants/thinknode_m3/variant.h +++ b/variants/thinknode_m3/variant.h @@ -32,7 +32,7 @@ #define EXT_CHRG_DETECT (32) // P1.3 #define EXT_PWR_DETECT (31) // P0.5 -#define PIN_VBAT_READ (5) +#define PIN_VBAT_READ (5) #define AREF_VOLTAGE (2.4f) #define ADC_MULTIPLIER (2.0) //(1.75f) // 2.0 gives more coherent value, 4.2V when charged, needs tweaking @@ -92,18 +92,19 @@ // GPS #define HAS_GPS 1 -#define PIN_GPS_RX (22) +#define PIN_GPS_RX (22) #define PIN_GPS_TX (20) #define PIN_GPS_POWER (14) #define PIN_GPS_EN (21) // STANDBY #define PIN_GPS_RESET (25) // REINIT -#define GPS_RESET_ACTIVE LOW +#define GPS_POWER_ACTIVE HIGH #define GPS_EN_ACTIVE HIGH +#define GPS_RESET (-1) #define GPS_BAUDRATE 9600 //////////////////////////////////////////////////////////////////////////////// // Buzzer #define BUZZER_EN (37) // P1.5 -#define BUZZER_PIN (25) // P0.25 \ No newline at end of file +#define BUZZER_PIN (25) // P0.25