Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jul 26, 2025

Description

WOOMOB-765
Related Android PR: woocommerce/woocommerce-android#14255

If there's an unfinished scan, show the error row after waiting for the last inter-character timeout, not when the next character is received.

Solution

  1. Add a timer in a barcode parser
  2. If the inter-character timeout is triggered, show an error
  3. Reset the timer if the scan is processed, canceled, or new character input is made

Steps to reproduce

Prerequisites: check the scanner settings to disable a terminating character or use an external keyboard for testing input without a terminator

  1. Open POS
  2. Connect a scanner with a disabled terminating character
  3. Make a scan
  4. Confirm the error row appears with "the scanner did not send an end-of-line character" error
  5. Re-enable the terminating character
  6. Confirm short barcode error, and non-existing errors continue to be shown

Testing information

Tested on iPadOS 26 with an Inateck scanner

Screenshots

ScreenRecording_07-26-2025.20-14-40_1.mov

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

staskus added 2 commits July 26, 2025 20:04
 If there's an unfinished scan, show the error row after waiting for the last inter-character timeout, not when the next character is received.
@staskus staskus added this to the 23.0 milestone Jul 26, 2025
@staskus staskus added type: task An internally driven task. feature: POS labels Jul 26, 2025
@staskus staskus requested review from iamgabrielma and jaclync July 26, 2025 18:17
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 26, 2025

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr15952-50872ac
Version22.9
Bundle IDcom.automattic.alpha.woocommerce
Commit50872ac
Installation URL5qk4cqe49e0vg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync
Copy link
Contributor

jaclync commented Jul 28, 2025

@staskus I brought the Android tablet + scanner to test i2 and don't have the iPad device for testing this PR today while working outside. Please let me know if it's urgent, and I can review the PR later today. Otherwise, I will review it tomorrow if it's still open.

@staskus
Copy link
Contributor Author

staskus commented Jul 28, 2025

@jaclync not urgent, good to merge by the end of the week.

Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit: TIL about the barcode scanner terminating character and settings.

Tera 1200 has 5 terminator settings, I tried all of them and "None" and "Add Tab" resulted in an error while the other 3 (CR, LF, CR/LF) succeeded as expected.

scanner settings result
Screenshot 2025-07-29 at 5 36 49 AM IMG_0504

"pointOfSale.barcodeScan.error.incompleteScan",
value: "Partial barcode scan",
comment: "Error message shown when scan is incomplete."
"pointOfSale.barcodeScan.error.incompleteScan2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: I think the convention in the code base before I started following it is *.versionNumber but it's really a minor detail that is internal to the code base.

Suggested change
"pointOfSale.barcodeScan.error.incompleteScan2",
"pointOfSale.barcodeScan.error.incompleteScan.2",


final class MockTimer: Timer {
var isCancelled = false
let mockTimeProvider: MockTimeProvider
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could there be a retain cycle between MockTimeProvider and MockTimer since they hold on to each other? Not sure if it's worth keeping one as weak var.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation, it would create retain cycles. We can make it weak to avoid some issues in tests.

}

@Test("proactive timeout triggers error without next character")
func timeout_whenTimerFires_triggersTimeoutError() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: the test function names feel like an interesting mix of snake & camel case 😆

@staskus staskus merged commit 0c3d8be into trunk Aug 4, 2025
13 checks passed
@staskus staskus deleted the woomob-765-woo-posbarcodes-if-theres-an-unfinished-scan-show-the-error branch August 4, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants