Skip to content

Commit

Permalink
Restrict runtime saves (#1178)
Browse files Browse the repository at this point in the history
* only write the config to flash when it is safe to

the USB host port uses the flash, as does our config. both cores
accessing the flash at the same time is no good and will crash the host
port, so the host port core is locked when writing flash for saving the
config, but this disables interrupts on the port and may interfere with
auth, so this only saves at runtime when it is safe to (when USB host is
disabled), with an optional flag to force saving the config, which
should only be used in limited circumstances.

* always force config saves in the webconfig

it doesn't use the USB host port for anything so this should always be
fine

* add a hotkey to force a save of the config
  • Loading branch information
bsstephan authored Nov 23, 2024
1 parent 58348ce commit f127a6b
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 36 deletions.
1 change: 1 addition & 0 deletions headers/storagemanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Storage {

void init();
bool save();
bool save(const bool force);

// Perform saves that were enqueued from core1
void performEnqueuedSaves();
Expand Down
1 change: 1 addition & 0 deletions proto/enums.proto
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ enum GamepadHotkey
HOTKEY_DPAD_LEFT = 40;
HOTKEY_DPAD_RIGHT = 41;
HOTKEY_PREVIOUS_PROFILE = 42;
HOTKEY_SAVE_CONFIG = 43;
}

// This has to be kept in sync with LEDFormat in NeoPico.hpp
Expand Down
6 changes: 0 additions & 6 deletions src/configmanager.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#include "configmanager.h"

#include "addonmanager.h"
#include "configs/webconfig.h"
#include "addons/neopicoleds.h"

void ConfigManager::setup(ConfigType config) {
if (config == CONFIG_TYPE_WEB)
Expand All @@ -19,7 +17,3 @@ void ConfigManager::setupConfig(GPConfig * gpconfig) {
gpconfig->setup();
this->config = gpconfig;
}

void ConfigManager::setGamepadOptions(Gamepad* gamepad) {
gamepad->save();
}
32 changes: 16 additions & 16 deletions src/configs/webconfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ std::string setDisplayOptions(DisplayOptions& displayOptions)
std::string setDisplayOptions()
{
std::string response = setDisplayOptions(Storage::getInstance().getDisplayOptions());
Storage::getInstance().save();
Storage::getInstance().save(true);
return response;
}

Expand Down Expand Up @@ -533,7 +533,7 @@ std::string setSplashImage()
memcpy(displayOptions.splashImage.bytes, decoded.data(), length);
displayOptions.splashImage.size = length;

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -578,7 +578,7 @@ std::string setProfileOptions()
if (altsIndex > 2) break;
}

Storage::getInstance().save();
Storage::getInstance().save(true);
return serialize_json(doc);
}

Expand Down Expand Up @@ -688,7 +688,7 @@ std::string setGamepadOptions()
ForcedSetupOptions& forcedSetupOptions = Storage::getInstance().getForcedSetupOptions();
readDoc(forcedSetupOptions.mode, doc, "forcedSetupMode");

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -800,7 +800,7 @@ std::string setLedOptions()
readDoc(ledOptions.pledIndex4, doc, "pledIndex4");
readDoc(ledOptions.pledColor, doc, "pledColor");

Storage::getInstance().save();
Storage::getInstance().save(true);
return serialize_json(doc);
}

Expand Down Expand Up @@ -1005,7 +1005,7 @@ std::string setCustomTheme()
options.buttonPressColorCooldownTimeInMs = pressCooldown;

AnimationStation::SetOptions(options);
AnimationStore.save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1081,7 +1081,7 @@ std::string setPinMappings()
gpioMappings.profileLabel[profileLabelSize - 1] = '\0';
gpioMappings.enabled = doc["enabled"];

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1175,7 +1175,7 @@ std::string setKeyMappings()
readDoc(keyboardMapping.keyButtonE11, doc, "E11");
readDoc(keyboardMapping.keyButtonE12, doc, "E12");

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1335,7 +1335,7 @@ std::string setPeripheralOptions()
profiles.gpioMappingsSets[2].pins[oldPinDplus+adjacent].action = GpioAction::NONE;
}

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1401,7 +1401,7 @@ std::string setExpansionPins()
}
Storage::getInstance().getAddonOptions().pcf8575Options.pins_count = 16;

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1435,7 +1435,7 @@ std::string setReactiveLEDs()
}
Storage::getInstance().getAddonOptions().reactiveLEDOptions.leds_count = 10;

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1628,7 +1628,7 @@ std::string setAddonOptions()
docToValue(drv8833RumbleOptions.dutyMin, doc, "drv8833RumbleDutyMin");
docToValue(drv8833RumbleOptions.dutyMax, doc, "drv8833RumbleDutyMax");

Storage::getInstance().save();
Storage::getInstance().save(true);

