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 052e28a760..045f14776e 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 39d290f619..aea6b1261b 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 18b4a72a73..66235e8a04 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 4eb7cf1370..db9aa01632 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 debe25cdb4..fce15b5931 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 7777d7d5d7..f8b860f375 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 48dec53083..24de8e7d89 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 dc5745fcc2..1e6b8e06ca 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 2a41308993..1e00c050dd 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; } }