Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ firmware/

- Follow existing code style - run `trunk fmt` before commits
- Prefer `LOG_DEBUG`, `LOG_INFO`, `LOG_WARN`, `LOG_ERROR` for logging
- **Three logging tiers for diagnostics.** `LOG_TRACE` is the per-packet/per-poll firehose - compiled out by default (`MESHTASTIC_TRACE_LOGGING=1` enables; always on for portduino). Subsystem bring-up detail routes through a per-subsystem gate macro instead, e.g. `LOG_DEBUG_GPS(...)` in `src/gps/GPSLog.h` (`GPS_DEBUG=1` enables; costs no flash when off) - model new subsystem gates on it or on `LOG_MIGRATION` (`src/mesh/WarmNodeStore.h`): `#ifndef` value-default, `#if SYM` value test, `((void)0)` off-branch. Genuine anomalies stay unconditional `LOG_WARN`/`LOG_ERROR`.
- **Format node IDs and packet IDs as `0x%08x` in logs.** This covers `NodeNum`/`PacketId` and the `uint32_t` packet fields `from`, `to`, `id`, `dest`, `source`, `request_id`, and `node_id`. They are 32-bit, so 8 hex digits is exact - `%08x` never truncates or leaves a value ragged. Do **not** use `%x` (variable width) or `%0x` (a no-op typo for `%08x` - the `0` flag does nothing without a width). User-facing display uses `!%08x` (the `!xxxxxxxx` convention), e.g. `Applet::hexifyNodeNum`.
- **Do not zero-pad one-byte values to 8.** `next_hop`, `relay_node`, and the next-hop hint are `uint8_t` last-byte route hints, and `channel` is a one-byte hash/index - log these as `0x%x` (or `%d`). Padding a byte to `0x000000ab` falsely implies a full node number. The same goes for I2C addresses, register values, flags/bitmasks, and error/reason codes: they are not IDs, so leave them `0x%x`.
- Use `assert()` for invariants that should never fail
Expand Down
5 changes: 2 additions & 3 deletions src/GPSStatus.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "NodeDB.h"
#include "Status.h"
#include "configuration.h"
#include "gps/GPSLog.h"
#include <Arduino.h>

namespace meshtastic
Expand Down Expand Up @@ -92,9 +93,7 @@ class GPSStatus : public Status