return serialize_json(doc);
}
Expand Down Expand Up @@ -1701,7 +1701,7 @@ std::string setPS4Options()
if (ps4Options.rsaQP.size != 0) ps4Options.rsaQP.size = 0;
if (ps4Options.rsaRN.size != 0) ps4Options.rsaRN.size = 0;

Storage::getInstance().save();
Storage::getInstance().save(true);

return "{\"success\":true}";
}
Expand Down Expand Up @@ -1784,7 +1784,7 @@ std::string setWiiControls()
readDoc(wiiOptions.controllers.turntable.effects.axisType, doc, "turntable.analogEffects.axisType");
readDoc(wiiOptions.controllers.turntable.fader.axisType, doc, "turntable.analogFader.axisType");

Storage::getInstance().save();
Storage::getInstance().save(true);

return "{\"success\":true}";
}
Expand Down Expand Up @@ -2106,7 +2106,7 @@ std::string setMacroAddonOptions()

macroOptions.macroList_count = MAX_MACRO_LIMIT;

Storage::getInstance().save();
Storage::getInstance().save(true);
return serialize_json(doc);
}

Expand Down Expand Up @@ -2254,7 +2254,7 @@ DataAndStatusCode setConfig()
{
Storage::getInstance().getConfig() = *config.get();
config.reset();
if (Storage::getInstance().save())
if (Storage::getInstance().save(true))
{
return DataAndStatusCode(getConfig(), HttpStatusCode::_200);
}
Expand Down
10 changes: 4 additions & 6 deletions src/gamepad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,6 @@ void Gamepad::read()
state.rt = 0;
}

void Gamepad::save()
{
Storage::getInstance().save();
}

void Gamepad::hotkey()
{
if (options.lockHotkeys)
Expand Down Expand Up @@ -500,6 +495,9 @@ void Gamepad::processHotkeyAction(GamepadHotkey action) {
case HOTKEY_REBOOT_DEFAULT:
System::reboot(System::BootMode::DEFAULT);
break;
case HOTKEY_SAVE_CONFIG:
Storage::getInstance().save(true);
break;
case HOTKEY_CAPTURE_BUTTON:
state.buttons |= GAMEPAD_MASK_A2;
break;
Expand Down Expand Up @@ -583,6 +581,6 @@ void Gamepad::processHotkeyAction(GamepadHotkey action) {

// only save if requested
if (reqSave) {
save();
Storage::getInstance().save();
}
}
4 changes: 3 additions & 1 deletion src/gp2040.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ void GP2040::setup() {
// Save the changed input mode
if (inputMode != gamepad->getOptions().inputMode) {
gamepad->setInputMode(inputMode);
gamepad->save();
// 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);
}
}

Expand Down
23 changes: 16 additions & 7 deletions src/storagemanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,38 @@

#include "BoardConfig.h"
#include "AnimationStorage.hpp"
#include "Effects/StaticColor.hpp"
#include "FlashPROM.h"
#include "peripheralmanager.h"
#include "config.pb.h"
#include "hardware/watchdog.h"
#include "Animation.hpp"
#include "CRC32.h"
#include "types.h"

#include "config_utils.h"

#include "bitmaps.h"

#include "helper.h"

void Storage::init() {
EEPROM.start();
critical_section_init(&animationOptionsCs);
ConfigUtils::load(config);
}

/**
* @brief Save the config, but only if it is safe to (as in USB host is not being used.)
*/
bool Storage::save()
{
return ConfigUtils::save(config);
return save(false);
}

/**
* @brief Save the config; if forcing a save is requested, or if USB host is not enabled, this will write to flash.
*/
bool Storage::save(const bool force) {
if (!PeripheralManager::getInstance().isUSBEnabled(0) || force) {
return ConfigUtils::save(config);
} else {
return false;
}
}

static void updateAnimationOptionsProto(const AnimationOptions& options)
Expand Down
1 change: 1 addition & 0 deletions www/src/Locales/en/SettingsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export default {
'load-profile-3': 'Load Profile #3',
'load-profile-4': 'Load Profile #4',
'reboot-default': 'Reboot GP2040-CE',
'save-config': 'Save Config',
'next-profile': 'Next Profile',
'previous-profile': 'Previous Profile',
},
Expand Down
1 change: 1 addition & 0 deletions www/src/Pages/SettingsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ const HOTKEY_ACTIONS = [
{ labelKey: 'hotkey-actions.r3-button', value: 20 },
{ labelKey: 'hotkey-actions.touchpad-button', value: 21 },
{ labelKey: 'hotkey-actions.reboot-default', value: 22 },
{ labelKey: 'hotkey-actions.save-config', value: 43 },
{ labelKey: 'hotkey-actions.b1-button', value: 23 },
{ labelKey: 'hotkey-actions.b2-button', value: 24 },
{ labelKey: 'hotkey-actions.b3-button', value: 25 },
Expand Down

0 comments on commit f127a6b

Please sign in to comment.