Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jul 7, 2025

Closes WOOMOB-755

Description

This PR implements GameController-based barcode scanning for POS to solve language-dependent input issues. The current HID approach causes barcode parsing problems when iPad keyboard language differs from the scanner's keyboard setting (e.g., barcode "1234" might be read as "&é"'" due to iOS keyboard configuration mismatch).

The business logic aims to be exactly the same as the current keyboard text input + HIDBarcodeParser setup.

The implementation adds:

  • GameController framework integration to treat barcode scanning output as US/English, regardless of the iOS software keyboard
  • Feature flag pointOfSaleBarcodeScanningi2
  • GameControllerBarcodeObserver to observe the external keyboard connection and output
  • Updated BarcodeScannerContainer that switches between HID and GameController based on feature flag
  • Character mapping including digits, letters (with proper case handling), punctuation, and symbols with shift variants
  • Copy HIDBarcodeParser logic for GameControllerBarcodeParser

Testing steps

Testing GameController Implementation (Feature Flag Enabled)

  1. Navigate to POS and ensure barcode scanning is available
  2. Test basic scanning:
    • Scan numeric barcodes (e.g., "123456789")
    • Scan alphanumeric barcodes with mixed case (e.g., "ABC123def")
    • Scan barcodes with punctuation (e.g., "PROD-123.45")
    • Scan QR codes behind the text or URLs with shift key modified symbols
  3. Test language independence:
    • Change iPad keyboard language to French/German/etc.
    • Scan the same barcodes and verify they parse correctly
  4. Test edge cases:
    • Very fast scanning (multiple barcodes quickly)
    • Slow typing with external (should timeout appropriately)
    • Scanner disconnection/reconnection
    • Start the new order with scanning

Testing information

Tested with iPad Air running iOS 26 using Inateck scanner and various 1D and 2D barcodes.

Testing session

Game.Controller.Scanning.MP4

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

@staskus staskus added this to the 22.8 milestone Jul 7, 2025
@staskus staskus added type: task An internally driven task. feature: POS labels Jul 7, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jul 7, 2025

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 22.8. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 7, 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 Numberpr15877-99f3fb6
Version22.7
Bundle IDcom.automattic.alpha.woocommerce
Commit99f3fb6
Installation URL29ekb3n48dr1o
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

staskus added 5 commits July 8, 2025 10:32
Introduces `GameControllerBarcodeParser`, a new mechanism for processing input from physical barcode scanners that connect as game controllers. This approach offers a more reliable, language-independent method of barcode scanning by interpreting raw `GCKeyCode` values directly from the hardware. This bypasses the iOS keyboard layout system, which can interfere with HID-based scanning on international keyboards.

