fix(logs): open log files from the UI task (#7513)

radio/src/logs.cpp

@@ -37,6 +37,24 @@ static tmr10ms_t lastLogTime = 0;
 
 static timer_handle_t loggingTimer = TIMER_INITIALIZER;
 
+// Last error reported to the user, used only to avoid repeating the popup.
+// Written from both the UI task (logsHandle) and the higher-priority timer
+// task (logsWrite); the two never run concurrently on the target (the UI task
+// cannot preempt the timer task) and a pointer store is atomic, so no lock is
+// needed.
+static const char* error_displayed = nullptr;
+
+static void logsWrite();
+const char* logsOpen();
+
+static void displayLogError(const char* err)
+{
+  if (err != error_displayed) {
+    error_displayed = err;
+    POPUP_WARNING_ON_UI_TASK(err, nullptr);
+  }
+}
+
 static void loggingTimerCb(timer_handle_t* timer)
 {
   (void)timer;
@@ -63,21 +81,53 @@ bool loggingTimerIsRunning()
   return timer_is_active(&loggingTimer);
 }
 
-void initLoggingTimer()
-{  // called cyclically by main.cpp:perMain()
+void logsHandle()
+{  // called cyclically by main.cpp:perMain(), i.e. from the UI task
   static uint8_t logDelay100msOld = 0;
 
-  if(!timer_is_active(&loggingTimer)) {                         // log Timer not running
-    if(isFunctionActive(FUNCTION_LOGS) && logDelay100ms > 0) {  // if SF Logging is active and log rate is valid
-      loggingTimerStart(logDelay100ms * 100);                   // start log timer
-    }  
-  } else {                                                      // log timer is already running
-    if(logDelay100msOld != logDelay100ms) {                     // if log rate was changed
-      logDelay100msOld = logDelay100ms;                         // memorize new log rate
-      if(logDelay100ms > 0) {
-        timer_set_period(&loggingTimer, logDelay100ms * 100);
-      }
+  bool logsActive = sdMounted() && isFunctionActive(FUNCTION_LOGS) &&
+                    logDelay100ms > 0 && !usbPlugged();
+
+  if (!logsActive) {
+    if (timer_is_active(&loggingTimer)) {
+      loggingTimerStop();
+      logsClose();
+    }
+    error_displayed = nullptr;
+    return;
+  }
+
+  // An error was reported (by logsWrite or a previous open): release the file
+  // and stop the no-op timer. A full SD card is the one recoverable case -
+  // keep checking cheaply (sdIsFull() needs no directory scan) and resume once
+  // space is freed. Any other error is treated as fatal: give up until the
+  // user re-enables logging, rather than rescanning the LOGS directory on
+  // every cycle for an error that will not fix itself.
+  if (error_displayed) {
+    if (timer_is_active(&loggingTimer)) loggingTimerStop();
+    logsClose();
+    if (error_displayed != STR_SDCARD_FULL_EXT) return;
+  }
+
+  // Open the log file here, in the UI task. f_open() may have to scan a large
+  // LOGS directory and can block for a long time; doing it from the timer task
+  // (logsWrite) would starve telemetry polling, which runs on the same task,
+  // and cause a brief "telemetry lost". See issue #7513.
+  if (!g_oLogFile.obj.fs) {
+    const char* result = sdIsFull() ? STR_SDCARD_FULL_EXT : logsOpen();
+    if (result) {
+      displayLogError(result);  // deduplicated: no repeated popup
+      return;  // keep the timer stopped until we have a valid file
     }
+    error_displayed = nullptr;  // (re)opened successfully
+  }
+
+  if (!timer_is_active(&loggingTimer)) {                        // log timer not running
+    logDelay100msOld = logDelay100ms;
+    loggingTimerStart(logDelay100ms * 100);                    // start log timer
+  } else if (logDelay100msOld != logDelay100ms) {              // log rate changed
+    logDelay100msOld = logDelay100ms;                          // memorize new log rate
+    timer_set_period(&loggingTimer, logDelay100ms * 100);
   }
 }
 
@@ -225,15 +275,19 @@ uint32_t getLogicalSwitchesStates(uint8_t first)
   return result;
 }
 
-void logsWrite()
+static void logsWrite()
 {
-  static const char * error_displayed = nullptr;
+  // Called from the logging timer callback (timer task). The log file is
+  // opened and closed by logsHandle() in the UI task, so this function only
+  // ever appends to an already-open file and never blocks on f_open().
 
-  if (!sdMounted()) {
+  // stop writing once an error has been reported; logsHandle() clears
+  // error_displayed when logging is toggled off, which resumes logging.
+  if (!sdMounted() || !g_oLogFile.obj.fs || error_displayed) {
     return;
   }
 
-  if (isFunctionActive(FUNCTION_LOGS) && logDelay100ms > 0 && !usbPlugged()) {
+  {
     #if defined(SIMU) || !defined(RTCLOCK)
     tmr10ms_t tmr10ms = get_tmr10ms();                                        // tmr10ms works in 10ms increments
     if (lastLogTime == 0 || (tmr10ms_t)(tmr10ms - lastLogTime) >= (tmr10ms_t)(logDelay100ms*10)-1) {
@@ -242,27 +296,10 @@ void logsWrite()
     {
     #endif
 
-      bool sdCardFull = sdIsFull();
-
-      // check if file needs to be opened
-      if (!g_oLogFile.obj.fs) {
-        const char *result = sdCardFull ? STR_SDCARD_FULL_EXT : logsOpen();
-
-        // SD card is full or file open failed
-        if (result) {
-          if (result != error_displayed) {
-            error_displayed = result;
-            POPUP_WARNING_ON_UI_TASK(result, nullptr);
-          }
-          return;
-        }
-      }
-
-      // check at every write cycle
-      if (sdCardFull) {
-        logsClose();  // timer is still running and code above will try to
-                      // open the file again but will fail with error
-                      // which will trigger the warning popup
+      // SD card became full: report and stop writing. The file is left open
+      // and closed later by logsHandle() from the UI task.
+      if (sdIsFull()) {
+        displayLogError(STR_SDCARD_FULL_EXT);
         return;
       }
 
@@ -355,19 +392,11 @@ void logsWrite()
       div_t qr = div(g_vbat100mV, 10);
       int result = f_printf(&g_oLogFile, "%d.%d\n", abs(qr.quot), abs(qr.rem));
 
-      if (result<0 && !error_displayed) {
-        error_displayed = STR_SDCARD_ERROR;
-        POPUP_WARNING_ON_UI_TASK(STR_SDCARD_ERROR, nullptr);
-        logsClose();
+      // Write error: report it and stop writing. The file is left open and
+      // closed later by logsHandle() from the UI task.
+      if (result < 0) {
+        displayLogError(STR_SDCARD_ERROR);
       }
     }
   }
-  else {
-    error_displayed = nullptr;
-    logsClose();
-
-    #if !defined(SIMU)
-    loggingTimerStop();
-    #endif
-  }
 }

radio/src/main.cpp

@@ -513,9 +513,6 @@ void guiMain(event_t evt)
 }
 #endif
 
-// from logs.cpp
-void initLoggingTimer();
-
 void perMain()
 {
   DEBUG_TIMER_START(debugTimerPerMain1);
@@ -524,7 +521,7 @@ void perMain()
 
   if (!usbPlugged() || (getSelectedUsbMode() == USB_UNSELECTED_MODE)) {
     checkStorageUpdate();
-    initLoggingTimer();  // initialize software timer for logging
+    logsHandle();  // open/close log file and drive the logging timer
   }
 
   handleUsbConnection();

radio/src/sdcard.h

@@ -119,7 +119,7 @@ const char YAMLFILE_CHECKSUM_TAG_NAME[] = "checksum";
 extern uint8_t logDelay100ms;
 void logsInit();
 void logsClose();
-void logsWrite();
+void logsHandle();
 
 void sdInit();
 void sdMount();