-
Notifications
You must be signed in to change notification settings - Fork 131
[WOOMOB-547] Add Heuristics for differentiating between barcode scanner and HW keyboard #14162
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
[WOOMOB-547] Add Heuristics for differentiating between barcode scanner and HW keyboard #14162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a dedicated BarcodeInputDetector
to distinguish fast scanner input from manual keyboard entry and integrates it into a Compose modifier, accompanied by new unit tests.
- Extract barcode collection and timing logic into
BarcodeInputDetector
with configurable timeouts and clear/reset behavior. - Update
listenForBarcodes
modifier to use the detector, handle lifecycle cleanup, and run timeouts viaCoroutineScope
. - Add comprehensive tests covering fast vs. slow input, invalid chars, timeouts, and consecutive scans.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/composeui/modifier/BarcodeScannerModifier.kt | Refactored modifier to use BarcodeInputDetector with coroutine-based timeouts and added disposal logic |
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/composeui/modifier/BarcodeInputDetectorTest.kt | Added unit tests simulating scanner vs. human typing, invalid inputs, and timeout behaviors |
...tlin/com/woocommerce/android/ui/woopos/common/composeui/modifier/BarcodeInputDetectorTest.kt
Outdated
Show resolved
Hide resolved
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14162 +/- ##
============================================
+ Coverage 37.87% 37.90% +0.02%
Complexity 8959 8959
============================================
Files 1956 1956
Lines 109225 109274 +49
Branches 14305 14313 +8
============================================
+ Hits 41367 41417 +50
+ Misses 64085 64082 -3
- Partials 3773 3775 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks for the ping @malinajirka The heuristics as you describe them look good. My only question is why 4 characters?
UPC-E is the shortest of the common symbologies we're looking to support, and that uses 6 characters. Is there some other use case I'm missing for shorter codes? That said, with custom barcodes in QR codes, I guess someone could have shorter codes. I think it comes down to: what's going to give us the best accuracy in scanning, without compromising common barcode format support? |
companion object { | ||
@Suppress("SpellCheckingInspection") | ||
const val ALLOWED_BARCODE_CHARS = | ||
"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-._~:/?#[]@!$&'()*+,;=" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the source for this character set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I realized it doesn't include all the characters supported by some of the allowed symbologies so I removed this check completely 3e5c772 - for example Code 39 Ext. or Code 128 support all ASCII characters, QR codes support even more.
Thanks for looking into this @joshheald !
As you mentioned for example QR codes support codes of any length, however, I don't think we can come up with a heuristic which would reliable allow such scans. I tried to find a reasonably but as short as possible length to give us decent accuracy. Wdyt? |
private val coroutineScope: CoroutineScope, | ||
private val currentTimeProvider: CurrentTimeProvider, | ||
) { | ||
companion object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np: probably should be private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in db700f6
Looks good, but I don't understand what specifically it gives as? That if a new line char is not coming, we'll still submit it as a barcode? The search in focus will still put scanned data in it, correct? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good but I am 100% understand what it is for 😀. I asked it in the comment
Thanks for the review!
It gives us two things:
|
WOOMOB-373
Description
This PR introduces the BarcodeInputDetector class that helps differentiate between input from a hardware keyboard and input from a barcode scanner.
How It Works:
The detector differentiates between manual keyboard typing and automatic barcode scanner input by analyzing timing patterns:
Time-based detection:
Uses two timing thresholds
Character validation: Only allows alphanumeric and common special characters
Minimum length: Rejects potential barcodes shorter than 4 characters
Auto-completion: Scan completes on Enter key or after timeout
Reset mechanism: Automatically resets state when human typing is detected
Testing information
The tests that have been performed
Images/gif
No user-facing changes.
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.cc @joshheald