The new parser's logic has been aligned with the existing `HIDBarcodeParser` to ensure identical behavior and maintain consistency across scanning methods. This includes mirroring the specific, nuanced timeout handling. A test suite, `GameControllerBarcodeParserTests`, has been added to validate all aspects of the parser's functionality and to lock in the behavioral parity with its HID counterpart.
Shift press and un-press events are reported by keyChangedHandler. We need to use those to determine whether the shift key needs to be applied to the next character.
@staskus staskus force-pushed the woomob-755-woo-posbarcodes-i2-use-gamecontroller-framework-for branch from a1dd2e5 to d8e1b48 Compare July 8, 2025 10:20
@staskus staskus changed the title Woomob 755 woo posbarcodes i2 use gamecontroller framework for [Woo POS][Barcodes] i2 Use GameController framework for detecting scanner input Jul 8, 2025
@staskus staskus requested a review from joshheald July 8, 2025 10:33
@staskus staskus marked this pull request as ready for review July 8, 2025 10:33
@joshheald joshheald self-assigned this Jul 9, 2025
func makeUIViewController(context: Context) -> UIViewController {
let featureFlagService = ServiceLocator.featureFlagService

if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered if we should keep this for now or replace it entirely.

We could remove BarcodeScannerHostingController, HIDBarcodeParser, and possibly most if not all the BarcodeScannerContainer code.

GameControllerBarcodeObserver theoretically allows us to observe all the input within the aggregate model, although we would lose the ability to control in which screens we want to receive the input.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I think we should keep the container. We already have the need to ignore scans in some screens (search, email receipt, everything in between building a cart and payment success.

The need for granular control is likely to increase over time – for example, any time we add a text field we need to think about it, and any control will be based on view state, what's focused. Quantity selectors, customer detail entry, configuring bookings or subscription purchases – there's lots of places we may end up having more complex text field management.

Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Works really well. I've left a few small comments/questions in line, but all good.

The only thing I've noticed is a possibly increased tendency to miss the first character of a scan after the scanner's been asleep. It's hard to judge, but have you seen the same thing? If not, let's not worry about it – I don't have anything conclusive, and whenever I try to make it happen, it doesn't!

Oh, BTW it would be good to merge trunk first, and check the feature flag by hand, once you've decided about keeping the feature flag. I added one with the same name, further down, so we could get a mess.

Comment on lines 43 to 55
let featureFlagService = ServiceLocator.featureFlagService

if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) {
return GameControllerBarcodeScannerHostingController(
configuration: configuration,
onScan: onScan
)
} else {
return BarcodeScannerHostingController(
configuration: configuration,
onScan: onScan
)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let featureFlagService = ServiceLocator.featureFlagService
if featureFlagService.isFeatureFlagEnabled(.pointOfSaleBarcodeScanningi2) {
return GameControllerBarcodeScannerHostingController(
configuration: configuration,
onScan: onScan
)
} else {
return BarcodeScannerHostingController(
configuration: configuration,
onScan: onScan
)
}
return GameControllerBarcodeScannerHostingController(
configuration: configuration,
onScan: onScan
)

I don't actually think this needs to be feature flagged, we can just ship it – we're changing the app to match Android i1 after all.

It is helpful for testing the PR though. If you want to keep the flag, perhaps rename it to something other than i2, so we can ship this before the rest of i2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was helpful for testing. I didn't want to remove it right away. However, we could simply replace one with another without any feature flagging.

Comment on lines 50 to 62
// This prevents a double-trigger-pull on a Star scanner from adding an error row –
// Star use this as a shortcut to switch to the software keyboard. They send keycode 174 0xAE, which is
// undefined and reserved in UIKeyboardHIDUsage. The scanner doesn't send a character with the code.
// There seems to be no reason to handle empty input when considering scans.
return false
}

guard character.isNotEmpty else {
// This prevents a double-trigger-pull on a Star scanner from adding an error row –
// Star use this as a shortcut to switch to the software keyboard. They send keycode 174 0xAE, which is
// undefined and reserved in UIKeyboardHIDUsage. The scanner doesn't send a character with the code.
// There seems to be no reason to handle empty input when considering scans.
return false
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this comment block to be repeated? Perhaps it could all be in a single guard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we don't. I'll clean it up.

case .period: return isShiftPressed ? ">" : "."
case .slash: return isShiftPressed ? "?" : "/"
case .graveAccentAndTilde: return isShiftPressed ? "~" : "`"
case .returnOrEnter: return "\r"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should \n be in this list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's returnOrEnter, I think they are both treated the same way with GameController framework.

@staskus
Copy link
Contributor Author

staskus commented Jul 9, 2025

The only thing I've noticed is a possibly increased tendency to miss the first character of a scan after the scanner's been asleep.

@joshheald, interesting. I will test on my side a few times and see from the implementation point of view if there's any good reason for this to happen.

@staskus
Copy link
Contributor Author

staskus commented Jul 9, 2025

@joshheald, thanks for testing again!

I haven't been able to reproduce the first value missing issue myself. I rotated a dozen rounds of my scanner, going to sleep and disconnecting, and then scanning different types of barcodes. All of them succeeded. Let's keep observing it.

I removed the feature flag and left GameController as the only way to get the scanned items while also keeping the container to only track scanning within certain views.

@staskus staskus enabled auto-merge July 9, 2025 17:53
@staskus staskus merged commit 8d5bf6b into trunk Jul 9, 2025
13 checks passed
@staskus staskus deleted the woomob-755-woo-posbarcodes-i2-use-gamecontroller-framework-for branch July 9, 2025 18:10
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.

5 participants