search

sparrowwallet / sparrow

Desktop Bitcoin Wallet focused on security and privacy. Free and open source.
https://sparrowwallet.com/
Apache License 2.0
1.36k stars 192 forks source link

Krux - BBQr densities #1502

Closed odudex closed 2 months ago

odudex commented 2 months ago

Thank you for adding BBQr option for Krux on 2.0.0! There's a small adjust that would improve UX for next versions: Make BBQr QR code high and low densities equal to UR densities. Current BBQr low density/version is at the limit of what Krux devices can handle, if camera focus is well adjusted. Current UR densities are great. A well adjusted camera will be able to scan UR with increased density, while a non adjusted camera can still work with UR decreased density.

craigraw commented 2 months ago

Thanks for the feedback.

The trouble with doing this is the difference to the Coldcard Q1. It would make scanning on that device with high density up to 5 times longer, and on low density up to 12 times longer. It would also introduce animated QRs much more frequently - one of the advantages of BBQr is a static QR is often used. I understand the Q1 has an expensive and very capable scanner, but it's quite a tradeoff for what is (as I understand it) not the default QR format for Krux devices.

Maybe I could drop the low density option a little. If you can run from source and experiment with the 1000 value at https://github.com/sparrowwallet/sparrow/blob/e1dab3a48e796e00c2c01d6f3cf8d023fec15d61/src/main/java/com/sparrowwallet/sparrow/control/QRDensity.java#L5 it would be useful.

odudex commented 2 months ago

Got it! I don’t want to make it less optimized for Coldcard Q1, nor add excessive code overhead with device-specific settings. I’ll try to play around with the code and see if I can find an idea with fewer tradeoffs.