bool matches(const GPSStatus *newStatus) const
{
#ifdef GPS_DEBUG
LOG_DEBUG("GPSStatus.match() new pos@%x to old pos@%x", newStatus->p.timestamp, p.timestamp);
#endif
LOG_DEBUG_GPS("GPSStatus.match() new pos@%x to old pos@%x", newStatus->p.timestamp, p.timestamp);
return (newStatus->hasLock != hasLock || newStatus->isConnected != isConnected || newStatus->hasTime != hasTime ||
newStatus->isPowerSaving != isPowerSaving || newStatus->p.latitude_i != p.latitude_i ||
newStatus->p.longitude_i != p.longitude_i || newStatus->p.altitude != p.altitude ||
Expand Down
112 changes: 35 additions & 77 deletions src/gps/GPS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#if !MESHTASTIC_EXCLUDE_GPS
#include "Default.h"
#include "GPS.h"
#include "GPSLog.h"
#include "GpioLogic.h"
#include "NodeDB.h"
#include "PowerMon.h"
Expand Down Expand Up @@ -337,7 +338,7 @@ uint8_t GPS::makeCASPacket(uint8_t class_id, uint8_t msg_id, uint8_t payload_siz
}
CASChecksum(UBXscratch, (payload_size + 10));

#if defined(GPS_DEBUG) && defined(DEBUG_PORT)
#if GPS_DEBUG && defined(DEBUG_PORT)
LOG_DEBUG("CAS packet: ");
DEBUG_PORT.hexDump(MESHTASTIC_LOG_LEVEL_DEBUG, UBXscratch, payload_size + 10);
#endif
Expand All @@ -350,26 +351,22 @@ GPS_RESPONSE GPS::getACK(const char *message, uint32_t waitMillis)
uint8_t b;
int bytesRead = 0;
uint32_t startTimeout = millis() + waitMillis;
#ifdef GPS_DEBUG
#if GPS_DEBUG
std::string debugmsg = "";
#endif
while (millis() < startTimeout) {
if (_serial_gps->available()) {
b = _serial_gps->read();

#ifdef GPS_DEBUG
#if GPS_DEBUG
debugmsg += vformat("%c", (b >= 32 && b <= 126) ? b : '.');
#endif
buffer[bytesRead] = b;
bytesRead++;
if ((bytesRead == 767) || (b == '\r')) {
#ifdef GPS_DEBUG
LOG_DEBUG("%s", debugmsg.c_str());
#endif
LOG_DEBUG_GPS("%s", debugmsg.c_str());
if (strnstr((char *)buffer, message, bytesRead) != nullptr) {
#ifdef GPS_DEBUG
LOG_DEBUG("Found: %s", message); // Log the found message
#endif
LOG_DEBUG_GPS("Found: %s", message); // Log the found message
return GNSS_RESPONSE_OK;
} else {
bytesRead = 0;
Expand Down Expand Up @@ -418,17 +415,13 @@ GPS_RESPONSE GPS::getACKCas(uint8_t class_id, uint8_t msg_id, uint32_t waitMilli

// Check for an ACK-ACK for the specified class and message id
if ((msg_cls == 0x05) && (msg_msg_id == 0x01) && payload_cls == class_id && payload_msg == msg_id) {
#ifdef GPS_DEBUG
LOG_INFO("Got ACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
#endif
LOG_DEBUG_GPS("Got ACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
return GNSS_RESPONSE_OK;
}

// Check for an ACK-NACK for the specified class and message id
if ((msg_cls == 0x05) && (msg_msg_id == 0x00) && payload_cls == class_id && payload_msg == msg_id) {
#ifdef GPS_DEBUG
LOG_WARN("Got NACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
#endif
LOG_DEBUG_GPS("Got NACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
return GNSS_RESPONSE_NAK;
}

Expand All @@ -450,7 +443,7 @@ GPS_RESPONSE GPS::getACK(uint8_t class_id, uint8_t msg_id, uint32_t waitMillis)
uint32_t startTime = millis();
const char frame_errors[] = "More than 100 frame errors";
int sCounter = 0;
#ifdef GPS_DEBUG
#if GPS_DEBUG
std::string debugmsg = "";
#endif

Expand All @@ -467,46 +460,37 @@ GPS_RESPONSE GPS::getACK(uint8_t class_id, uint8_t msg_id, uint32_t waitMillis)

while (Throttle::isWithinTimespanMs(startTime, waitMillis)) {
if (ack > 9) {
#ifdef GPS_DEBUG
LOG_INFO("Got ACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
#endif
LOG_DEBUG_GPS("Got ACK for class %02X msg %02X in %dms", class_id, msg_id, millis() - startTime);
return GNSS_RESPONSE_OK; // ACK received
}
if (_serial_gps->available()) {
b = _serial_gps->read();
if (b == frame_errors[sCounter]) {
sCounter++;
if (sCounter == 26) {
#ifdef GPS_DEBUG

LOG_DEBUG("%s", debugmsg.c_str());
#endif
LOG_DEBUG_GPS("%s", debugmsg.c_str());
return GNSS_RESPONSE_FRAME_ERRORS;
}
} else {
sCounter = 0;
}
#ifdef GPS_DEBUG
#if GPS_DEBUG
debugmsg += vformat("%02X", b);
#endif
if (b == buf[ack]) {
ack++;
} else {
if (ack == 3 && b == 0x00) { // UBX-ACK-NAK message
#ifdef GPS_DEBUG
LOG_DEBUG("%s", debugmsg.c_str());
#endif
LOG_DEBUG_GPS("%s", debugmsg.c_str());
LOG_WARN("Got NAK for class %02X msg %02X", class_id, msg_id);
return GNSS_RESPONSE_NAK; // NAK received
}
ack = 0; // Reset the acknowledgement counter
}
}
}
#ifdef GPS_DEBUG
LOG_DEBUG("%s", debugmsg.c_str());
LOG_WARN("No response for class %02X msg %02X", class_id, msg_id);
#endif
LOG_DEBUG_GPS("%s", debugmsg.c_str());
LOG_DEBUG_GPS("No response for class %02X msg %02X", class_id, msg_id);
return GNSS_RESPONSE_NONE; // No response received within timeout
}

Expand Down Expand Up @@ -577,9 +561,7 @@ int GPS::getACK(uint8_t *buffer, uint16_t size, uint8_t requestedClass, uint8_t
ubxFrameCounter = 0;
} else {
// return payload length
#ifdef GPS_DEBUG
LOG_INFO("Got ACK for class %02X msg %02X in %dms", requestedClass, requestedID, millis() - startTime);
#endif
LOG_DEBUG_GPS("Got ACK for class %02X msg %02X in %dms", requestedClass, requestedID, millis() - startTime);
return needRead;
}
break;
Expand Down Expand Up @@ -1234,9 +1216,7 @@ void GPS::writePinEN(bool on)

// Write and log
enablePin->set(on);
#ifdef GPS_DEBUG
LOG_DEBUG("Pin EN %s", on == HIGH ? "HI" : "LOW");
#endif
LOG_DEBUG_GPS("Pin EN %s", on == HIGH ? "HI" : "LOW");
}

// Set the value of the STANDBY pin, if relevant
Expand All @@ -1259,9 +1239,7 @@ void GPS::writePinStandby(bool standby)
_serial_gps->write("$PMTK225,4*2F\r\n");
}

#ifdef GPS_DEBUG
LOG_DEBUG("Pin STANDBY %s", val == HIGH ? "HI" : "LOW");
#endif
LOG_DEBUG_GPS("Pin STANDBY %s", val == HIGH ? "HI" : "LOW");
#endif
}

Expand All @@ -1272,9 +1250,7 @@ void GPS::writePinRFEN(bool on)
bool val = on ? GPS_RF_EN_ACTIVE : !GPS_RF_EN_ACTIVE;
pinMode(PIN_GPS_RF_EN, OUTPUT);
digitalWrite(PIN_GPS_RF_EN, val);
#ifdef GPS_DEBUG
LOG_DEBUG("Pin RF EN %s", val == HIGH ? "HI" : "LOW");
#endif
LOG_DEBUG_GPS("Pin RF EN %s", val == HIGH ? "HI" : "LOW");
#else
(void)on;
#endif
Expand Down Expand Up @@ -1310,9 +1286,7 @@ void GPS::setPowerPMU(bool on)
// t-beam v1.1 GNSS power channel
on ? PMU->enablePowerOutput(XPOWERS_LDO3) : PMU->disablePowerOutput(XPOWERS_LDO3);
}
#ifdef GPS_DEBUG
LOG_DEBUG("PMU %s", on ? "on" : "off");
#endif
LOG_DEBUG_GPS("PMU %s", on ? "on" : "off");
#endif
}

Expand Down Expand Up @@ -1358,9 +1332,7 @@ void GPS::setPowerUBLOX(bool on, uint32_t sleepMs)

// Send the UBX packet
gps->_serial_gps->write(gps->UBXscratch, msglen);
#ifdef GPS_DEBUG
LOG_DEBUG("UBLOX: sleep for %dmS", sleepMs);
#endif
LOG_DEBUG_GPS("UBLOX: sleep for %dmS", sleepMs);
}
}

Expand Down Expand Up @@ -1546,7 +1518,7 @@ int32_t GPS::runOnce()
// 2. Got a lock for the first time, or 3. Got a lock after turning back on
bool gotLoc = lookForLocation();
if (gotLoc) {
#ifdef GPS_DEBUG
#if GPS_DEBUG
if (!hasValidLocation) { // declare that we have location ASAP
LOG_DEBUG("hasValidLocation RISING EDGE");
}
Expand All @@ -1561,9 +1533,7 @@ int32_t GPS::runOnce()
if (holdTime > GPS_FIX_HOLD_MAX_MS)
holdTime = GPS_FIX_HOLD_MAX_MS;
fixHoldEnds = millis() + holdTime;
#ifdef GPS_DEBUG
LOG_DEBUG("Holding for %ums after lock", holdTime);
#endif
LOG_DEBUG_GPS("Holding for %ums after lock", holdTime);
}
}

Expand All @@ -1575,9 +1545,7 @@ int32_t GPS::runOnce()
p = meshtastic_Position_init_default;
hasValidLocation = false;
shouldPublish = true;
#ifdef GPS_DEBUG
LOG_DEBUG("hasValidLocation FALLING EDGE");
#endif
LOG_DEBUG_GPS("hasValidLocation FALLING EDGE");
}
}

Expand All @@ -1597,7 +1565,7 @@ int32_t GPS::runOnce()
down();
}

