diff --git a/radio/src/gui/colorlcd/model_mixer_scripts.cpp b/radio/src/gui/colorlcd/model_mixer_scripts.cpp index f3e6929fd..462d0335a 100644 --- a/radio/src/gui/colorlcd/model_mixer_scripts.cpp +++ b/radio/src/gui/colorlcd/model_mixer_scripts.cpp @@ -44,12 +44,12 @@ class ScriptEditWindow : public Page { } protected: - uint8_t idx; - bool update = false; + const uint8_t idx; + bool update = false; void checkEvents() override { - if (update) { + if ((update) && (luaState == INTERPRETER_RUNNING)) { TRACE("rebuilding ScriptEditWindow..."); rebuildBody(&body); update = false; @@ -75,20 +75,25 @@ class ScriptEditWindow : public Page { FormGridLayout grid; grid.spacer(PAGE_PADDING); - ScriptData* sd = &(g_model.scriptsData[idx]); + // the general pattern seems to be using capture-by-value for the closures: so need to copy the pointers, not the objects + ScriptData* const sd = &(g_model.scriptsData[idx]); + ScriptInputsOutputs* const sio = &(scriptInputsOutputs[idx]); + bool* const updatePtr = &update; - // Filename new StaticText(window, grid.getLabelSlot(), STR_SCRIPT, 0, COLOR_THEME_PRIMARY1); auto fc = new FileChoice( window, grid.getFieldSlot(), SCRIPTS_MIXES_PATH, SCRIPTS_EXT, LEN_SCRIPT_FILENAME, - [=]() { return std::string(sd->file, LEN_SCRIPT_FILENAME); }, + [=]() { return stringFromNtString(sd->file); }, [=](std::string newValue) { - strncpy(sd->file, newValue.c_str(), LEN_SCRIPT_FILENAME); - if (newValue.empty()) { memset((void*)sd, 0, sizeof(ScriptData)); } + clearStruct(*sd); + clearStruct(*sio); + if (!newValue.empty()) { + copyToUnTerminated(sd->file, newValue); + } storageDirty(EE_MODEL); - LUA_LOAD_MODEL_SCRIPT(idx); // async reload... - update = true; + LUA_LOAD_MODEL_SCRIPT(idx); // async reload ... + *updatePtr = true; // TODO: sets update==true and afterwards calls rebuildBody() via checkEvents() }, true); grid.nextLine(); @@ -97,18 +102,15 @@ class ScriptEditWindow : public Page { new ModelTextEdit(window, grid.getFieldSlot(), sd->name, sizeof(sd->name)); grid.nextLine(); - // scriptInputsOutputs - ScriptInputsOutputs& sio = scriptInputsOutputs[idx]; - - if (sio.inputsCount > 0) { + if (sio->inputsCount > 0) { new Subtitle(window, grid.getLineSlot(), STR_INPUTS, 0, COLOR_THEME_PRIMARY1); grid.nextLine(); auto gInputs = new FormGroup(window, grid.getFieldSlot(), FORM_BORDER_FOCUS_ONLY | PAINT_CHILDREN_FIRST); GridLayout inputsGrid(gInputs); - for (int i = 0; i < sio.inputsCount; i++) { - ScriptInput& si = sio.inputs[i]; + for (int i = 0; i < sio->inputsCount; i++) { + ScriptInput& si = sio->inputs[i]; new StaticText(window, grid.getLabelSlot(true), si.name, 0, COLOR_THEME_PRIMARY1); grid.nextLine(); if (si.type == INPUT_TYPE_VALUE) { @@ -124,7 +126,7 @@ class ScriptEditWindow : public Page { gInputs->setHeight(inputsGrid.getWindowHeight()); } - if (sio.outputsCount > 0) { + if (sio->outputsCount > 0) { new Subtitle(window, grid.getLabelSlot(), STR_OUTPUTS, 0, COLOR_THEME_PRIMARY1); grid.nextLine(); @@ -133,8 +135,8 @@ class ScriptEditWindow : public Page { FORM_BORDER_FOCUS_ONLY | PAINT_CHILDREN_FIRST); FormGridLayout outputsGrid(gOutputs->width()); - for (int i = 0; i < sio.outputsCount; i++) { - ScriptOutput* so = &(sio.outputs[i]); + for (int i = 0; i < sio->outputsCount; i++) { + ScriptOutput* so = &(sio->outputs[i]); new DynamicText(gOutputs, outputsGrid.getLabelSlot(), [=]() { char s[16]; getSourceString(s, MIXSRC_FIRST_LUA + (idx * MAX_SCRIPT_OUTPUTS) + i); @@ -161,8 +163,6 @@ class ScriptEditWindow : public Page { } }; -constexpr char SCRIPT_STATUS_ERROR[] = "(error)"; - class ScriptLineButton : public Button { public: @@ -193,12 +193,18 @@ class ScriptLineButton : public Button y = FIELD_PADDING_TOP; textColor |= RIGHT; + // TODO: runtimeData->instructions has no value switch (runtimeData->state) { case SCRIPT_SYNTAX_ERROR: - dc->drawSizedText(x, y, SCRIPT_STATUS_ERROR, sizeof(SCRIPT_STATUS_ERROR), textColor); + dc->drawSizedText(x, y, STR_SCRIPT_ERROR, strlen(STR_SCRIPT_ERROR), textColor); break; - default: - dc->drawNumber(x, y, runtimeData->instructions, textColor, 0, nullptr, "%"); + case SCRIPT_NOFILE: + dc->drawSizedText(x, y, STR_NEEDS_FILE, strlen(STR_NEEDS_FILE), textColor); + break; + case SCRIPT_OK: + dc->drawSizedText(x, y, "-", 1, textColor); + break; + default: break; } } @@ -238,27 +244,29 @@ void ModelMixerScriptsPage::build(FormWindow * window, int8_t focusIdx) for (int8_t idx = 0; idx < MAX_SCRIPTS; idx++) { ScriptInternalData* runtimeData = nullptr; - ScriptData &sd = g_model.scriptsData[idx]; + ScriptData* const sd = &(g_model.scriptsData[idx]); + ScriptInputsOutputs* const sio = &(scriptInputsOutputs[idx]); - if (sd.file[0] != '\0') { + if (sd->file[0] != '\0') { runtimeData = &(scriptInternalData[scriptIdx++]); } // LUAx label - auto txt = new StaticText(window, grid.getLabelSlot(), + auto const txt = new StaticText(window, grid.getLabelSlot(), std::string("LUA") + std::to_string(idx + 1), BUTTON_BACKGROUND, COLOR_THEME_PRIMARY1 | CENTERED); - Button* button = new ScriptLineButton(window, grid.getFieldSlot(), sd, runtimeData); + Button* const button = new ScriptLineButton(window, grid.getFieldSlot(), *sd, runtimeData); button->setPressHandler([=]() -> uint8_t { - Menu* menu = new Menu(window); + Menu* const menu = new Menu(window); menu->addLine(STR_EDIT, [=]() { editLine(window, idx); }); if (runtimeData != nullptr) { - menu->addLine(STR_RESET, [=]() { - memset((void*)&sd, 0, sizeof(sd)); - // TODO: anything else? reload scripts??? + menu->addLine(STR_DELETE, [=]() { + clearStruct(*sd); + clearStruct(*sio); + LUA_LOAD_MODEL_SCRIPTS(); storageDirty(EE_MODEL); rebuild(window, idx); }); diff --git a/radio/src/lua/interface.cpp b/radio/src/lua/interface.cpp index 4457b8303..53a8279b1 100644 --- a/radio/src/lua/interface.cpp +++ b/radio/src/lua/interface.cpp @@ -604,9 +604,9 @@ static const char * getScriptName(uint8_t idx) } } -static bool luaLoad(const char * filename, ScriptInternalData & sid) +static bool luaLoad(const char * pathname, ScriptInternalData & sid) { - sid.state = luaLoadScriptFileToState(lsScripts, filename, LUA_SCRIPT_LOAD_MODE); + sid.state = luaLoadScriptFileToState(lsScripts, pathname, LUA_SCRIPT_LOAD_MODE); if (sid.state != SCRIPT_OK) { luaFree(lsScripts, sid); @@ -615,22 +615,29 @@ static bool luaLoad(const char * filename, ScriptInternalData & sid) return false; } +template +static bool luaLoadFile(const char (&dirname)[LD], const char (&filename)[LF], ScriptInternalData & sid) { + constexpr size_t maxlen{LD + LF + (sizeof(SCRIPT_EXT) - 1) + 1 + 1}; // iff dirname is string-literal (LD includes '\0') this is one byte too large, but with C++11 there is no chance to check if dirname is a literal or a (maybe-unterminated) char-array + char pathname[maxlen]; + snprintf(pathname, maxlen, "%.*s/%.*s%s", LD, dirname, LF, filename, SCRIPT_EXT); + return luaLoad(pathname, sid); +} + #if defined(LUA_MODEL_SCRIPTS) static bool luaLoadMixScript(uint8_t ref) { +#ifdef DEBUG + if (ref < SCRIPT_MIX_FIRST) { + return false; + } +#endif uint8_t idx = ref - SCRIPT_MIX_FIRST; ScriptData & sd = g_model.scriptsData[idx]; if (ZEXIST(sd.file)) { ScriptInternalData & sid = scriptInternalData[luaScriptsCount++]; sid.reference = ref; - - char filename[sizeof(SCRIPTS_MIXES_PATH) + LEN_SCRIPT_FILENAME + sizeof(SCRIPT_EXT)] = SCRIPTS_MIXES_PATH "/"; - strncpy(filename + sizeof(SCRIPTS_MIXES_PATH), sd.file, LEN_SCRIPT_FILENAME); - filename[sizeof(SCRIPTS_MIXES_PATH) + LEN_SCRIPT_FILENAME] = '\0'; - strcat(filename + sizeof(SCRIPTS_MIXES_PATH), SCRIPT_EXT); - - return luaLoad(filename, sid); + return luaLoadFile(SCRIPTS_MIXES_PATH, sd.file, sid); } return false; } @@ -656,13 +663,7 @@ static bool luaLoadFunctionScript(uint8_t ref) if (luaScriptsCount < MAX_SCRIPTS) { ScriptInternalData & sid = scriptInternalData[luaScriptsCount++]; sid.reference = ref; - - char filename[sizeof(SCRIPTS_FUNCS_PATH) + LEN_FUNCTION_NAME + sizeof(SCRIPT_EXT)] = SCRIPTS_FUNCS_PATH "/"; - strncpy(filename + sizeof(SCRIPTS_FUNCS_PATH), fn->play.name, LEN_FUNCTION_NAME); - filename[sizeof(SCRIPTS_FUNCS_PATH) + LEN_FUNCTION_NAME] = '\0'; - strcat(filename + sizeof(SCRIPTS_FUNCS_PATH), SCRIPT_EXT); - - return luaLoad(filename, sid); + return luaLoadFile(SCRIPTS_FUNCS_PATH, fn->play.name, sid); } else { POPUP_WARNING(STR_TOO_MANY_LUA_SCRIPTS); @@ -685,13 +686,7 @@ static bool luaLoadTelemetryScript(uint8_t ref) if (luaScriptsCount < MAX_SCRIPTS) { ScriptInternalData & sid = scriptInternalData[luaScriptsCount++]; sid.reference = ref; - - char filename[sizeof(SCRIPTS_TELEM_PATH) + LEN_SCRIPT_FILENAME + sizeof(SCRIPT_EXT)] = SCRIPTS_TELEM_PATH "/"; - strncpy(filename + sizeof(SCRIPTS_TELEM_PATH), script.file, LEN_SCRIPT_FILENAME); - filename[sizeof(SCRIPTS_TELEM_PATH) + LEN_SCRIPT_FILENAME] = '\0'; - strcat(filename + sizeof(SCRIPTS_TELEM_PATH), SCRIPT_EXT); - - return luaLoad(filename, sid); + return luaLoadFile(SCRIPTS_TELEM_PATH, script.file, sid); } else { POPUP_WARNING(STR_TOO_MANY_LUA_SCRIPTS); @@ -822,7 +817,7 @@ static int luaRegisterFunction(const char * key) } else { if (typ != LUA_TNIL) { - TRACE_ERROR("luaRegisterFunction(%s): Error: '%s' is not a function\n", getScriptName(luaScriptsCount - 1), key); + TRACE_ERROR("luaRegisterFunction(%s): Error: '%.*s' is not a function\n", LEN_SCRIPT_FILENAME, getScriptName(luaScriptsCount - 1), key); } lua_pop(lsScripts, 1); return LUA_NOREF; @@ -929,7 +924,7 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) sid.background = luaRegisterFunction("background"); initFunction = luaRegisterFunction("init"); if (sid.run == LUA_NOREF) { - snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "luaLoadScripts(%s): No run function\n", getScriptName(idx)); + snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "luaLoadScripts(%.*s): No run function\n", LEN_SCRIPT_FILENAME, getScriptName(idx)); sid.state = SCRIPT_SYNTAX_ERROR; initFunction = LUA_NOREF; } @@ -947,7 +942,7 @@ static void luaLoadScripts(bool init, const char * filename = nullptr) #endif } else { - snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "luaLoadScripts(%s): The script did not return a table\n", getScriptName(idx)); + snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "luaLoadScripts(%.*s): The script did not return a table\n", LEN_SCRIPT_FILENAME, getScriptName(idx)); sid.state = SCRIPT_SYNTAX_ERROR; initFunction = LUA_NOREF; } @@ -1179,7 +1174,7 @@ static bool resumeLua(bool init, bool allowLcdUsage) for (int j = sio -> outputsCount - 1; j >= 0; j--) { if (!lua_isnumber(lsScripts, -1)) { sid.state = SCRIPT_SYNTAX_ERROR; - snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "Script %s: run function did not return a number\n", getScriptName(idx)); + snprintf(lua_warning_info, LUA_WARNING_INFO_LEN, "Script %.*s: run function did not return a number\n", LEN_SCRIPT_FILENAME, getScriptName(idx)); luaError(lsScripts, sid.state); break; } diff --git a/radio/src/strhelpers.cpp b/radio/src/strhelpers.cpp index 6f4858618..404ff88ab 100644 --- a/radio/src/strhelpers.cpp +++ b/radio/src/strhelpers.cpp @@ -549,7 +549,10 @@ char *getSwitchPositionName(char *dest, swsrc_t idx) return dest; } -char *getSourceString(char *dest, mixsrc_t idx) + +// this should be declared in header, but it used so much foreign symbols that we declare it in cpp-file and pre-instantiate it for the uses +template +char *getSourceString(char (&dest)[L], mixsrc_t idx) { if (idx == MIXSRC_NONE) { return getStringAtIndex(dest, STR_VSRCRAW, 0); @@ -557,36 +560,36 @@ char *getSourceString(char *dest, mixsrc_t idx) idx -= MIXSRC_FIRST_INPUT; *dest = CHAR_INPUT; if (strlen(g_model.inputNames[idx])) { - memset(dest + 1, 0, LEN_INPUT_NAME + 1); - strncpy(dest + 1, g_model.inputNames[idx], LEN_INPUT_NAME); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-truncation" + snprintf(dest + 1, L - 1, "%.*s", (int)sizeof(g_model.inputNames[idx]), g_model.inputNames[idx]); +#pragma GCC diagnostic pop } else { strAppendUnsigned(dest + 1, idx + 1, 2); } } + #if defined(LUA_INPUTS) else if (idx <= MIXSRC_LAST_LUA) { #if defined(LUA_MODEL_SCRIPTS) div_t qr = div(idx - MIXSRC_FIRST_LUA, MAX_SCRIPT_OUTPUTS); - if (qr.quot < MAX_SCRIPTS && - qr.rem < scriptInputsOutputs[qr.quot].outputsCount) { - *dest++ = CHAR_LUA; -// temporary string -#define MAX_CHAR 16 - char temp[MAX_CHAR]; - strncpy(temp, g_model.scriptsData[qr.quot].name, MAX_CHAR); - + if (qr.quot < MAX_SCRIPTS && qr.rem < scriptInputsOutputs[qr.quot].outputsCount) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-truncation" + // possible truncation is intentional + snprintf(dest + 1, L - 1, "%.*s", (int)sizeof(g_model.scriptsData[qr.quot].name), g_model.scriptsData[qr.quot].name); // instance Name is empty : dest = n-ScriptFileName/OutputName - if (temp[0] == 0) { - snprintf(temp, MAX_CHAR, "%d-%s/%s", qr.quot + 1, - g_model.scriptsData[qr.quot].file, - scriptInputsOutputs[qr.quot].outputs[qr.rem].name); + if (*(dest + 1) == '\0') { + snprintf(dest + 1, L - 1, "%d-%.*s/%.*s", qr.quot + 1, + (int)sizeof(g_model.scriptsData[qr.quot].file), g_model.scriptsData[qr.quot].file, + (int)sizeof(scriptInputsOutputs[qr.quot].outputs[qr.rem].name), scriptInputsOutputs[qr.quot].outputs[qr.rem].name); // instance Name is not empty : dest = InstanceName/OutputName } else { - snprintf(temp, MAX_CHAR, "%s/%s", g_model.scriptsData[qr.quot].name, - scriptInputsOutputs[qr.quot].outputs[qr.rem].name); + snprintf(dest + 1, L - 1, "%.*s/%.*s", LEN_SCRIPT_NAME, g_model.scriptsData[qr.quot].name, + (int)sizeof(scriptInputsOutputs[qr.quot].outputs[qr.rem].name), scriptInputsOutputs[qr.quot].outputs[qr.rem].name); } - - strcpy(dest, temp); +#pragma GCC diagnostic pop + *dest = CHAR_LUA; } #else strcpy(dest, "N/A"); @@ -594,7 +597,6 @@ char *getSourceString(char *dest, mixsrc_t idx) } #endif else if (idx <= MIXSRC_LAST_POT) { - if (g_eeGeneral.anaNames[idx - MIXSRC_Rud][0]) { // TODO: add correct symbol if (idx <= MIXSRC_LAST_STICK) { @@ -611,8 +613,8 @@ char *getSourceString(char *dest, mixsrc_t idx) } idx -= MIXSRC_Rud; - memcpy(dest + 1, g_eeGeneral.anaNames[idx], LEN_ANA_NAME); - dest[LEN_ANA_NAME + 1] = '\0'; + memcpy(dest + 1, g_eeGeneral.anaNames[idx], L - 1); + dest[L - 1] = '\0'; } else { idx -= MIXSRC_Rud; getStringAtIndex(dest, STR_VSRCRAW, idx + 1); @@ -646,8 +648,8 @@ char *getSourceString(char *dest, mixsrc_t idx) } else if (idx <= MIXSRC_LAST_TIMER) { if (g_model.timers[idx - MIXSRC_FIRST_TIMER].name[0] != '\0') { strncpy(dest, g_model.timers[idx - MIXSRC_FIRST_TIMER].name, - LEN_TIMER_NAME); - dest[LEN_TIMER_NAME] = '\0'; + L - 1); + dest[L - 1] = '\0'; } else { getStringAtIndex(dest, STR_VSRCRAW, idx - MIXSRC_Rud + 1 - MAX_LOGICAL_SWITCHES - @@ -663,9 +665,14 @@ char *getSourceString(char *dest, mixsrc_t idx) if (qr.rem) *pos = (qr.rem == 2 ? '+' : '-'); *++pos = '\0'; } - - return dest; + dest[L - 1] = '\0'; // assert the termination (should not be nesseccary) + return dest; } + +// pre-instantiate for use from external +// all other instantiations are done from this file +template char *getSourceString<16>(char (&dest)[16], mixsrc_t idx); + #endif char *strAppendUnsigned(char *dest, uint32_t value, uint8_t digits, diff --git a/radio/src/strhelpers.h b/radio/src/strhelpers.h index 5ed7d8807..22ff8a9bd 100644 --- a/radio/src/strhelpers.h +++ b/radio/src/strhelpers.h @@ -26,6 +26,7 @@ #include "opentx_types.h" #include +#include #define SHOW_TIME 0x1 #define SHOW_TIMER 0x0 @@ -75,12 +76,16 @@ char *getGVarString(char *dest, int idx); char *getGVarString(int idx); char *getSwitchPositionName(char *dest, swsrc_t idx); char *getSwitchName(char *dest, swsrc_t idx); -char *getSourceString(char *dest, mixsrc_t idx); + +template +char *getSourceString(char (&dest)[L], mixsrc_t idx); + int getRawSwitchIdx(char sw); char getRawSwitchFromIdx(int sw); #endif char *getFlightModeString(char *dest, int8_t idx); + #define SWITCH_WARNING_STR_SIZE 3 // char *getSwitchWarningString(char *dest, swsrc_t idx); @@ -96,4 +101,16 @@ template std::string stringFromNtString(const char (&a)[N]) { return std::string(a, strnlen(a, N)); } +template +void copyToUnTerminated(char (&dest)[L], const char* const src) { + strncpy(dest, src, L); +} +template +void copyToUnTerminated(char (&dest)[L], const std::string& src) { + strncpy(dest, src.c_str(), L); +} +template +void clearStruct(S& s) { + memset((void*) &s, 0, sizeof(S)); +} #endif // _STRHELPERS_H_