Android: expose Barcode::extra() metadata as a Map on Result - #1147
Android: expose Barcode::extra() metadata as a Map on Result#1147bscanlon-kr wants to merge 1 commit into
Conversation
Symbology-specific metadata (e.g. the original UPC-E text) was only reachable via the core C++ extra() API and had no path through the JNI bridge into the Kotlin wrapper.
There was a problem hiding this comment.
Pull request overview
This PR updates the Android wrapper to expose ZXing::Barcode::extra() metadata on the Android BarcodeReader.Result, enabling access to symbology-specific fields (e.g. the original UPC-E text via UPCE) that are currently only available through the native extras JSON.
Changes:
- Add an
extraJsonfield to AndroidBarcodeReader.Resultand expose a lazily-parsedextra: Map<String, String>. - Extend the JNI
NewResultconstructor call to passBarcode::extra()into the KotlinResultobject.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wrappers/android/zxingcpp/src/main/java/zxingcpp/BarcodeReader.kt | Adds extraJson and exposes extra as a lazily-parsed map on Result. |
| wrappers/android/zxingcpp/src/main/cpp/ZXingCpp.cpp | Extends JNI Result constructor signature and passes result.extra() into Kotlin. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| val extra: Map<String, String> by lazy { | ||
| extraJson?.takeIf { it.isNotEmpty() }?.let { json -> | ||
| val obj = org.json.JSONObject(json) | ||
| obj.keys().asSequence().associateWith(obj::getString) | ||
| } ?: emptyMap() | ||
| } |
There was a problem hiding this comment.
That sounds plausible to me. What is your opinion @bscanlon-kr?
@markusfisch: Do you agree that the Map<String, String> is the best / most easy to use API to access those values?
To both of you: would returning a JSONObject not be even better as it would preserve the type of the data? In Python I went for the native dict type as adding an external dependency on JSON was not bringing any advantage. Simply returning the raw String and let the client code decide if constructing a JSONObject is worth it, would also make sense to me. I don't have a clear opinion, as I am not a regular Android/Kotlin developer.
There was a problem hiding this comment.
Update: I asked my LLM of choice and that came up with a 3rd suggestion that actually feels convincing to me:
For a typical Android/Kotlin developer, the most “native” API is a Kotlin map with typed values, not a raw JSON object and not a Map<String, String> that silently throws away type information.
The best fit is:
result.extra: Map<String, Any?>
optionally also expose result.extraJson: String? as a raw escape hatch
That gives a very Kotlin-ish usage pattern:
val upce = result.extra["UPCE"] as? String
val readerInit = result.extra["ReaderInit"] as? Boolean
val dataMask = result.extra["DataMask"] as? Int
This is much more natural than:
val obj = JSONObject(result.extraJson)
and avoids the Java-ish feel of org.json.JSONObject in a Kotlin-first API.
There was a problem hiding this comment.
Yes, I think that’s the best fit, too 👍 Also, getting outpaced by an LLM even for a quick question is the new normal, I guess 😉 Bright new world… 🤖
There was a problem hiding this comment.
@bscanlon-kr do you agree and would you be interested in updating your PR accordingly?
In this issue its called out that to get access to the raw scan data for the UPC-E scan the extras need to be referenced. The extras aren't exposed in the Android wrapper. (I suspect this is the case for many of the wrappers) this PR adds the ability to pull from the extras map.