#ifdef GPS_DEBUG
#if GPS_DEBUG
} else if (fixHoldEnds != 0) {
LOG_DEBUG("Holding for GPS data download: %d ms (numSats=%d)", fixHoldEnds - millis(), p.sats_in_view);
#endif
Expand Down Expand Up @@ -1903,27 +1871,21 @@ GnssModel_t GPS::getProbeResponse(unsigned long timeout, const std::vector<ChipI
// check if we can see our chips
for (const auto &chipInfo : responseMap) {
if (strstr(response.get(), chipInfo.detectionString.c_str()) != nullptr) {
#ifdef GPS_DEBUG
LOG_DEBUG("%s", response.get());
#endif
LOG_DEBUG_GPS("%s", response.get());
LOG_INFO("%s detected", chipInfo.chipName.c_str());
return chipInfo.driver;
}
}
}
if (responseLen >= 2 && response[responseLen - 2] == '\r' && response[responseLen - 1] == '\n') {
#ifdef GPS_DEBUG
LOG_DEBUG("%s", response.get());
#endif
LOG_DEBUG_GPS("%s", response.get());
// Reset the response buffer for the next potential message
responseLen = 0;
response[0] = '\0';
}
}
}
#ifdef GPS_DEBUG
LOG_DEBUG("%s", response.get());
#endif
LOG_DEBUG_GPS("%s", response.get());
return GNSS_MODEL_UNKNOWN; // Return unknown on timeout
}

