[INJIMOB-3652] enhance sample with data transfer logging and qr connect#183
[INJIMOB-3652] enhance sample with data transfer logging and qr connect#183swatigoel merged 1 commit intoinji:developfrom
Conversation
WalkthroughAdds QR scanning and QR display flows, refactors MainActivity for wallet/verifier event-driven flows and UI state, introduces QrScanActivity, updates permissions and annotations for camera/Bluetooth, adds ZXing and Tuvali dependencies, and introduces new/send-input and revised main layouts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainActivity
participant QrScanActivity
participant ZXing
participant Wallet
participant Verifier
User->>MainActivity: Tap Wallet
MainActivity->>QrScanActivity: startActivityForResult (scan)
QrScanActivity->>ZXing: launch scanner UI
User->>ZXing: scan QR
ZXing-->>QrScanActivity: scan result
QrScanActivity-->>MainActivity: onActivityResult (SCANNED_URI)
MainActivity->>Wallet: startScanningWithUri(uri)
Wallet-->>MainActivity: SecureChannelEstablishedEvent
MainActivity->>User: show send-input UI
User->>MainActivity: send data
MainActivity->>Wallet: send payload
Wallet-->>MainActivity: DataSent / DataReceivedEvent
alt Verifier flow
User->>MainActivity: Tap Verifier
MainActivity->>Verifier: startAdvertising()
Verifier-->>MainActivity: QR URI / QR bitmap
MainActivity->>User: display QR
User (other device)->>ZXing: scan QR (wallet)
Wallet->>Verifier: connect via URI
Verifier-->>MainActivity: SecureChannelEstablishedEvent
Wallet->>Verifier: send data
Verifier-->>MainActivity: DataReceivedEvent (payload)
MainActivity->>User: display transfer metrics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
kotlin/example/demo-android-app/app/src/main/res/layout/activity_main.xml (1)
18-29: Extract hardcoded strings tostrings.xml.The button texts "Start Wallet" and "Start Verifier" are hardcoded. For i18n/l10n support, consider extracting these to
strings.xml.<Button android:id="@+id/walletButton" android:layout_width="match_parent" android:layout_height="wrap_content" - android:text="Start Wallet" + android:text="@string/start_wallet" android:layout_marginBottom="16dp" /> <Button android:id="@+id/verifierButton" android:layout_width="match_parent" android:layout_height="wrap_content" - android:text="Start Verifier" /> + android:text="@string/start_verifier" />kotlin/example/demo-android-app/app/src/main/res/layout/activity_send_input.xml (1)
1-49: LGTM!The layout structure is well-organized with a scrollable input area and fixed action buttons. Consider extracting hardcoded strings ("Enter your message", "Send", "Disconnect") to
strings.xmlfor i18n support.kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/Common.kt (1)
72-76: Consider migrating to Activity Result API.
startActivityForResultis deprecated. Consider usingActivityResultLauncherfrom the Activity Result API for future compatibility.Additionally, the
@RequiresPermission(BLUETOOTH_CONNECT)annotation may cause lint warnings on API < 31 whereBLUETOOTHpermission is used instead. Consider usinganyOfor conditional annotation:@RequiresPermission(anyOf = [Manifest.permission.BLUETOOTH_CONNECT, Manifest.permission.BLUETOOTH])kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/QrScannerActivity.kt (2)
21-32: Callsuper.onActivityResultbeforefinish().The superclass method should typically be called before processing and finishing the activity to ensure proper lifecycle handling.
override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { + super.onActivityResult(requestCode, resultCode, data) val result = IntentIntegrator.parseActivityResult(requestCode, resultCode, data) if (result != null && result.contents != null) { val intent = Intent() intent.putExtra("SCANNED_URI", result.contents) setResult(RESULT_OK, intent) } else { setResult(RESULT_CANCELED) } finish() - super.onActivityResult(requestCode, resultCode, data) }
8-8: Consider extendingAppCompatActivityfor consistency.This activity extends
android.app.ActivitywhileMainActivityextendsAppCompatActivity. For consistency and access to compat features (themes, etc.), consider usingAppCompatActivity.-class QrScanActivity : Activity() { +class QrScanActivity : AppCompatActivity() {kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt (2)
60-62: Consider migrating toregisterForActivityResult.
startActivityForResultis deprecated. The Activity Result API provides a cleaner callback-based approach.private val qrScanLauncher = registerForActivityResult( ActivityResultContracts.StartActivityForResult() ) { result -> if (result.resultCode == RESULT_OK) { val uri = result.data?.getStringExtra("SCANNED_URI") if (!uri.isNullOrEmpty()) { startScanningWithUri(uri) } } } // Then in showActionsView: findViewById<Button>(R.id.walletButton)?.setOnClickListener { val intent = Intent(this, QrScanActivity::class.java) qrScanLauncher.launch(intent) }
31-31: Usecompanion objectfor TAG constant.For consistency with Kotlin best practices, define
TAGas a compile-time constant in a companion object.+ companion object { + private const val TAG = "TuvaliDemo" + } + - private val TAG = "TuvaliDemo"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
kotlin/example/demo-android-app/app/build.gradle.kts(1 hunks)kotlin/example/demo-android-app/app/src/main/AndroidManifest.xml(2 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/Common.kt(3 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt(1 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/QrScannerActivity.kt(1 hunks)kotlin/example/demo-android-app/app/src/main/res/layout/activity_main.xml(1 hunks)kotlin/example/demo-android-app/app/src/main/res/layout/activity_send_input.xml(1 hunks)kotlin/example/demo-android-app/gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt
[warning] 179-179: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-kotlin / maven-build
🔇 Additional comments (5)
kotlin/example/demo-android-app/app/src/main/AndroidManifest.xml (2)
5-10: LGTM!Camera feature marked as
android:required="false"is appropriate for optional QR scanning functionality, and the CAMERA permission declaration is correctly placed.
27-27: Activity class name matches the declaration.The manifest declares
.QrScanActivity, which correctly matches the class nameQrScanActivityinQrScannerActivity.kt.kotlin/example/demo-android-app/app/src/main/res/layout/activity_main.xml (1)
2-8: LGTM!Clean layout structure with proper organization into logical sections (actions, QR, loader) and sensible visibility defaults.
kotlin/example/demo-android-app/gradle/libs.versions.toml (1)
11-12: LGTM!Centralizing dependency versions in the version catalog is a good practice for maintainability.
Also applies to: 29-30
kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt (1)
225-239: LGTM! QR code generation is correct.The QR code generation using ZXing's
QRCodeWriteris properly implemented with error handling and logging.
kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt
Outdated
Show resolved
Hide resolved
kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt
Show resolved
Hide resolved
Signed-off-by: Abhishek Paul <paul.apaul.abhishek.ap@gmail.com>
abhip2565
force-pushed
the
injimob-3652
branch
from
87418a5 to
177f83a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/Common.kt (1)
52-76: Reconsider@RequiresPermissionon the permission-request helper
requestForRequiredPermissionsis the entry point you call beforeBLUETOOTH_CONNECTis granted (it is the method that actually requests it), so annotating it with@RequiresPermission(Manifest.permission.BLUETOOTH_CONNECT)is semantically misleading and may trigger lint on callers likeshowPermErrorView.You already annotate
startBluetooth, which is the only place that actually touches Bluetooth state; that’s the more accurate place for the permission contract. Consider dropping the annotation fromrequestForRequiredPermissionsand keeping it only onstartBluetooth:- @RequiresPermission(Manifest.permission.BLUETOOTH_CONNECT) - fun requestForRequiredPermissions( + fun requestForRequiredPermissions( activity: Activity, context: Context, showActionsView: KFunction0<Unit> ) {This keeps the static contract closer to the real runtime behaviour while still allowing lint to track BLUETOOTH_CONNECT usage through
startBluetooth.kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt (3)
35-41: Clarify permission annotations and result handling on lifecycle callbacksAnnotating
onCreateandonRequestPermissionsResultwith@RequiresPermission(Manifest.permission.BLUETOOTH_CONNECT)is a bit at odds with reality: these callbacks are invoked by the framework even when the app doesn’t yet hold the permission, and you’re explicitly using them to request it viaCommon.requestForRequiredPermissions.If the main goal was to satisfy lint for the Bluetooth enable flow, it’s usually cleaner to keep
@RequiresPermissionon the smallest methods that actually require it (e.g.,startBluetooth) and let the permission-request flow remain unannotated or guarded by explicit checks, rather than marking lifecycle callbacks as requiring the permission.While you’re here, you may also want to switch from the magic number
0toPackageManager.PERMISSION_GRANTEDin thegrantResultscheck for readability:- if (grantResults.any { it != 0 }) { + if (grantResults.any { it != PackageManager.PERMISSION_GRANTED }) {Also applies to: 43-55
84-131: Guard against accumulating Wallet/Verifier subscriptions across sessionsBoth
startScanningWithUriandstartAdvertisingcallwallet.subscribe { … }/verifier.subscribe { … }each time a new session is started, but there’s no corresponding unsubscribe or guard to ensure only one active subscriber.Depending on the Tuvali API semantics, this may be fine (e.g., if
subscribereplaces the previous listener), but if it adds listeners, repeated wallet/verifier sessions would cause multiple callbacks to fire for a single event.Consider ensuring that:
- You subscribe once (e.g., in
onCreate) and route events based on current UI/state; or- You keep a handle to the subscription (if the API returns one) and dispose it in
stopScanning()/stopAdvertisement()oronDestroy().This will keep event handling predictable if users restart wallet/verifier flows multiple times.
Also applies to: 137-189
200-239: UI helper and keyboard-dismiss logic are well factored (minor QR error logging tweak)The separation into
showLoadingLayout/showPermErrorView/updateLoadingText/updateQRCodeDataplusdispatchTouchEventfor keyboard dismissal keeps the main wallet/verifier flows readable and encapsulates UI concerns nicely.One small optional improvement: in
showQRCode, you currently log onlye.message. If you ever need to debug QR generation issues, including the exception itself will give you a stack trace:- } catch (e: Exception) { - Log.e(TAG, "QR generation error: ${e.message}") - } + } catch (e: Exception) { + Log.e(TAG, "QR generation error", e) + }Otherwise, this structure looks good.
Also applies to: 241-258, 264-281
kotlin/example/demo-android-app/app/src/main/res/layout/activity_main.xml (1)
2-101: Layout structure aligns well with the new flows (add contentDescription for QR image)The new
LinearLayout-based structure withactionsLayout,qrSection, andloaderLayoutmaps cleanly onto the wallet/verifier/loader states inMainActivity, and the IDs line up with the helper methods and event handlers.For accessibility, consider adding a
contentDescription(or explicitly setting it to@nullif purely decorative) onqrCodeImageso screen readers handle it appropriately, for example:<ImageView android:id="@+id/qrCodeImage" android:layout_width="250dp" android:layout_height="250dp" android:layout_margin="10dp" - android:background="@android:color/white" /> + android:background="@android:color/white" + android:contentDescription="@string/qr_code_content_description" />(and add
@string/qr_code_content_descriptiontostrings.xml, e.g., “QR code for establishing a connection”.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
kotlin/example/demo-android-app/app/build.gradle.kts(1 hunks)kotlin/example/demo-android-app/app/src/main/AndroidManifest.xml(2 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/Common.kt(3 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt(1 hunks)kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/QrScannerActivity.kt(1 hunks)kotlin/example/demo-android-app/app/src/main/res/layout/activity_main.xml(1 hunks)kotlin/example/demo-android-app/app/src/main/res/layout/activity_send_input.xml(1 hunks)kotlin/example/demo-android-app/gradle/libs.versions.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- kotlin/example/demo-android-app/app/build.gradle.kts
- kotlin/example/demo-android-app/app/src/main/res/layout/activity_send_input.xml
- kotlin/example/demo-android-app/app/src/main/AndroidManifest.xml
- kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/QrScannerActivity.kt
🔇 Additional comments (2)
kotlin/example/demo-android-app/gradle/libs.versions.toml (1)
29-30: Library entries correctly reference versioned dependencies.The library entries properly use
version.refto reference the versions defined earlier, following Gradle version catalog best practices. This ensures consistent dependency versions across the project.kotlin/example/demo-android-app/app/src/main/java/com/example/myapplication/MainActivity.kt (1)
160-185: DataReceived parsing, logging, and UI updates look solidThe
DataReceivedEventhandler now cleanly:
- Decodes the payload,
- Parses the timestamp and message with a safe
split(limit = 2),- Computes
transferTime,- Logs both message and transfer time, and
- Falls back to a plain “Received (no timestamp)” view on parse errors while logging the full exception.
Also, the earlier duplicated “Transfer Time” in the UI has been removed. No further changes needed here.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.