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

Hold QR animation on brightness tips and reset animated QR sequence #495

Merged

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Sep 23, 2023

Depends on #494

This PR actually only has minimal changes; to get a proper diff you need to compare against https://github.com/kdmukai/seedsigner/tree/qr_encoding_refactor

Description

For fountain encoders, the first n frames are extremely valuable. But since we start the animated QR sequence with the brightness tip overlaid, many of the early frames will be wasted and/or unreadable.

This change pauses the current frame whenever the brightness tip is visible and then restarts the animated QR sequence (per @jdlcdl's suggestion) when the brightness tip deactivates.

Fountain encoders: should be beneficial
"Simple" animated encoders (Specter xpub): doesn't really matter
Static encoders: no effect

This pull request is categorized as a:

  • New feature (really a behavior change)

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • N/A

If this evolves to support more elaborate changes to the fountain encoder sequence, then dedicated tests will be added.

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

@kdmukai kdmukai force-pushed the hold_qr_animation_on_brightness_tips branch from 37bd999 to eb3c101 Compare March 3, 2024 21:38
@kdmukai kdmukai force-pushed the hold_qr_animation_on_brightness_tips branch from eb3c101 to 2653c8e Compare July 3, 2024 18:10
@kdmukai kdmukai marked this pull request as ready for review July 3, 2024 18:22
@kdmukai kdmukai changed the title [DRAFT] Hold QR animation on brightness tips and reset animated QR sequence Hold QR animation on brightness tips and reset animated QR sequence Jul 3, 2024
@jdlcdl
Copy link

jdlcdl commented Jul 3, 2024

ACK tested as of commit 2653c8e

This worked for me, rather nicely especially when late about getting the camera focused on seedsigner screen.
I see all tests passed above, they passed on a linux desktop and also on my rpi0 dev setup.

@newtonick
Copy link
Collaborator

ACK and tested without any issues

@newtonick newtonick merged commit 2b42c43 into SeedSigner:dev Jul 4, 2024
1 check passed
@kdmukai kdmukai deleted the hold_qr_animation_on_brightness_tips branch September 2, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged Not Yet Released
Development

Successfully merging this pull request may close these issues.

3 participants