cryptoadvance / specter-diy

DIY airgapped hardware wallet that uses QR codes for communication with the host
MIT License
441 stars 73 forks source link

gui: animated qr #55

Closed gorazdko closed 4 years ago

gorazdko commented 4 years ago

close #48

Status

Ready for review

Edit

New implementation force pushed which resembles https://github.com/cryptoadvance/specter-desktop/pull/104

stepansnigirev commented 4 years ago

I think it would be better to extend QRCode class defined in https://github.com/cryptoadvance/specter-diy/blob/master/src/gui/components.py#L15

We could add a method to set max size, and if QR code text is larger than that size - add two arrows on the right and left sides of the qr code. When you click on the arrow - next / prev part of the QR code is shown. Label under the QR code could show current and total number of qr codes.

What do you think?

stepansnigirev commented 4 years ago

Maybe something like this:

Screenshot 2020-03-14 at 14 22 12
gorazdko commented 4 years ago

I force pushed the implementation which now works the same as https://github.com/cryptoadvance/specter-desktop/pull/104

What should we do with animations in simulator mode? It doesnt look to have a timer.

stepansnigirev commented 4 years ago

What should we do with animations in simulator mode? It doesnt look to have a timer.

We can write a simple scheduler where we can register / unregister callbacks. Maybe even look at asyncio. I think we can do the same both in simulator and on the board itself. At the moment we have something very similar to asyncio in ioloop() function - we constantly call .update() function on everything that requires updates.

Here is a pure python implementation: https://github.com/micropython/micropython-lib/blob/master/asyncio_slow/asyncio_slow.py

Recently uasyncio was added to micropython: https://github.com/micropython/micropython/pull/5332

We can rebase to the latest version of micropython and check that nothing breaks, but I think we should start with pure python implementation.

gorazdko commented 4 years ago

Maybe even look at asyncio

For now I just used ticks.diff for simulator and for stm32 with the .update function, so the code is the same. Is that ok?

In simulator i notice this qr width glitching, it doesnt happen on hardware though:

qrA

Its a consequence of the fact that i have to adapt the qr width after i draw it. Will ponder on this one a bit...

stepansnigirev commented 4 years ago

Looks good! I will test a bit and merge :)

stepansnigirev commented 4 years ago

Tested works great, I didn't see any width glitches in the simulator. On hardware the close button becomes a bit laggy but I think it's ok, we can live with it.

With https://github.com/cryptoadvance/specter-desktop/pull/118 we can use it in specter-desktop.

Now we can sign and display large transactions! Awesome!

gorazdko commented 4 years ago

On hardware the close button becomes a bit laggy but I think it's ok, we can live with it

You mean pressing OK button while animation is running? Weird, I havent noticed that at all. Mine has never lagged and works blazingly fast.

k9ert commented 4 years ago

This should also go into the release, i guess? I'd suggest to quickly merge it and then test everything together?!

stepansnigirev commented 4 years ago

I agree, works great, I don't see a reason not to merge :)