feat: Defer reboot when changing settings #28
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When changing any settings that involve sending a message to
AdminModule
, the device immediately reboots, which is very annoying, and a frustrating user experience. This change modifies the behavior to only reboot either when the "Apply & Reboot" button is pressed, or when the user exits the settings menu.User flows
Before
Open the Settings tab -> Tap on Region -> Change to US -> Click OK -> Device reboots
After
Open the Settings tab -> Tap on Region -> Change to US -> Click OK -> Tap on Device Role -> Change to Client -> Click OK -> Click "Apply & Reboot" -> Device reboots
Open the Settings tab -> Tap on Region -> Change to US -> Click OK -> Tap on Device Role -> Change to Client -> Click OK -> Open the Home tab -> Device reboots
Open the Settings tab -> Tap on Region -> Change to US -> Click OK -> Tap on Language -> Change to German -> Click OK -> Device reboots
Screenshots:
The "Apply & Reboot" button if no changes are made (disabled). I decided to put the button on top of the menu so that the user immediately notices it, and is therefore aware that changes are not automatically applied.
The "Apply & Reboot" button if a change is made which would require a reboot (user name, region, role, modem, ...). The banner on top changes text to "Unsaved changes ..." to let the user know that changes will be lost if power is turned off.
A message pops up and the device reboots when the "Apply & Reboot" button is pressed.
The same thing also happens if the user leaves the Settings page. A more user-friendly approach would be to pop up a dialog asking to apply&reboot, or discard changes. (TODO?)
Technical details
The
AdminModule
API supports opening a "config transaction", during which multiple config changes can be made without rebooting the device, and the changes are only applied once the transaction is closed. This is done withmeshtastic_AdminMessage_begin_edit_settings_tag
andmeshtastic_AdminMessage_commit_edit_settings_tag
.I added two wrapper functions in
ViewController
:openConfigTransaction(uint32_t nodeId)
andcloseConfigTransaction(uint32_t nodeId)
.A config transaction is opened when a config parameter is changed. This is done by calling
TFTView_320x240::configTransactionOpen()
before callingsendConfig
, which also takes care of enabling the "Apply" button. Currently, I only implemented this for the following config items:WLAN, User name, Device role, Region, Modem preset, Alert buzzer, Language
.TFTView_320x240::configTransactionOpen()
should be added to any future config items tat get implemented. Local changes that don't involveAdminModule
don't need to open a config transaction.A config transaction can be closed in one of two (well, three) ways: by clicking on the "Apply & Reset" button, or by clicking on one of the buttons for selecting a different home screen tab (Home, Maps, Messages, ...). This is to prevent users from trying to use the device while the settings are not applied. The transaction is also immediately closed when the language is changed, since we want the change to be effective immediately (and a user might not understand what to do next until the language is changed). Either way, this is done by calling
TFTView_320x240::checkForConfigTransactionClose()
, which checks if any changes were done which require a reboot. If such changes were made, it pops up a warning that the device is about to reboot, and callsViewController::closeConfigTransaction()
.Notably, a config transaction cannot be cancelled once it's opened (or at least I couldn't find a way to do so), which complicates a potential implementation of a "Do you want to save or discard changes?" dialog. Any changes made would need to be buffered in the GUI, and only once the user hits "Apply" would the GUI open a config transaction, issue multiple
sendConfig
calls, and close the config transaction. This is how most config dialogs work, and would be the most user-friendly way to do things, so I think it should be considered a future TODO.I did my best to google-translate the three newly added messages to the other implemented languages, but I have no way of verifying how good the translations are.
Testing
I tested this using the
native-tft
platform, as I'm still waiting for my T-Deck to arrive. I went through all the basic settings in all the languages. Some languages have text that is too long for the pop up message at the default resolution, but I don't know enough about languages like Finnish to know how to properly shorten words.