Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Report: Enabling Persistent Settings w/ No MicroSD Inserted Produces Error #457

Closed
SeedSigner opened this issue Aug 27, 2023 · 3 comments · Fixed by #464
Closed

Bug Report: Enabling Persistent Settings w/ No MicroSD Inserted Produces Error #457

SeedSigner opened this issue Aug 27, 2023 · 3 comments · Fixed by #464

Comments

@SeedSigner
Copy link
Owner

Steps to reproduce: Remove microSD card, scan any SettingsQR that includes enabling persistent settings.

Error produced includes text "persistent = 'E' is not valid"

Desired behavior: A more informative error message that either describes the specific issue in language more accessible to an end user, and/or directs the user to insert the microSD card if they wish to enable persistent settings.

@kdmukai
Copy link
Contributor

kdmukai commented Aug 29, 2023

I would prefer to just silently ignore that setting in that scenario. Thoughts?

@kdmukai
Copy link
Contributor

kdmukai commented Aug 29, 2023

Proposed fix via the "just ignore it" approach:

settings_views.SettingsIngestSettingsQRView:

        self.config_name, settings_update_dict = Settings.parse_settingsqr(data)

        persistent_settings = settings_update_dict.get(SettingsConstants.SETTING__PERSISTENT_SETTINGS)
        if persistent_settings == SettingsConstants.OPTION__ENABLED and not MicroSD.get_instance().is_inserted:
            # SettingsQR wants to enable persistent settings, but no MicroSD is inserted.
            # For the sake of simplicity we just ignore that setting for now.
            # TODO: Can consider a warning screen instead that gives the user some options.
            del settings_update_dict[SettingsConstants.SETTING__PERSISTENT_SETTINGS]
            
        self.settings.update(settings_update_dict)

@jdlcdl
Copy link

jdlcdl commented Aug 29, 2023

I support ignoring persistent settings for SettingsQR because its appeal to me is to have custom settings for a device that remembers absolutely nothing when powered off. I can still have what I want without a fix, I'll just be careful never to enable persistent settings.

If I only had a time machine, I'd go back and argue that persistent settings, done in a manner where they could be enabled permanently so that seedsigner is writing to the microsd:

  • just because I changed a setting or
  • just because I inserted my microsd that had a settings.json file,
  • or just because I scanned a SettingsQR with an extra E,
    ... instead of forcing me to knowingly take a deliberate action like "Save all settings to microsd now!", is too much "magic" (as I've opined in the past) for me.

My bias is based on the fact that I don't want anything being written to my microsd unless Im deliberately asking for that to happen. I want to be able to dd my microsd frequently to get the hash of what's on it. I understand that others will disagree. This is just my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants