search

SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
691 stars 158 forks source link

[Enhancement] MicroSD Toast Timer #546

Open easyuxd opened 4 months ago

easyuxd commented 4 months ago

Updated Design:

Provide 3 microSD options:

  1. Toast disabled
  2. 5-sec toast (default)
  3. SD card removal required* (fullscreen dialog, cannot proceed unless microSD is removed)

Proposed Enhancements:

1. Add automatic dismissal of MicroSD Toast message.

Rationale:

2. Add advanced setting to disable toast

Rationale:

Proposed Design

5s timer:

SD-Snackbar

alvroble commented 1 month ago

Looking into RemoveSDCardToastManagerThread, wouldn't this be easily achievable by modifying the supperclass init call?

https://github.com/SeedSigner/seedsigner/blob/0f329f6fb568f1c7aaae6288ce33e38c566f3d1d/src/seedsigner/gui/toast.py#L189-L196

Setting duration=5 would provide the requested behavior. I tested it with this change in Rapberry Pi OS build and it worked as expected. A question for other devs: do you think this change could create any side effects?

jdlcdl commented 1 month ago

In the lead-up to v0.7.0, this was such a big deal (that user could remove any capability for secrets to be stored... by a software that is already super-paranoid about never storing any secrets in the first place) that many (including me) advocated a scary warning screen to entice the user to remove the sdcard.

The only way that secrets would get written, is if the evil maid got to the sdcard, and then removing it wouldn't be helpful because she'd just have you sign every tx to her own wallet... whether or not the sdcard was removed.

Back to your question, I believe that your proposed change of altering the duration is all that is necessary, and that Keith foresaw this in his design of this thread. Next question to overcome... since the warning goes away as soon as there is user activity... do we want the user NOT to see this in the event they plug it in and forget to pay attention (instead of dealing with their wallet coordinator while it boots) for the next 20s? Truth is, they'll pay attention at least a few times and they'll already know that the sdcard can be removed; 5s of this will make the feature well known and will be less-likely to become the featured "project photo" that is so common since v0.7.0 (unless that is the intention of the photographer).

easyuxd commented 1 month ago

@jdlcdl

Very thoughtful, as always. If the user HAD to perform a task here in order to proceed, I think it could warrant a persistent message.

But as you said, occasionally missing the message is an edge case, and several seconds is long enough to catch it most times (especially considering the boot time is much faster now). At present, the UI is obfuscated with a non-critical message.

alvroble commented 1 month ago

Thank you for your thorough comments. Maybe there should be added some personalization in the tools menu? I was thinking of some default behavior, let's say 5s timer. But if the user just wants just to let it show until he removes the SD card (maybe they just plug the device and forget for some minutes, but want to be sure they can remove it when getting in action) just having a checkbox in the settings for a persistent warning could cover this use case also. Maybe even this could be a blocking warning, so the user could force themselves to unplug the SD card each time.

Maybe we are making such a big deal of this, but I believe it’s worth considering. This is one of the user’s first interactions with the device each time they power it on.

easyuxd commented 1 month ago

Love your recommendation, @alvroble. An advanced menu setting would be great. Everyone can be appeased that way. Options something like?

kdmukai commented 1 month ago