Expand Down Expand Up @@ -2124,7 +2086,7 @@ bool GPS::lookForLocation()
#ifndef TINYGPS_OPTION_NO_STATISTICS
if (reader.failedChecksum() > lastChecksumFailCount) {
// In a GPS_DEBUG build we want to log all of these. In production, we only care if there are many of them.
#ifndef GPS_DEBUG
#if !GPS_DEBUG
if (reader.failedChecksum() > 4)
#endif
LOG_WARN("%u new GPS checksum failures, total %u", reader.failedChecksum() - lastChecksumFailCount,
Expand All @@ -2141,7 +2103,7 @@ bool GPS::lookForLocation()
if (!hasLock())
return false;

#ifdef GPS_DEBUG
#if GPS_DEBUG
LOG_DEBUG("AGE: LOC=%d FIX=%d DATE=%d TIME=%d", reader.location.age(),
#ifndef TINYGPS_OPTION_NO_CUSTOM_FIELDS
gsafixtype.age(),
Expand Down Expand Up @@ -2172,15 +2134,11 @@ bool GPS::lookForLocation()

// Bail out EARLY to avoid overwriting previous good data (like #857)
if (toDegInt(loc.lat) > 900000000) {
#ifdef GPS_DEBUG
LOG_DEBUG("Bail out EARLY on LAT %i", toDegInt(loc.lat));
#endif
LOG_DEBUG_GPS("Bail out EARLY on LAT %i", toDegInt(loc.lat));
return false;
}
if (toDegInt(loc.lng) > 1800000000) {
#ifdef GPS_DEBUG
LOG_DEBUG("Bail out EARLY on LNG %i", toDegInt(loc.lng));
#endif
LOG_DEBUG_GPS("Bail out EARLY on LNG %i", toDegInt(loc.lng));
return false;
}

Expand Down Expand Up @@ -2265,7 +2223,7 @@ bool GPS::whileActive()
{
unsigned int charsInBuf = 0;
bool isValid = false;
#ifdef GPS_DEBUG
#if GPS_DEBUG
std::string debugmsg = "";
#endif
if (powerState != GPS_ACTIVE) {
Expand All @@ -2282,7 +2240,7 @@ bool GPS::whileActive()
while (_serial_gps->available() > 0) {
int c = _serial_gps->read();
UBXscratch[charsInBuf] = c;
#ifdef GPS_DEBUG
#if GPS_DEBUG
debugmsg += vformat("%c", (c >= 32 && c <= 126) ? c : '.');
#endif
isValid |= reader.encode(c);
Expand All @@ -2295,7 +2253,7 @@ bool GPS::whileActive()
charsInBuf++;
}
}
#ifdef GPS_DEBUG
#if GPS_DEBUG
if (debugmsg != "") {
LOG_DEBUG("%s", debugmsg.c_str());
}
Expand Down
14 changes: 14 additions & 0 deletions src/gps/GPSLog.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#pragma once

#include "DebugConfiguration.h"

// GPS_DEBUG=1 enables verbose GNSS diagnostics (probe/ACK byte dumps, pin states, NMEA ages).
// Costs no flash when off. Genuine LOG_WARN anomalies stay unconditional.
#ifndef GPS_DEBUG
#define GPS_DEBUG 0
#endif
#if GPS_DEBUG
#define LOG_DEBUG_GPS(...) LOG_DEBUG(__VA_ARGS__)
#else
#define LOG_DEBUG_GPS(...) ((void)0)
#endif
Loading
Loading