fix(fw): lua telemetry handling (#7773)

radio/src/lua/api_general.cpp

@@ -1206,13 +1206,19 @@ static int luaCrossfireTelemetryPush(lua_State* L)
 
   if (lua_gettop(L) == 0) {
     lua_pushboolean(L, outputTelemetryBuffer.isAvailable());
-  } else if (lua_gettop(L) > TELEMETRY_OUTPUT_BUFFER_SIZE) {
-    lua_pushboolean(L, false);
-    return 1;
   } else if (outputTelemetryBuffer.isAvailable()) {
     uint8_t command = luaL_checkinteger(L, 1);
     luaL_checktype(L, 2, LUA_TTABLE);
-    uint8_t length = luaL_len(L, 2);
+    lua_Integer payloadLen = luaL_len(L, 2);
+
+    // ADDRESS + LENGTH + COMMAND + payload + CRC (2 bytes for COMMAND_ID)
+    lua_Integer frameLen = 3 + payloadLen + (command == COMMAND_ID ? 2 : 1);
+    if (payloadLen < 0 || frameLen > TELEMETRY_OUTPUT_BUFFER_SIZE) {
+      lua_pushboolean(L, false);
+      return 1;
+    }
+
+    uint8_t length = (uint8_t)payloadLen;
 
     outputTelemetryBuffer.pushByte(MODULE_ADDRESS);
 
@@ -1232,6 +1238,7 @@ static int luaCrossfireTelemetryPush(lua_State* L)
     for (int i = 0; i < length; i++) {
       lua_rawgeti(L, 2, i + 1);
       outputTelemetryBuffer.pushByte(luaL_checkinteger(L, -1));
+      lua_pop(L, 1);
     }
 
     // CRC
@@ -1324,20 +1331,18 @@ static int luaGhostTelemetryPush(lua_State * L)
   if (lua_gettop(L) == 0) {
     lua_pushboolean(L, outputTelemetryBuffer.isAvailable());
   }
-  else if (lua_gettop(L) > TELEMETRY_OUTPUT_BUFFER_SIZE ) {
-    lua_pushboolean(L, false);
-    return 1;
-  }
   else if (outputTelemetryBuffer.isAvailable()) {
     uint8_t type = luaL_checkinteger(L, 1);
     luaL_checktype(L, 2, LUA_TTABLE);
-    uint8_t length = luaL_len(L, 2);              // payload length
+    lua_Integer payloadLen = luaL_len(L, 2);      // payload length
 
-    if( length > 10 ) {                           // max 10B payload
+    if (payloadLen < 0 || payloadLen > 10) {      // max 10B payload
       lua_pushboolean(L, false);
       return 1;
     }
 
+    uint8_t length = (uint8_t)payloadLen;
+
     // Ghost frames are fixed 14B:
     // address(1B) + len (1B) + type(1B) + payload(10B) + crc(1B)
     // -> address + len up-front are inserted later
@@ -1346,6 +1351,7 @@ static int luaGhostTelemetryPush(lua_State * L)
     for (; i < length; i++) {                     // data, max 10B
       lua_rawgeti(L, 2, i + 1);
       outputTelemetryBuffer.pushByte(luaL_checkinteger(L, -1));
+      lua_pop(L, 1);
     }
     for (; i < 10; i++) {                         // fill zeroes to frame size
       outputTelemetryBuffer.pushByte(0);

radio/src/lua/lua_widget.cpp

@@ -617,8 +617,7 @@ LuaScriptManager::~LuaScriptManager()
 {
   luaL_unref(lsWidgets, LUA_REGISTRYINDEX, luaScriptContextRef);
   if (luaInputTelemetryFifo != nullptr) {
-    deregisterTelemetryQueue(luaInputTelemetryFifo);
-    delete luaInputTelemetryFifo;
+    destroyTelemetryQueue(luaInputTelemetryFifo);
     luaInputTelemetryFifo = nullptr;
   }
 }

radio/src/tasks.cpp

@@ -168,6 +168,10 @@ void tasksStart()
 {
   mutex_create(&audioMutex);
 
+#if defined(LUA)
+  telemetryQueuesInit();
+#endif
+
 #if defined(CLI) && !defined(SIMU)
   cliStart();
 #endif

radio/src/telemetry/telemetry.cpp

@@ -22,6 +22,7 @@
 #include "edgetx.h"
 #include "multi.h"
 #include "os/async.h"
+#include "os/task.h"
 #include "os/timer.h"
 #include "pulses/afhds3.h"
 #include "pulses/flysky.h"
@@ -459,15 +460,30 @@ TelemetryQueue* luaInputTelemetryFifo = nullptr;
 #if defined(COLORLCD)
 std::list<TelemetryQueue*> telemetryQueues;
 
+// Scripts register / destroy their queue from the menus task, while frames are
+// pushed into them from the timer task (telemetry RX runs there, at a higher
+// priority). Both the list and the queue lifetime need to be serialised.
+static mutex_handle_t telemetryQueueMutex;
+
+void telemetryQueuesInit()
+{
+  mutex_create(&telemetryQueueMutex);
+}
+
 void registerTelemetryQueue(TelemetryQueue* queue)
 {
+  MutexLock lock = MutexLock::MakeInstance(&telemetryQueueMutex);
   telemetryQueues.emplace_back(queue);
 }
 
-void deregisterTelemetryQueue(TelemetryQueue* queue)
+void destroyTelemetryQueue(TelemetryQueue* queue)
 {
+  MutexLock lock = MutexLock::MakeInstance(&telemetryQueueMutex);
   telemetryQueues.remove(queue);
+  delete queue;
 }
+#else
+void telemetryQueuesInit() {}
 #endif
 
 static void pushDataToQueue(TelemetryQueue* queue, uint8_t* data, int length)
@@ -482,8 +498,11 @@ static void pushDataToQueue(TelemetryQueue* queue, uint8_t* data, int length)
 void pushTelemetryDataToQueues(uint8_t* data, int length)
 {
 #if defined(COLORLCD)
-  for (auto it = telemetryQueues.cbegin(); it != telemetryQueues.cend(); ++it)
-    pushDataToQueue(*it, data, length);
+  {
+    MutexLock lock = MutexLock::MakeInstance(&telemetryQueueMutex);
+    for (auto it = telemetryQueues.cbegin(); it != telemetryQueues.cend(); ++it)
+      pushDataToQueue(*it, data, length);
+  }
 #endif
   pushDataToQueue(luaInputTelemetryFifo, data, length);
 }

radio/src/telemetry/telemetry.h

@@ -237,8 +237,17 @@ extern OutputTelemetryBuffer outputTelemetryBuffer __DMA_NO_CACHE;
 #define LUA_TELEMETRY_INPUT_FIFO_SIZE  256
 typedef Fifo<uint8_t, LUA_TELEMETRY_INPUT_FIFO_SIZE> TelemetryQueue;
 extern TelemetryQueue* luaInputTelemetryFifo;
+
+// Create the lock protecting the queue list. Must be called before the
+// scheduler is started.
+void telemetryQueuesInit();
+
 void registerTelemetryQueue(TelemetryQueue*);
-void deregisterTelemetryQueue(TelemetryQueue*);
+
+// Unregister and delete in one go: the producer runs in the timer task and
+// would otherwise be able to write into a queue that has just been freed.
+void destroyTelemetryQueue(TelemetryQueue*);
+
 void pushTelemetryDataToQueues(uint8_t* data, int length);
 #endif