SeedSigner / seedsigner

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

[Screenshot Generator] Add explicit Toast `duration`, additional code comments #613

Open kdmukai opened 2 months ago

kdmukai commented 2 months ago

Description

Light refactor of RemoveSDCardToastManagerThread to give the screenshot generator more explicit control over that Toast.

Screenshot generator now sets RemoveSDCardToastManagerThread.duration=0 to guarantee that the "infinite" Toast would never block during the screenshot generation. It was succeeding (not blocking) before because MicroSD.is_inserted checks Settings.HOSTNAME == Settings.SEEDSIGNER_OS, but this took too much digging to understand that Toast's behavior in the screenshot generator.

How the Toast screenshots are rendered at all is quite confusing so additional step-by-step explanatory comments were added.


This pull request is categorized as a:

Checklist

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

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

jdlcdl commented 2 months ago

Viewed code changes, which I like, but have not yet tested.

alvroble commented 2 months ago

Looks neat @kdmukai! I noticed this behavior in test case while developing PR https://github.com/SeedSigner/seedsigner/pull/600. From what I understand, setting it to 0 in the test cases, or potentially a different value in future developments, seems like a solid approach, so ACK for me. I will update that PR to prevent conflicts if this one gets merged, as I also have changes in this file: https://github.com/SeedSigner/seedsigner/pull/600/files#diff-887aac80585a20b298dabebe9d8fc3b36151a83b79f30c963f79f1585dae96e2