From 9081bd33385f4fefebfd135604ff4fe25bd4e490 Mon Sep 17 00:00:00 2001 From: Bruce Luckcuck Date: Mon, 20 May 2019 12:20:34 -0400 Subject: [PATCH] Add CMS menu entry level control of forced reboot on changes CMS menu items can now be defined with a `REBOOT_REQUIRED` flag. If the user changes flagged elements the the exit/save options will change to `EXIT&REBOOT` and `SAVE&REBOOT` - ensuring that the changed items will be handled properly. This should be used for options where runtime changes either won't take effect (because they are read at boot), or for items that could have unexpected behavior if changed. Several appropriate menus have been updated to include the flag. To accomodate the dynamic nature of the save/exit options, the individual options have been removed from the main menu and replaced with a `SAVE/EXIT` submenu. This is the same menu that is presented as the quick-access popup save menu (yaw-right). This menu will adapt to requiring a reboot if any flagged setting is changed. Additionally the `REBOOT_REQD` arming disabled reason will be set (cleared by the reboot). --- src/main/cms/cms.c | 101 ++++++++++++++++++++++--------- src/main/cms/cms.h | 1 + src/main/cms/cms_menu_blackbox.c | 4 +- src/main/cms/cms_menu_builtin.c | 23 +++++-- src/main/cms/cms_menu_misc.c | 4 +- src/main/cms/cms_menu_saveexit.c | 24 +++++++- src/main/cms/cms_menu_saveexit.h | 1 + src/main/cms/cms_types.h | 1 + src/test/unit/cms_unittest.cc | 2 + 9 files changed, 119 insertions(+), 42 deletions(-) diff --git a/src/main/cms/cms.c b/src/main/cms/cms.c index 052e28a76..045f14776 100644 --- a/src/main/cms/cms.c +++ b/src/main/cms/cms.c @@ -776,8 +776,9 @@ long cmsMenuExit(displayPort_t *pDisplay, const void *ptr) cmsTraverseGlobalExit(&menuMain); - if (currentCtx.menu->onExit) + if (currentCtx.menu->onExit) { currentCtx.menu->onExit((OSD_Entry *)NULL); // Forced exit + } if ((exitType == CMS_POPUP_SAVE) || (exitType == CMS_POPUP_SAVEREBOOT)) { // traverse through the menu stack and call their onExit functions @@ -800,7 +801,7 @@ long cmsMenuExit(displayPort_t *pDisplay, const void *ptr) displayRelease(pDisplay); currentCtx.menu = NULL; - if ((exitType == CMS_EXIT_SAVEREBOOT) || (exitType == CMS_POPUP_SAVEREBOOT)) { + if ((exitType == CMS_EXIT_SAVEREBOOT) || (exitType == CMS_POPUP_SAVEREBOOT) || (exitType == CMS_POPUP_EXITREBOOT)) { displayClearScreen(pDisplay); displayWrite(pDisplay, 5, 3, "REBOOTING..."); @@ -832,8 +833,9 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) uint16_t res = BUTTON_TIME; const OSD_Entry *p; - if (!currentCtx.menu) + if (!currentCtx.menu) { return res; + } if (key == CMS_KEY_MENU) { cmsMenuOpen(); @@ -851,7 +853,11 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) if (key == CMS_KEY_SAVEMENU) { osdElementEditing = false; - cmsMenuChange(pDisplay, &cmsx_menuSaveExit); + if (getRebootRequired()) { + cmsMenuChange(pDisplay, &cmsx_menuSaveExitReboot); + } else { + cmsMenuChange(pDisplay, &cmsx_menuSaveExit); + } return BUTTON_PAUSE; } @@ -868,8 +874,9 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) currentCtx.cursorRow--; // Skip non-title labels - if ((pageTop + currentCtx.cursorRow)->type == OME_Label && currentCtx.cursorRow > 0) + if ((pageTop + currentCtx.cursorRow)->type == OME_Label && currentCtx.cursorRow > 0) { currentCtx.cursorRow--; + } if (currentCtx.cursorRow == -1 || (pageTop + currentCtx.cursorRow)->type == OME_Label) { // Goto previous page @@ -878,8 +885,9 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) } } - if ((key == CMS_KEY_DOWN || key == CMS_KEY_UP) && (!osdElementEditing)) + if ((key == CMS_KEY_DOWN || key == CMS_KEY_UP) && (!osdElementEditing)) { return res; + } p = pageTop + currentCtx.cursorRow; @@ -895,8 +903,9 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) long retval; if (p->func && key == CMS_KEY_RIGHT) { retval = p->func(pDisplay, p->data); - if (retval == MENU_CHAIN_BACK) + if (retval == MENU_CHAIN_BACK) { cmsMenuBack(pDisplay); + } res = BUTTON_PAUSE; } break; @@ -917,11 +926,12 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_Bool: if (p->data) { uint8_t *val = p->data; - if (key == CMS_KEY_RIGHT) - *val = 1; - else - *val = 0; + const uint8_t previousValue = *val; + *val = (key == CMS_KEY_RIGHT) ? 1 : 0; SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*val != previousValue)) { + setRebootRequired(); + } } break; @@ -929,6 +939,7 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_VISIBLE: if (p->data) { uint16_t *val = (uint16_t *)p->data; + const uint16_t previousValue = *val; if ((key == CMS_KEY_RIGHT) && (!osdElementEditing)) { osdElementEditing = true; osdProfileCursor = 1; @@ -953,6 +964,9 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) } } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*val != previousValue)) { + setRebootRequired(); + } } break; #endif @@ -961,15 +975,21 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_FLOAT: if (p->data) { OSD_UINT8_t *ptr = p->data; + const uint16_t previousValue = *ptr->val; if (key == CMS_KEY_RIGHT) { - if (*ptr->val < ptr->max) + if (*ptr->val < ptr->max) { *ptr->val += ptr->step; + } } else { - if (*ptr->val > ptr->min) + if (*ptr->val > ptr->min) { *ptr->val -= ptr->step; + } } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*ptr->val != previousValue)) { + setRebootRequired(); + } if (p->func) { p->func(pDisplay, p); } @@ -979,33 +999,44 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_TAB: if (p->type == OME_TAB) { OSD_TAB_t *ptr = p->data; + const uint8_t previousValue = *ptr->val; if (key == CMS_KEY_RIGHT) { - if (*ptr->val < ptr->max) + if (*ptr->val < ptr->max) { *ptr->val += 1; - } - else { - if (*ptr->val > 0) + } + } else { + if (*ptr->val > 0) { *ptr->val -= 1; + } } - if (p->func) + if (p->func) { p->func(pDisplay, p->data); + } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*ptr->val != previousValue)) { + setRebootRequired(); + } } break; case OME_INT8: if (p->data) { OSD_INT8_t *ptr = p->data; + const int8_t previousValue = *ptr->val; if (key == CMS_KEY_RIGHT) { - if (*ptr->val < ptr->max) + if (*ptr->val < ptr->max) { *ptr->val += ptr->step; - } - else { - if (*ptr->val > ptr->min) + } + } else { + if (*ptr->val > ptr->min) { *ptr->val -= ptr->step; + } } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*ptr->val != previousValue)) { + setRebootRequired(); + } if (p->func) { p->func(pDisplay, p); } @@ -1015,15 +1046,20 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_UINT16: if (p->data) { OSD_UINT16_t *ptr = p->data; + const uint16_t previousValue = *ptr->val; if (key == CMS_KEY_RIGHT) { - if (*ptr->val < ptr->max) + if (*ptr->val < ptr->max) { *ptr->val += ptr->step; - } - else { - if (*ptr->val > ptr->min) + } + } else { + if (*ptr->val > ptr->min) { *ptr->val -= ptr->step; + } } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*ptr->val != previousValue)) { + setRebootRequired(); + } if (p->func) { p->func(pDisplay, p); } @@ -1033,15 +1069,20 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, cms_key_e key) case OME_INT16: if (p->data) { OSD_INT16_t *ptr = p->data; + const int16_t previousValue = *ptr->val; if (key == CMS_KEY_RIGHT) { - if (*ptr->val < ptr->max) + if (*ptr->val < ptr->max) { *ptr->val += ptr->step; - } - else { - if (*ptr->val > ptr->min) + } + } else { + if (*ptr->val > ptr->min) { *ptr->val -= ptr->step; + } } SET_PRINTVALUE(runtimeEntryFlags[currentCtx.cursorRow]); + if ((p->flags & REBOOT_REQUIRED) && (*ptr->val != previousValue)) { + setRebootRequired(); + } if (p->func) { p->func(pDisplay, p); } diff --git a/src/main/cms/cms.h b/src/main/cms/cms.h index 39d290f61..aea6b1261 100644 --- a/src/main/cms/cms.h +++ b/src/main/cms/cms.h @@ -62,4 +62,5 @@ void cmsSetExternKey(cms_key_e extKey); #define CMS_EXIT_SAVEREBOOT (2) #define CMS_POPUP_SAVE (3) #define CMS_POPUP_SAVEREBOOT (4) +#define CMS_POPUP_EXITREBOOT (5) diff --git a/src/main/cms/cms_menu_blackbox.c b/src/main/cms/cms_menu_blackbox.c index 18b4a72a7..66235e8a0 100644 --- a/src/main/cms/cms_menu_blackbox.c +++ b/src/main/cms/cms_menu_blackbox.c @@ -200,11 +200,11 @@ static long cmsx_Blackbox_onExit(const OSD_Entry *self) static const OSD_Entry cmsx_menuBlackboxEntries[] = { { "-- BLACKBOX --", OME_Label, NULL, NULL, 0}, - { "DEVICE", OME_TAB, NULL, &cmsx_BlackboxDeviceTable, 0 }, + { "DEVICE", OME_TAB, NULL, &cmsx_BlackboxDeviceTable, REBOOT_REQUIRED }, { "(STATUS)", OME_String, NULL, &cmsx_BlackboxStatus, 0 }, { "(USED)", OME_String, NULL, &cmsx_BlackboxDeviceStorageUsed, 0 }, { "(FREE)", OME_String, NULL, &cmsx_BlackboxDeviceStorageFree, 0 }, - { "P RATIO", OME_UINT16, NULL, &(OSD_UINT16_t){ &blackboxConfig_p_ratio, 1, INT16_MAX, 1 },0 }, + { "P RATIO", OME_UINT16, NULL, &(OSD_UINT16_t){ &blackboxConfig_p_ratio, 1, INT16_MAX, 1 }, REBOOT_REQUIRED }, #ifdef USE_FLASHFS { "ERASE FLASH", OME_Funcall, cmsx_EraseFlash, NULL, 0 }, diff --git a/src/main/cms/cms_menu_builtin.c b/src/main/cms/cms_menu_builtin.c index 4eb7cf137..db9aa0163 100644 --- a/src/main/cms/cms_menu_builtin.c +++ b/src/main/cms/cms_menu_builtin.c @@ -40,11 +40,12 @@ #include "cms/cms_menu_imu.h" #include "cms/cms_menu_blackbox.h" -#include "cms/cms_menu_osd.h" +#include "cms/cms_menu_failsafe.h" #include "cms/cms_menu_ledstrip.h" #include "cms/cms_menu_misc.h" +#include "cms/cms_menu_osd.h" #include "cms/cms_menu_power.h" -#include "cms/cms_menu_failsafe.h" +#include "cms/cms_menu_saveexit.h" // VTX supplied menus @@ -54,6 +55,8 @@ #include "drivers/system.h" +#include "fc/config.h" + #include "msp/msp_protocol.h" // XXX for FC identification... not available elsewhere #include "cms_menu_builtin.h" @@ -139,6 +142,18 @@ static CMS_Menu menuFeatures = { .entries = menuFeaturesEntries, }; +static long cmsx_SaveExitMenu(displayPort_t *pDisplay, const void *ptr) +{ + UNUSED(ptr); + + if (getRebootRequired()) { + cmsMenuChange(pDisplay, &cmsx_menuSaveExitReboot); + } else { + cmsMenuChange(pDisplay, &cmsx_menuSaveExit); + } + return 0; +} + // Main static const OSD_Entry menuMainEntries[] = @@ -152,9 +167,7 @@ static const OSD_Entry menuMainEntries[] = #endif {"FC&FW INFO", OME_Submenu, cmsMenuChange, &menuInfo, 0}, {"MISC", OME_Submenu, cmsMenuChange, &cmsx_menuMisc, 0}, - {"EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_EXIT, 0}, - {"SAVE&EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_EXIT_SAVE, 0}, - {"SAVE&REBOOT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_EXIT_SAVEREBOOT, 0}, + {"SAVE/EXIT", OME_Funcall, cmsx_SaveExitMenu, NULL, 0}, #ifdef CMS_MENU_DEBUG {"ERR SAMPLE", OME_Submenu, cmsMenuChange, &menuInfoEntries[0], 0}, #endif diff --git a/src/main/cms/cms_menu_misc.c b/src/main/cms/cms_menu_misc.c index debe25cdb..fce15b593 100644 --- a/src/main/cms/cms_menu_misc.c +++ b/src/main/cms/cms_menu_misc.c @@ -123,8 +123,8 @@ static const OSD_Entry menuMiscEntries[]= { { "-- MISC --", OME_Label, NULL, NULL, 0 }, - { "MIN THR", OME_UINT16, NULL, &(OSD_UINT16_t){ &motorConfig_minthrottle, 1000, 2000, 1 }, 0 }, - { "DIGITAL IDLE", OME_UINT8, NULL, &(OSD_UINT8_t) { &motorConfig_digitalIdleOffsetValue, 0, 200, 1 }, 0 }, + { "MIN THR", OME_UINT16, NULL, &(OSD_UINT16_t){ &motorConfig_minthrottle, 1000, 2000, 1 }, REBOOT_REQUIRED }, + { "DIGITAL IDLE", OME_UINT8, NULL, &(OSD_UINT8_t) { &motorConfig_digitalIdleOffsetValue, 0, 200, 1 }, REBOOT_REQUIRED }, { "DEBUG MODE", OME_TAB, NULL, &(OSD_TAB_t) { &systemConfig_debug_mode, DEBUG_COUNT - 1, debugModeNames }, 0 }, { "RC PREV", OME_Submenu, cmsMenuChange, &cmsx_menuRcPreview, 0}, diff --git a/src/main/cms/cms_menu_saveexit.c b/src/main/cms/cms_menu_saveexit.c index 7777d7d5d..f8b860f37 100644 --- a/src/main/cms/cms_menu_saveexit.c +++ b/src/main/cms/cms_menu_saveexit.c @@ -37,9 +37,9 @@ static const OSD_Entry cmsx_menuSaveExitEntries[] = { { "-- SAVE/EXIT --", OME_Label, NULL, NULL, 0}, - {"EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_EXIT, 0}, - {"SAVE&EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_SAVE, 0}, - {"SAVE&REBOOT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_SAVEREBOOT, 0}, + { "EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_EXIT, 0}, + { "SAVE&EXIT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_SAVE, 0}, + { "SAVE&REBOOT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_SAVEREBOOT, 0}, { "BACK", OME_Back, NULL, NULL, 0 }, { NULL, OME_END, NULL, NULL, 0 } }; @@ -52,4 +52,22 @@ CMS_Menu cmsx_menuSaveExit = { .entries = cmsx_menuSaveExitEntries }; +static const OSD_Entry cmsx_menuSaveExitRebootEntries[] = +{ + { "-- SAVE/EXIT (REBOOT REQD)", OME_Label, NULL, NULL, 0}, + { "EXIT&REBOOT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_EXITREBOOT, 0}, + { "SAVE&REBOOT", OME_OSD_Exit, cmsMenuExit, (void *)CMS_POPUP_SAVEREBOOT, 0}, + { "BACK", OME_Back, NULL, NULL, 0 }, + { NULL, OME_END, NULL, NULL, 0 } +}; + +CMS_Menu cmsx_menuSaveExitReboot = { +#ifdef CMS_MENU_DEBUG + .GUARD_text = "MENUSAVE", + .GUARD_type = OME_MENU, +#endif + .entries = cmsx_menuSaveExitRebootEntries +}; + + #endif diff --git a/src/main/cms/cms_menu_saveexit.h b/src/main/cms/cms_menu_saveexit.h index 48dec5308..24de8e7d8 100644 --- a/src/main/cms/cms_menu_saveexit.h +++ b/src/main/cms/cms_menu_saveexit.h @@ -21,3 +21,4 @@ #pragma once extern CMS_Menu cmsx_menuSaveExit; +extern CMS_Menu cmsx_menuSaveExitReboot; diff --git a/src/main/cms/cms_types.h b/src/main/cms/cms_types.h index dc5745fcc..1e6b8e06c 100644 --- a/src/main/cms/cms_types.h +++ b/src/main/cms/cms_types.h @@ -70,6 +70,7 @@ typedef struct #define PRINT_LABEL 0x02 // Text label should be printed #define DYNAMIC 0x04 // Value should be updated dynamically #define OPTSTRING 0x08 // (Temporary) Flag for OME_Submenu, indicating func should be called to get a string to display. +#define REBOOT_REQUIRED 0x10 // Reboot is required if the value is changed #define IS_PRINTVALUE(x) ((x) & PRINT_VALUE) #define SET_PRINTVALUE(x) do { (x) |= PRINT_VALUE; } while (0) diff --git a/src/test/unit/cms_unittest.cc b/src/test/unit/cms_unittest.cc index 2a4130899..1e00c050d 100644 --- a/src/test/unit/cms_unittest.cc +++ b/src/test/unit/cms_unittest.cc @@ -147,4 +147,6 @@ void systemReset(void) {} void setArmingDisabled(armingDisableFlags_e flag) { UNUSED(flag); } void unsetArmingDisabled(armingDisableFlags_e flag) { UNUSED(flag); } bool IS_RC_MODE_ACTIVE(boxId_e) { return false; } +void setRebootRequired(void) {} +bool getRebootRequired(void) { return false; } }