There was also talk at one point of having an SD card status icon, perhaps just on the Home screen, to indicate if the SD card was present or not (e.g. green SD card icon at top left, gray if it's removed?).

That might satisfy the most hard-core paranoids since they can just glance at that corner and reassure themselves via the icon that they haven't forgotten to remove the SD card. But it's quiet and innocuous so that less savvy users would barely even notice it.

It would help to think through the matrix of what types of different people we are designing around (first time noobs, experienced users, max paranoids) and what approach best suits each type w/minimal interference or confusion for the other types (i.e. there's almost definitely no one-size-fits-all perfect solution here, but probably a more-than-good-enough solution for everyone).

AND / OR there was also previous discussion about an option to require SD card removal before proceeding. Obviously the best choice for the max paranoids. Also probably makes the SD icon unnecessary. I can't recall if there was any reason why we didn't implement that as an option.

kdmukai commented 1 month ago

In which case I'd modify @easyuxd's suggestion above to something more like:

I don't think there's a need for an infinite persistent option for the toast.

*(probably need an escape hatch on that View if we know we're running Raspi OS (i.e. a dev environment) that doesn't actually support SD card removal)

easyuxd commented 1 month ago

@kdmukai I'm on board. These 3 options cover all the bases nicely.

It would help to think through the matrix of what types of different people we are designing around (first time noobs, experienced users, max paranoids) and what approach best suits each type w/minimal interference or confusion for the other types (i.e. there's almost definitely no one-size-fits-all perfect solution here, but probably a more-than-good-enough solution for everyone).

Exactly. This highlights the need for user personas. I'll start working on this.

alvroble commented 1 month ago

I completely agree @kdmukai!

@easyuxd let me know if I can help on this, as I have time to contribute more. I'll keep an eye on what you propose.

jdlcdl commented 1 week ago

Today, someone in the main telegram group expressed interest in a settings choice that forces them to remove the sdcard when using seedsigner.

easyuxd commented 1 week ago

I'm working on some user research (it will take a little while), but think it's safe to go with Keith's 3 options we've agreed on.

@alvroble Do you still want to tackle this PR? I can help with the fullscreen dialog for #3 if needed.

alvroble commented 1 week ago

@easyuxd I will try to build something around. Regarding the fullscreen dialog, as soon as I have the logic flow ready I will submit a first UI iteration so you can then propose improvements.

Semisol commented 6 days ago

SD card removal required* (fullscreen dialog, cannot proceed unless microSD is removed)

There should be an option to go to settings to disable this feature, or to edit other persistent settings before removal of the SD card.

alvroble commented 5 days ago

@Semisol, the default behavior would be a 5-second timer, allowing anyone to change any persistent settings before this one. I don't really understand the problem you mention.

I understand what you said now—I encountered the issue while testing my code for this feature (it's not a PR yet, but it's in my own SS repo). If the "SD card removal required" option is set, there won't be any way to unset it the next time. To exit the fullscreen dialog and access the UI, the SD card must be removed, preventing the setting from being changed to any other options. Thank you for pointing this out; I didn't notice it until I tested it.

I agree that there should be a way to unset this option without having to write to the SD card, specially since currently, the only way to change this setting in my SS is to wait 11 days (1e6 seconds) 🥴. Implementing a large timer, such as 5 or 10 minutes, could be a good alternative, allowing the user to wait briefly if they wish to change the setting. This longer wait time could also be mentioned in the permanent dialog.

Semisol commented 5 days ago

@Semisol, ~the default behavior would be a 5-second timer, allowing anyone to change any persistent settings before this one. I don't really understand the problem you mention.~

I understand what you said now—I encountered the issue while testing my code for this feature (it's not a PR yet, but it's in my own SS repo). If the "SD card removal required" option is set, there won't be any way to unset it the next time. To exit the fullscreen dialog and access the UI, the SD card must be removed, preventing the setting from being changed to any other options. Thank you for pointing this out; I didn't notice it until I tested it.

I agree that there should be a way to unset this option without having to write to the SD card, specially since currently, the only way to change this setting in my SS is to wait 11 days (1e6 seconds) 🥴. Implementing a large timer, such as 5 or 10 minutes, could be a good alternative, allowing the user to wait briefly if they wish to change the setting. This longer wait time could also be mentioned in the permanent dialog.

The dialog should have the option to go to settings, no way to exit without either changing the setting or removing the card. This also allows you to configure things like coordinators persistently before removal.

alvroble commented 4 days ago

I'm thinking of two alternative solutions for this catch-22 situation:

  1. Use the Raspberry Pi’s internal storage (e.g., /boot) to save a minimal config file that only stores the SD card setting. This requires modifying how settings are accessed and stored, and it might imply a slightly complex refactor.
  2. Define a specific key combination or button press sequence during the full screen dialog to exit it. This would let the user access the UI and modify settings as needed. However it must be documented for the user, maybe in the dialog itself.
  3. In my other comment I proposed another option:

    Implementing a large timer, such as 5 or 10 minutes, could be a good alternative, allowing the user to wait briefly if they wish to change the setting. This longer wait time could also be mentioned in the permanent dialog.

I believe the second one would be better since it doesn't mix the settings writing/reading, although maybe the first approach is a good alternative to write all the settings for the SeedSigner OS builds.

I'd love some input here from other devs so I can redirect the coding to one of these solutions (or any other one proposed)

easyuxd commented 4 days ago

@alvroble Could it be as simple as what @Semisol proposed -- make Option 3 (microSD removal required) a fullscreen dialog add a button to go to the specific settings screen? Maybe I'm not seeing the benefit of having a wait time or special key combo if the setting can eventually be changed anyway.

remove-memory-card
alvroble commented 4 days ago

@easyuxd thanks for your comment. The problem here is that in my branch I actually use a fullscreen toast, so the remove SD card flow is unaltered. This would imply a different approach, as we'd talk about a view and not a toast. Maybe a more complex refactor. I'll check on it.

newtonick commented 8 hours ago

@alvroble Could it be as simple as what @Semisol proposed -- make Option 3 (microSD removal required) a fullscreen dialog add a button to go to the specific settings screen? Maybe I'm not seeing the benefit of having a wait time or special key combo if the setting can eventually be changed anyway.

remove-memory-card

I like this better for option 3 as well. Would be ideal if it jumped straight to the specific settings screen when the "settings" button is selection. Then when the user selects the back "<" arrow, the "Remove Memory Card" would re-evaluate the state of things and proceed accordingly. I'd have to dive into the code to see if jumping straight to a specific setting is possible.

SettingsEntryUpdateSelectionView_microsd_toast_timer