Web Config Saves Fix + Removing Legacy Code (#1339)

* Moved config mode check to the DriverManager

Removed ConfigManager as this idiom never took off and we have better ones in place

Modified NetDriver to run rndis making it the controller of web

Moved Storage -> config Mode to Driver -> config Mode

Fixed no-save issue with web config calls

* Move our reboot translations back to WEB -> SYSTEM

* Added rebootMode to event trigger handling in gp2040

CMakeLists.txt

@@ -149,7 +149,6 @@ src/gp2040aux.cpp
 src/gamepad.cpp
 src/gamepad/GamepadState.cpp
 src/addonmanager.cpp
-src/configmanager.cpp
 src/playerleds.cpp
 src/drivers/shared/xinput_host.cpp
 src/drivers/shared/xgip_protocol.cpp
@@ -211,7 +210,7 @@ src/usbdriver.cpp
 src/usbhostmanager.cpp
 src/config_legacy.cpp
 src/config_utils.cpp
-src/configs/webconfig.cpp
+src/webconfig.cpp
 src/addons/analog.cpp
 src/addons/board_led.cpp
 src/addons/bootsel_button.cpp

headers/configmanager.h

@@ -1,26 +0,0 @@
-#ifndef CONFIGMANAGER_H
-#define CONFIGMANAGER_H
-
-#include "enums.h"
-#include "gpconfig.h"
-#include "storagemanager.h"
-
-class ConfigManager {
-public:
-	ConfigManager(ConfigManager const&) = delete;
-	void operator=(ConfigManager const&)  = delete;
-    static ConfigManager& getInstance() {// Thread-safe storage ensures cross-thread talk
-		static ConfigManager instance; // Guaranteed to be destroyed. // Instantiated on first use.
-		return instance;
-	}
-    void setup(ConfigType);
-    void loop(); // If anything needs to update in the gpconfig driver
-    void setGamepadOptions(Gamepad*);
-private:
-    ConfigManager() {}
-    void setupConfig(GPConfig*);
-    ConfigType cType;
-    GPConfig * config;
-};
-
-#endif

headers/configs/displayconfig.h

@@ -1,29 +0,0 @@
-#ifndef _DISPLAYCONFIG_H_
-#define _DISPLAYCONFIG_H_
-
-#include "gpconfig.h"
-
-#include <vector>
-
-class DisplayState {
-public:
-private:
-    std::vector<char*> options;
-};
-
-class DisplayMenu {
-public:
-    
-private:
-};
-
-class DisplayConfig : public GPConfig
-{
-public:
-    virtual void setup();
-    virtual void loop();
-private:
-    std::vector<char **> menu_data;
-};
-
-#endif

headers/configs/serialconfig.h

@@ -1,14 +0,0 @@
-#ifndef _SERIALCONFIG_H_
-#define _SERIALCONFIG_H_
-
-#include "gpconfig.h"
-
-class SerialConfig : public GPConfig
-{
-public:
-    virtual void setup();
-    virtual void loop();
-private:
-};
-
-#endif

headers/configs/webconfig.h

@@ -1,20 +0,0 @@
-#ifndef _WEBCONFIG_H_
-#define _WEBCONFIG_H_
-
-#include "gpconfig.h"
-
-class WebConfig : public GPConfig
-{
-public:
-    virtual void setup();
-    virtual void loop();
-
-    enum BootModes {
-        GAMEPAD,
-        WEBCONFIG,
-        BOOTSEL
-    };
-private:
-};
-
-#endif

headers/drivermanager.h

@@ -17,6 +17,7 @@ public:
     GPDriver * getDriver() { return driver; }
     void setup(InputMode);
     InputMode getInputMode(){ return inputMode; }
+    bool isConfigMode(){ return (inputMode == INPUT_MODE_CONFIG); }
 private:
     DriverManager() {}
     GPDriver * driver;

headers/gp2040.h

@@ -85,6 +85,8 @@ private:
 
     bool rebootRequested = false;
     void handleSystemReboot(GPEvent* e);
+
+    System::BootMode rebootMode = System::BootMode::DEFAULT;
 };
 
 #endif

headers/storagemanager.h

@@ -53,9 +53,6 @@ public:
 	bool save();
 	bool save(const bool force);
 
-	void SetConfigMode(bool); 			// Config Mode (on-boot)
-	bool GetConfigMode();
-
 	void SetGamepad(Gamepad *); 		// MPGS Gamepad Get/Set
 	Gamepad * GetGamepad();
 
@@ -70,6 +67,8 @@ public:
 
 	void ResetSettings(); 				// EEPROM Reset Feature
 
+	uint32_t GetFlashSize() { return systemFlashSize; }
+
 private:
 	Storage() {}
 	bool CONFIG_MODE = false; 			// Config mode (boot)
@@ -79,6 +78,7 @@ private:
 	DisplayOptions previewDisplayOptions;
 	Config config;
 	GpioMappingInfo functionalPinMappings[NUM_BANK0_GPIOS];
+	uint32_t systemFlashSize;
 };
 
 #endif

src/addons/board_led.cpp

@@ -13,7 +13,7 @@ bool BoardLedAddon::available() {
 void BoardLedAddon::setup() {
     const OnBoardLedOptions& options = Storage::getInstance().getAddonOptions().onBoardLedOptions;
     onBoardLedMode = options.mode;
-    isConfigMode = Storage::getInstance().GetConfigMode();
+    isConfigMode = DriverManager::getInstance().isConfigMode();
     timeSinceBlink = getMillis();
     prevState = -1;
 

src/addons/buzzerspeaker.cpp

@@ -1,6 +1,7 @@
 #include "hardware/pwm.h"
 #include "addons/buzzerspeaker.h"
 #include "songs.h"
+#include "drivermanager.h"
 #include "storagemanager.h"
 #include "usbdriver.h"
 #include "math.h"
@@ -45,7 +46,7 @@ void BuzzerSpeakerAddon::playIntro() {
 		return;
 	}
 
-	bool isConfigMode = Storage::getInstance().GetConfigMode();
+	bool isConfigMode = DriverManager::getInstance().isConfigMode();
 
 	if (!get_usb_mounted() || isConfigMode) {
 		play(&configModeSong);

src/addons/display.cpp

@@ -48,7 +48,7 @@ void DisplayAddon::setup() {
 
     displaySaverTimer = options.displaySaverTimeout;
     displaySaverTimeout = displaySaverTimer;
-    configMode = Storage::getInstance().GetConfigMode();
+    configMode = DriverManager::getInstance().isConfigMode();
     turnOffWhenSuspended = options.turnOffWhenSuspended;
     displaySaverMode = options.displaySaverMode;
 
@@ -210,7 +210,7 @@ void DisplayAddon::process() {
 }
 
 const DisplayOptions& DisplayAddon::getDisplayOptions() {
-    bool configMode = Storage::getInstance().GetConfigMode();
+    bool configMode = DriverManager::getInstance().isConfigMode();
     return configMode ? Storage::getInstance().getPreviewDisplayOptions() : Storage::getInstance().getDisplayOptions();
 }
 

src/config_utils.cpp

@@ -37,7 +37,7 @@
 
 #include "CRC32.h"
 #include "FlashPROM.h"
-#include "configs/base64.h"
+#include "base64.h"
 
 #include <ArduinoJson.h>
 

src/configmanager.cpp

@@ -1,19 +0,0 @@
-#include "configmanager.h"
-
-#include "configs/webconfig.h"
-
-void ConfigManager::setup(ConfigType config) {
-	if (config == CONFIG_TYPE_WEB)
-		setupConfig(new WebConfig());
-
-    this->cType = config;
-}
-
-void ConfigManager::loop() {
-    config->loop();
-}
-
-void ConfigManager::setupConfig(GPConfig * gpconfig) {
-    gpconfig->setup();
-    this->config = gpconfig;
-}

src/display/GPGFX_UI.cpp

@@ -1,5 +1,7 @@
 #include "GPGFX_UI.h"
 
+#include "drivermanager.h"
+
 #include <cstring>
 
 GPGFX_UI::GPGFX_UI() {
@@ -14,7 +16,7 @@ Gamepad* GPGFX_UI::getProcessedGamepad() {
 }
 
 DisplayOptions GPGFX_UI::getDisplayOptions() {
-    bool configMode = Storage::getInstance().GetConfigMode();
+    bool configMode = DriverManager::getInstance().isConfigMode();
     return configMode ? Storage::getInstance().getPreviewDisplayOptions() : Storage::getInstance().getDisplayOptions();
 }
 

src/display/ui/screens/ButtonLayoutScreen.cpp

@@ -92,7 +92,7 @@ void ButtonLayoutScreen::shutdown() {
 }
 
 int8_t ButtonLayoutScreen::update() {
-    bool configMode = Storage::getInstance().GetConfigMode();
+    bool configMode = DriverManager::getInstance().isConfigMode();
     uint8_t profileNumber = getGamepad()->getOptions().profileNumber;
     
     // Check if we've updated button layouts while in config mode
@@ -120,7 +120,7 @@ int8_t ButtonLayoutScreen::update() {
 		processInputHistory();
 
     // check for exit/screen change
-    if (Storage::getInstance().GetConfigMode()) {
+    if (DriverManager::getInstance().isConfigMode()) {
         uint16_t buttonState = getGamepad()->state.buttons;
         if (prevButtonState && !buttonState) {
             if (prevButtonState == GAMEPAD_MASK_B1) {

src/display/ui/screens/DisplaySaverScreen.cpp

@@ -1,5 +1,6 @@
 #include "DisplaySaverScreen.h"
 
+#include "drivermanager.h"
 #include "pico/stdlib.h"
 #include "version.h"
 
@@ -45,7 +46,7 @@ void DisplaySaverScreen::drawScreen() {
 }
 
 int8_t DisplaySaverScreen::update() {
-    if (!Storage::getInstance().GetConfigMode()) {
+    if (!DriverManager::getInstance().isConfigMode()) {
         uint16_t buttonState = getGamepad()->state.buttons;
         if (prevButtonState && !buttonState) {
             if (prevButtonState != 0) {

src/display/ui/screens/PinViewerScreen.cpp

@@ -1,6 +1,7 @@
 #include "PinViewerScreen.h"
 
 #include "pico/stdlib.h"
+#include "drivermanager.h"
 
 void PinViewerScreen::init() {
     getRenderer()->clearScreen();
@@ -51,7 +52,7 @@ void PinViewerScreen::drawScreen() {
 }
 
 int8_t PinViewerScreen::update() {
-    if (Storage::getInstance().GetConfigMode()) {
+    if (DriverManager::getInstance().isConfigMode()) {
         uint16_t buttonState = getGamepad()->state.buttons;
         if (prevButtonState && !buttonState) {
             if (prevButtonState == GAMEPAD_MASK_A2) {

src/display/ui/screens/RestartScreen.cpp

@@ -8,7 +8,6 @@
 void RestartScreen::init() {
     getRenderer()->clearScreen();
     //splashStartTime = getMillis();
-    //configMode = Storage::getInstance().GetConfigMode();
 }
 
 void RestartScreen::shutdown() {

src/display/ui/screens/SplashScreen.cpp

@@ -1,11 +1,12 @@
 #include "SplashScreen.h"
 
 #include "pico/stdlib.h"
+#include "drivermanager.h"
 
 void SplashScreen::init() {
     getRenderer()->clearScreen();
     splashStartTime = getMillis();
-    configMode = Storage::getInstance().GetConfigMode();
+    configMode = DriverManager::getInstance().isConfigMode();
 }
 
 void SplashScreen::shutdown() {

src/display/ui/screens/StatsScreen.cpp

@@ -2,6 +2,7 @@
 
 #include "pico/stdlib.h"
 #include "version.h"
+#include "drivermanager.h"
 
 void StatsScreen::init() {
     getRenderer()->clearScreen();
@@ -23,7 +24,7 @@ void StatsScreen::drawScreen() {
 }
 
 int8_t StatsScreen::update() {
-    if (Storage::getInstance().GetConfigMode()) {
+    if (DriverManager::getInstance().isConfigMode()) {
         uint16_t buttonState = getGamepad()->state.buttons;
         if (prevButtonState && !buttonState) {
             if (prevButtonState == GAMEPAD_MASK_B2) {

src/drivers/net/NetDriver.cpp

@@ -1,7 +1,7 @@
 #include "drivers/net/NetDriver.h"
 #include "drivers/shared/driverhelper.h"
 #include "class/net/net_device.h"
-
+#include "rndis.h"
 
 /* A combination of interfaces must have a unique product id, since PC will save device driver after the first plug.
  * Same VID/PID with different interface e.g MSC (first), then CDC (later) will possibly cause system error on PC.
@@ -54,9 +54,15 @@ void NetDriver::initialize() {
         .xfer_cb          = netd_xfer_cb,
         .sof              = NULL,
     };
+
+    rndis_init();
 }
 
-bool NetDriver::process(Gamepad * gamepad) {return false;}
+// Run RNDIS task from web config
+bool NetDriver::process(Gamepad * gamepad) {
+    rndis_task();
+    return false;
+}
 
 // tud_hid_get_report_cb
 uint16_t NetDriver::get_report(uint8_t report_id, hid_report_type_t report_type, uint8_t *buffer, uint16_t reqlen) {

src/gp2040.cpp

@@ -5,7 +5,6 @@
 #include "enums.pb.h"
 
 #include "build_info.h"
-#include "configmanager.h" // Global Managers
 #include "peripheralmanager.h"
 #include "storagemanager.h"
 #include "addonmanager.h"
@@ -117,11 +116,8 @@ void GP2040::setup() {
 	const BootAction bootAction = getBootAction();
 	switch (bootAction) {
 		case BootAction::ENTER_WEBCONFIG_MODE:
-			// Move this to the Net driver initialize
-			Storage::getInstance().SetConfigMode(true);
-			DriverManager::getInstance().setup(INPUT_MODE_CONFIG);
-			ConfigManager::getInstance().setup(CONFIG_TYPE_WEB);
-			return;
+			inputMode = INPUT_MODE_CONFIG;
+			break;
 		case BootAction::ENTER_USB_MODE:
 			reset_usb_boot(0, 0);
 			return;
@@ -178,16 +174,16 @@ void GP2040::setup() {
 	// Setup USB Driver
 	DriverManager::getInstance().setup(inputMode);
 
-	// Save the changed input mode
-	if (inputMode != gamepad->getOptions().inputMode) {	
+	// save to match user expectations on choosing mode at boot, and this is
+	// before USB host will be used so we can force it to ignore the check
+	if (inputMode != INPUT_MODE_CONFIG && inputMode != gamepad->getOptions().inputMode) {
 		gamepad->setInputMode(inputMode);
-		// save to match user expectations on choosing mode at boot, and this is
-		// before USB host will be used so we can force it to ignore the check
 		Storage::getInstance().save(true);
 	}
 
 	// register system event handlers
 	EventManager::getInstance().registerEventHandler(GP_EVENT_STORAGE_SAVE, GPEVENT_CALLBACK(this->handleStorageSave(event)));
+	EventManager::getInstance().registerEventHandler(GP_EVENT_RESTART, GPEVENT_CALLBACK(this->handleSystemReboot(event)));
 }
 
 /**
@@ -263,10 +259,10 @@ void GP2040::debounceGpioGetAll() {
 }
 
 void GP2040::run() {
+	bool configMode = DriverManager::getInstance().isConfigMode();
 	GPDriver * inputDriver = DriverManager::getInstance().getDriver();
 	Gamepad * gamepad = Storage::getInstance().GetGamepad();
 	Gamepad * processedGamepad = Storage::getInstance().GetProcessedGamepad();
-	bool configMode = Storage::getInstance().GetConfigMode();
     GamepadState prevState;
 
     // Start the TinyUSB Device functionality
@@ -284,11 +280,11 @@ void GP2040::run() {
 
 		checkRawState(prevState, gamepad->state);
 
-		// Config Loop (Web-Config does not require gamepad)
+		// Config Loop (Web-Config skips Core0 add-ons)
 		if (configMode == true) {
-			
-			ConfigManager::getInstance().loop();
+			inputDriver->process(gamepad);
 			rebootHotkeys.process(gamepad, configMode);
+			checkSaveRebootState();
 			continue;
 		}
 
@@ -314,7 +310,8 @@ void GP2040::run() {
 		// Process Input Driver
 		bool processed = inputDriver->process(gamepad);
 		
-		tud_task(); // TinyUSB Task update
+		// TinyUSB Task update
+		tud_task();
 
 		// Post-Process Add-ons with USB Report Processed Sent
 		addons.PostprocessAddons(processed);
@@ -533,16 +530,17 @@ void GP2040::checkProcessedState(GamepadState prevState, GamepadState currState)
 
 void GP2040::checkSaveRebootState() {
 	if (saveRequested) {
-		if (rebootRequested) {
-			rebootRequested = false;
-			rebootDelayTimeout = make_timeout_time_ms(rebootDelayMs);
-		}
 		saveRequested = false;
 		Storage::getInstance().save(forceSave);
 	}
 
+	if (rebootRequested) {
+		rebootRequested = false;
+		rebootDelayTimeout = make_timeout_time_ms(rebootDelayMs);
+	}
+
 	if (!is_nil_time(rebootDelayTimeout) && time_reached(rebootDelayTimeout)) {
-		System::reboot(System::BootMode::DEFAULT);
+		System::reboot(rebootMode);
 	}
 }
 
@@ -550,8 +548,10 @@ void GP2040::handleStorageSave(GPEvent* e) {
     saveRequested = true;
 	forceSave = ((GPStorageSaveEvent*)e)->forceSave; 
     rebootRequested = ((GPStorageSaveEvent*)e)->restartAfterSave;
+	rebootMode = System::BootMode::DEFAULT;
 }
 
 void GP2040::handleSystemReboot(GPEvent* e) {
     rebootRequested = true;
+	rebootMode = ((GPRestartEvent*)e)->bootMode;
 }

src/storagemanager.cpp

@@ -18,6 +18,7 @@
 #include "config_utils.h"
 
 void Storage::init() {
+	systemFlashSize = System::getPhysicalFlash(); // System Flash Size must be called once
 	EEPROM.start();
 	ConfigUtils::load(config);
 }
@@ -121,16 +122,6 @@ void Storage::setFunctionalPinMappings()
 	}
 }
 
-void Storage::SetConfigMode(bool mode) { // hack for config mode
-	CONFIG_MODE = mode;
-	previewDisplayOptions = config.displayOptions;
-}
-
-bool Storage::GetConfigMode()
-{
-	return CONFIG_MODE;
-}
-
 void Storage::SetGamepad(Gamepad * newpad)
 {
 	gamepad = newpad;

src/webconfig.cpp

@@ -1,9 +1,8 @@
-#include "configs/webconfig.h"
 #include "config.pb.h"
-#include "configs/base64.h"
+#include "base64.h"
 
+#include "drivermanager.h"
 #include "storagemanager.h"
-#include "configmanager.h"
 #include "eventmanager.h"
 #include "layoutmanager.h"
 #include "peripheralmanager.h"
@@ -44,8 +43,6 @@ const static uint32_t rebootDelayMs = 500;
 static string http_post_uri;
 static char http_post_payload[LWIP_HTTPD_POST_MAX_PAYLOAD_LEN];
 static uint16_t http_post_payload_len = 0;
-static absolute_time_t rebootDelayTimeout = nil_time;
-static System::BootMode rebootMode = System::BootMode::DEFAULT;
 
 // Don't inline this function, we do not want to consume stack space in the calling function
 template <typename T, typename K>
@@ -211,23 +208,6 @@ static void __attribute__((noinline)) writeDoc(DynamicJsonDocument& doc, const K
 
 static int32_t cleanPin(int32_t pin) { return isValidPin(pin) ? pin : -1; }
 
-static uint32_t systemFlashSize;
-
-void WebConfig::setup() {
-    // System Flash Size must be called once
-    systemFlashSize = System::getPhysicalFlash();
-    rndis_init();
-}
-
-void WebConfig::loop() {
-    // rndis http server requires inline functions (non-class)
-    rndis_task();
-
-    if (!is_nil_time(rebootDelayTimeout) && time_reached(rebootDelayTimeout)) {
-        System::reboot(rebootMode);
-    }
-}
-
 enum class HttpStatusCode
 {
     _200,
@@ -2209,7 +2189,7 @@ std::string getMemoryReport()
     DynamicJsonDocument doc(capacity);
     writeDoc(doc, "totalFlash", System::getTotalFlash());
     writeDoc(doc, "usedFlash", System::getUsedFlash());
-    writeDoc(doc, "physicalFlash", systemFlashSize);
+    writeDoc(doc, "physicalFlash", Storage::getInstance().GetFlashSize());
     writeDoc(doc, "staticAllocs", System::getStaticAllocs());
     writeDoc(doc, "totalHeap", System::getTotalHeap());
     writeDoc(doc, "usedHeap", System::getUsedHeap());
@@ -2250,7 +2230,8 @@ std::string getHeldPins()
 
     uint32_t currentMillis = 0;
     while ((isAnyPinHeld || (((currentMillis = getMillis()) - timePinWait) < 5000))) { // 5 seconds of idle time
-        ConfigManager::getInstance().loop(); // Keep the loop going for interrupt call
+        // rndis http server requires inline functions (non-class)
+        rndis_task();
 
         if (_abortGetHeldPins)
             break;
@@ -2340,27 +2321,26 @@ std::string echo()
 }
 #endif
 
-std::string reboot()
-{
+// MUST MATCH NAVIGATION.JSX
+enum BOOT_MODES {
+	GAMEPAD = 0,
+	WEBCONFIG = 1,
+	BOOTSEL = 2,
+};
+
+std::string reboot() {
     DynamicJsonDocument doc = get_post_data();
-    doc["success"] = true;
-    // We need to wait for a bit before we actually reboot to leave the webclient some time to receive the response
-    rebootDelayTimeout = make_timeout_time_ms(rebootDelayMs);
-    WebConfig::BootModes bootMode = doc["bootMode"];
-    switch (bootMode) {
-        case WebConfig::BootModes::GAMEPAD:
-            rebootMode = System::BootMode::GAMEPAD;
-        break;
-        case WebConfig::BootModes::WEBCONFIG:
-            rebootMode = System::BootMode::WEBCONFIG;
-        break;
-        case WebConfig::BootModes::BOOTSEL:
-            rebootMode = System::BootMode::USB;
-        break;
-        default:
-            rebootMode = System::BootMode::DEFAULT;
+    uint32_t bootMode = doc["bootMode"];
+    System::BootMode systemBootMode = System::BootMode::DEFAULT;
+    if ( bootMode == BOOT_MODES::GAMEPAD ) {
+        systemBootMode = System::BootMode::GAMEPAD;
+    } else if ( bootMode == BOOT_MODES::WEBCONFIG ) {
+        systemBootMode = System::BootMode::WEBCONFIG;
+    } else if (bootMode == BOOT_MODES::BOOTSEL ) {
+        systemBootMode = System::BootMode::USB;
     }
-    EventManager::getInstance().triggerEvent(new GPRestartEvent(rebootMode));
+    EventManager::getInstance().triggerEvent(new GPRestartEvent((System::BootMode)systemBootMode));
+    doc["success"] = true;
     return serialize_json(doc);
 }