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

add a screensaver #621

Merged
merged 5 commits into from Oct 12, 2020
Merged

add a screensaver #621

merged 5 commits into from Oct 12, 2020

Conversation

benma
Copy link
Collaborator

@benma benma commented Oct 9, 2020

Currently set to ~1min until activation, see screen_saver.c - ACTIVE_AFTER.

The screensaver is reset with any touch or any screen_stack
change. The latter means there is something new for the user to see.

API calls per se do not reset the screensaver - only if they display
something via screen_stack.

The screensaver currently also appears if the user is in the middle of
a workflow or API call, if they do not interact with the device. It is
unclear if this is desirable, but in general it would work well
together with a general API timeout or automatic device lock.

Finally, some more thinking is required around the three main screens
and when and how to present them:

  1. "See the BitBoxApp" - currently shown until unlocked.
  2. Default waiting screen when nothing is going on. Shows the
    BitBox02 logo.
  3. Screensaver after inactivity. Currently it is an animated
    BitBox02 logo.

Currently it is confusing to leave the animated screensaver just to go
back to the same image logo.

@benma benma requested a review from x1ddos October 9, 2020 19:45
@benma benma force-pushed the screensaver branch 2 times, most recently from cc05f22 to 4c360a3 Compare October 9, 2020 23:47
@My1
Copy link
Contributor

My1 commented Oct 10, 2020

  • "See the BitBoxApp" - currently shown until unlocked.

not exactly precise after the last few tests I had at least before this PR. if you use U2F you unlock the device and get the see bitboxapp screen after using regardless, so it's not really a good indicator of it being locked.

@benma
Copy link
Collaborator Author

benma commented Oct 10, 2020

  • "See the BitBoxApp" - currently shown until unlocked.

not exactly precise after the last few tests I had at least before this PR. if you use U2F you unlock the device and get the see bitboxapp screen after using regardless, so it's not really a good indicator of it being locked.

Right, to be accurate: it is shown until a noise connection is requested by the host, which does not happen via U2F. This is inconsistent, good catch!

@My1
Copy link
Contributor

My1 commented Oct 10, 2020

honestly I am not even sure whether that's that useful info to the user, rather than the acutal lock state.
in fact I think personally at least that see see bitboxapp screen makes most sense on a reset device for new users but if you have own a device that you setup the bitbox app should be known.

@benma
Copy link
Collaborator Author

benma commented Oct 10, 2020

Currently it is confusing to leave the animated screensaver just to go back to the same image logo.

This is pretty much solved with the reduced brightness during the screensaver, which makes it clear that one left the screensaver.

@My1
Copy link
Contributor

My1 commented Oct 10, 2020

Currently it is confusing to leave the animated screensaver just to go back to the same image logo.

This is pretty much solved with the reduced brightness during the screensaver, which makes it clear that one left the screensaver.

it helps but if anyone wants or cares, I do have (a prototype of) a bouncing shiftcrypto logo similar to the screensavers of DVD Players back in the day

src/ui/screen_saver.c Outdated Show resolved Hide resolved
@benma benma force-pushed the screensaver branch 2 times, most recently from ad43cfd to 6644765 Compare October 11, 2020 11:02
@My1
Copy link
Contributor

My1 commented Oct 11, 2020

3. Screensaver after inactivity. Currently it is an animated
BitBox02 logo.

not anymore :-)

Copy link
Contributor

@x1ddos x1ddos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot, is it going into the next release?

src/ui/components/screensaver.c Outdated Show resolved Hide resolved
src/ui/components/screensaver.h Outdated Show resolved Hide resolved
src/ui/components/screensaver.c Show resolved Hide resolved
src/ui/components/screensaver.c Outdated Show resolved Hide resolved
src/ui/components/screensaver.c Outdated Show resolved Hide resolved
src/ui/components/screensaver.c Show resolved Hide resolved
src/ui/screen_saver.c Show resolved Hide resolved
benma and others added 5 commits October 12, 2020 01:04
Currently set to ~1min until activation, see screen_saver.c - ACTIVE_AFTER.

The screensaver is reset with any touch or any screen_stack
change. The latter means there is something new for the user to see.

API calls per se do not reset the screensaver - only if they display
something via screen_stack.

The screensaver currently also appears if the user is in the middle of
a workflow or API call, if they do not interact with the device. It is
unclear if this is desirable, but in general it would work well
together with a general API timeout or automatic device lock.

Animation: logo scrolls from left ouf of screen to right out of
screen, and wraps around one pixel lower. For a short while before the
logo is visible, the screen is black, so that the change to the
screensaver is not too quick.

Finally, some more thinking is required around the three main screens
and when and how to present them:

 1. "See the BitBoxApp" - currently shown until unlocked.
 2. Default waiting screen when nothing is going on. Shows the
      BitBox02 logo.
 3. Screensaver after inactivity. Currently it is an animated
      BitBox02 logo.

Currently it is confusing to leave the animated screensaver just to go
back to the same image logo.
@benma
Copy link
Collaborator Author

benma commented Oct 11, 2020

I forgot, is it going into the next release?

yes ;)

addressed all comments, PTAL

@My1
Copy link
Contributor

My1 commented Oct 11, 2020

what's PTAL?

Copy link
Contributor

@x1ddos x1ddos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice work you two! 🚀

@x1ddos
Copy link
Contributor

x1ddos commented Oct 12, 2020

@My1 PTAL = please take another look. LGTM = looks good to me. Just quicker to type.

@My1
Copy link
Contributor

My1 commented Oct 12, 2020

to be honest, caging the screensaver in like I did took me a bit lol. basically no matter where the thing is it will fly towards the screen, and when inside (even on just one axis) the bounce logic will activate for that axis or if in screen obviously constrained to that.
I got the Idea from the DVD screensavers btw.

@benma benma merged commit 99879c7 into digitalbitbox:master Oct 12, 2020
1 check passed
@My1
Copy link
Contributor

My1 commented Oct 12, 2020

Shiftcrypto logo
for reference, here the image used for the screensaver, not perfect but the best I could do in 17x20 full mono

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

Successfully merging this pull request may close these issues.

None yet

3 participants