Request and pin cleanup

src/gps/GPS.cpp

@@ -319,6 +319,7 @@ GPS_RESPONSE GPS::getACKCas(uint8_t class_id, uint8_t msg_id, uint32_t waitMilli
     return GNSS_RESPONSE_NONE;
 }
 
+// Fix memory leak in getACK where buffer is allocated but not properly managed
 GPS_RESPONSE GPS::getACK(uint8_t class_id, uint8_t msg_id, uint32_t waitMillis)
 {
     uint8_t b;
@@ -356,7 +357,6 @@ GPS_RESPONSE GPS::getACK(uint8_t class_id, uint8_t msg_id, uint32_t waitMillis)
                 sCounter++;
                 if (sCounter == 26) {
 #ifdef GPS_DEBUG
-
                     LOG_DEBUG(debugmsg.c_str());
 #endif
                     return GNSS_RESPONSE_FRAME_ERRORS;
@@ -798,8 +798,41 @@ bool GPS::setup()
 
 GPS::~GPS()
 {
-    // we really should unregister our sleep observer
+    // Unregister our sleep observer
     notifyDeepSleepObserver.unobserve(&notifyDeepSleep);
+
+    // Clean up GPIO resources
+    if (enablePin) {
+        // First delete any transformer objects
+        if (pinTransformer) {
+            delete pinTransformer;
+            pinTransformer = nullptr;
+        }
+
+        if (unaryTransformer) {
+            delete unaryTransformer;
+            unaryTransformer = nullptr;
+        }
+
+        if (notTransformer) {
+            delete notTransformer;
+            notTransformer = nullptr;
+        }
+
+        if (enableTransformer) {
+            delete enableTransformer;
+            enableTransformer = nullptr;
+        }
+
+        if (hwPin) {
+            delete hwPin;
+            hwPin = nullptr;
+        }
+
+        // Finally delete the enablePin
+        delete enablePin;
+        enablePin = nullptr;
+    }
 }
 
 // Put the GPS hardware into a specified state
@@ -1340,34 +1373,38 @@ GnssModel_t GPS::probe(int serialSpeed)
 
 GnssModel_t GPS::getProbeResponse(unsigned long timeout, const std::vector<ChipInfo> &responseMap)
 {
-    String response = "";
+    uint8_t buffer[768] = {0};
-    unsigned long start = millis();
+    uint8_t b;
-    while (millis() - start < timeout) {
+    int bytesRead = 0;
-        if (_serial_gps->available()) {
+    uint32_t startTimeout = millis() + timeout;
-            response += (char)_serial_gps->read();
 
-            if (response.endsWith(",") || response.endsWith("\r\n")) {
+    while (millis() < startTimeout) {
-#ifdef GPS_DEBUG
+        if (_serial_gps->available()) {
-                LOG_DEBUG(response.c_str());
+            b = _serial_gps->read();
-#endif
+            buffer[bytesRead] = b;
-                // check if we can see our chips
+            bytesRead++;
-                for (const auto &chipInfo : responseMap) {
+
-                    if (strstr(response.c_str(), chipInfo.detectionString.c_str()) != nullptr) {
+            if ((bytesRead == 767) || (b == '\r')) {
-                        LOG_INFO("%s detected", chipInfo.chipName.c_str());
+                // Check the buffer against each possible response in the map
-                        return chipInfo.driver;
+                for (const auto &chip : responseMap) {
+                    if (strnstr((char *)buffer, chip.response, bytesRead) != nullptr) {
+                        LOG_INFO(DETECTED_MESSAGE, chip.name);
+                        // Clear any remaining bytes in the buffer to prevent interfering with future commands
+                        clearBuffer();
+                        return chip.driver;
                     }
                 }
-            }
+
-            if (response.endsWith("\r\n")) {
+                // Reset buffer for next attempt
-                response.trim();
+                memset(buffer, 0, sizeof(buffer));
-                response = ""; // Reset the response string for the next potential message
+                bytesRead = 0;
             }
         }
     }
-#ifdef GPS_DEBUG
+
-    LOG_DEBUG(response.c_str());
+    // Clear the buffer before returning to prevent memory issues
-#endif
+    clearBuffer();
-    return GNSS_MODEL_UNKNOWN; // Return empty string on timeout
+    return GNSS_MODEL_UNKNOWN;
 }
 
 GPS *GPS::createGps()
@@ -1399,23 +1436,27 @@ GPS *GPS::createGps()
         return nullptr;
 
     GPS *new_gps = new GPS;
+
+    new_gps->notifyDeepSleepObserver.observe(&notifyDeepSleep);
     new_gps->rx_gpio = _rx_gpio;
     new_gps->tx_gpio = _tx_gpio;
 
+    // Create a virtual pin and store it in the GPS object to be properly cleaned up in destructor
     GpioVirtPin *virtPin = new GpioVirtPin();
-    new_gps->enablePin = virtPin; // Always at least populate a virtual pin
+    new_gps->enablePin = virtPin;
+
     if (_en_gpio) {
         GpioPin *p = new GpioHwPin(_en_gpio);
+        GpioTransformer *transformer;
 
         if (!GPS_EN_ACTIVE) { // Need to invert the pin before hardware
-            new GpioNotTransformer(
+            transformer = new GpioNotTransformer(virtPin, p);
-                virtPin,
-                p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio
         } else {
-            new GpioUnaryTransformer(
+            transformer = new GpioUnaryTransformer(virtPin, p);
-                virtPin,
-                p); // We just leave this created object on the heap so it can stay watching virtPin and driving en_gpio
         }
+
+        // Store the transformer for cleanup in destructor
+        new_gps->pinTransformer = transformer;
     }
 
 #ifdef PIN_GPS_PPS
@@ -1433,8 +1474,8 @@ GPS *GPS::createGps()
 // when fixed upstream, can be un-disabled to enable 3D FixType and PDOP
 #ifndef TINYGPS_OPTION_NO_CUSTOM_FIELDS
     // see NMEAGPS.h
-    gsafixtype.begin(reader, NMEA_MSG_GXGSA, 2);
+    new_gps->gsafixtype.begin(new_gps->reader, NMEA_MSG_GXGSA, 2);
-    gsapdop.begin(reader, NMEA_MSG_GXGSA, 15);
+    new_gps->gsapdop.begin(new_gps->reader, NMEA_MSG_GXGSA, 15);
     LOG_DEBUG("Use " NMEA_MSG_GXGSA " for 3DFIX and PDOP");
 #endif
 
@@ -1466,6 +1507,10 @@ GPS *GPS::createGps()
         _serial_gps->begin(GPS_BAUDRATE);
 #endif
     }
+
+    // Setup sleep observer to ensure cleanup on sleep
+    new_gps->notifyDeepSleepObserver.observe(&notifyDeepSleep);
+
     return new_gps;
 }
 
@@ -1746,23 +1791,30 @@ bool GPS::whileActive()
 }
 void GPS::enable()
 {
-    // Clear the old scheduling info (reset the lock-time prediction)
+    LOG_INFO("GPS - enabling");
-    scheduling.reset();
+    // ensure we are marked as enabled in config
-
+    config.position.gps_mode = meshtastic_Config_PositionConfig_GpsMode_ENABLED;
-    enabled = true;
+    // Start our thread
-    setInterval(GPS_THREAD_INTERVAL);
+    setIntervalFromNow(0); // Run right away
-
+    shouldPublish = true;
-    scheduling.informSearching();
-    setPowerState(GPS_ACTIVE);
 }
 
 int32_t GPS::disable()
 {
-    enabled = false;
+    LOG_INFO("GPS - disabling");
-    setInterval(INT32_MAX);
+    config.position.gps_mode = meshtastic_Config_PositionConfig_GpsMode_DISABLED;
+
+    // Clear any resources
     setPowerState(GPS_OFF);
 
-    return INT32_MAX;
+    // Don't really terminate our thread (because it might be holding locks etc...), just let it sleep forever
+    // enableThread(false);
+    setInterval(INT32_MAX);
+
+    // Ensure no memory leak during disable
+    clearBuffer();
+
+    return 0;
 }
 
 void GPS::toggleGpsMode()

src/gps/GPS.h

@@ -70,6 +70,18 @@ class GPS : private concurrency::OSThread
      */
     GpioVirtPin *enablePin = NULL;
 
+    // Store pointers to transformers for cleanup
+    GpioTransformer *pinTransformer = NULL;
+
+    // Added pointers to manage GPIO transformers to fix memory leak
+    GpioUnaryTransformer *gpioUnaryTransformer = NULL;
+    GpioNotTransformer *gpioNotTransformer = NULL;
+
+    // Add pointers to the transformer objects so we can delete them in the destructor
+    GpioUnaryTransformer *unaryTransformer = NULL;
+    GpioNotTransformer *notTransformer = NULL;
+    GpioPin *hwPin = NULL;
+
     virtual ~GPS();
 
     /** We will notify this observable anytime GPS state has changed meaningfully */
@@ -119,6 +131,13 @@ class GPS : private concurrency::OSThread
   private:
     GPS() : concurrency::OSThread("GPS") {}
 
+    // Pointers to GPIO transformers created in createGps(), to be cleaned up in destructor
+    GpioTransformer *gpioTransformer = nullptr;
+    GpioTransformer *enablePinTransformer = nullptr;
+
+    // Store pointers to the GPIO transformers to prevent memory leaks
+    GpioTransformer *enableTransformer = nullptr;
+
     /// Record that we have a GPS
     void setConnected();
 

src/modules/Telemetry/DeviceTelemetry.cpp

@@ -74,14 +74,24 @@ meshtastic_MeshPacket *DeviceTelemetryModule::allocReply()
             LOG_ERROR("Error decoding DeviceTelemetry module!");
             return NULL;
         }
+
+        // Store our reply packet before we release currentRequest
+        meshtastic_MeshPacket *reply = NULL;
+
         // Check for a request for device metrics
         if (decoded->which_variant == meshtastic_Telemetry_device_metrics_tag) {
             LOG_INFO("Device telemetry reply to request");
-            return allocDataProtobuf(getDeviceTelemetry());
+            reply = allocDataProtobuf(getDeviceTelemetry());
         } else if (decoded->which_variant == meshtastic_Telemetry_local_stats_tag) {
             LOG_INFO("Device telemetry reply w/ LocalStats to request");
-            return allocDataProtobuf(getLocalStatsTelemetry());
+            reply = allocDataProtobuf(getLocalStatsTelemetry());
         }
+
+        // We no longer need the request packet - free it
+        packetPool.release((meshtastic_MeshPacket *)currentRequest);
+        currentRequest = NULL;
+
+        return reply;
     }
     return NULL;
 }
@@ -173,17 +183,30 @@ bool DeviceTelemetryModule::sendTelemetry(NodeNum dest, bool phoneOnly)
              telemetry.variant.device_metrics.uptime_seconds);
 
     meshtastic_MeshPacket *p = allocDataProtobuf(telemetry);
+    if (!p) {
+        LOG_ERROR("Failed to allocate packet for telemetry");
+        return false;
+    }
+
     p->to = dest;
     p->decoded.want_response = false;
     p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;
 
     nodeDB->updateTelemetry(nodeDB->getNodeNum(), telemetry, RX_SRC_LOCAL);
+    bool success = false;
+
     if (phoneOnly) {
         LOG_INFO("Send packet to phone");
-        service->sendToPhone(p);
+        success = service->sendToPhone(p);
     } else {
         LOG_INFO("Send packet to mesh");
-        service->sendToMesh(p, RX_SRC_LOCAL, true);
+        success = service->sendToMesh(p, RX_SRC_LOCAL, true);
     }
-    return true;
+
+    // If sending failed, free the packet to prevent memory leak
+    if (!success) {
+        packetPool.release(p);
+    }
+
+    return success;
 }