Skip to content

Add support for NFC write in FrontendScreen#6719

Merged
jpelgrom merged 4 commits intomainfrom
feature/nfc_write
Apr 21, 2026
Merged

Add support for NFC write in FrontendScreen#6719
jpelgrom merged 4 commits intomainfrom
feature/nfc_write

Conversation

@TimoPtr
Copy link
Copy Markdown
Member

@TimoPtr TimoPtr commented Apr 16, 2026

Summary

Add support for NFC write action on the FrontendScreen.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

I'm not a super fan of adding more and more methods to the view model, we might need to think about how we can limit that a first "easy" thing might be using delegation, but let see how it goes.

Copilot AI review requested due to automatic review settings April 16, 2026 13:17
@TimoPtr TimoPtr requested a review from jpelgrom April 16, 2026 13:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds NFC tag write support to the Compose-based FrontendScreen flow by wiring the tag/write external-bus message through the handler + ViewModel into a host-launched NFC write activity contract, then responding back to the frontend with a result message.

Changes:

  • Introduces tag/write as a typed incoming external-bus message and maps it to a new WriteNfcTag handler event
  • Adds a new one-shot navigation event (LaunchNfcWrite) and launches WriteNfcTag via an ActivityResultContract from FrontendNavigation
  • Sends a result response back to the frontend when the NFC write flow completes, plus adds/updates unit tests across parsing/handler/viewmodel/event-handler layers

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
app/src/main/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/IncomingExternalBusMessage.kt Adds TagWriteMessage / TagWritePayload for tag/write incoming messages
app/src/main/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendMessageHandler.kt Handles TagWriteMessage and emits FrontendHandlerEvent.WriteNfcTag
app/src/main/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendHandlerEvent.kt Adds WriteNfcTag handler event to carry messageId/tagId
app/src/main/kotlin/io/homeassistant/companion/android/frontend/navigation/FrontendEvent.kt Adds FrontendEvent.LaunchNfcWrite one-shot event
app/src/main/kotlin/io/homeassistant/companion/android/frontend/navigation/FrontendNavigation.kt Launches NFC write flow with rememberLauncherForActivityResult and consumes LaunchNfcWrite
app/src/main/kotlin/io/homeassistant/companion/android/frontend/externalbus/outgoing/OutgoingExternalBusMessage.kt Adds ResultMessage.success(...) helper for sending empty success results
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt Emits LaunchNfcWrite on handler event and sends ResultMessage on NFC completion
app/src/test/kotlin/io/homeassistant/companion/android/frontend/externalbus/incoming/IncomingExternalBusMessageTest.kt Adds JSON parsing tests for tag/write
app/src/test/kotlin/io/homeassistant/companion/android/frontend/handler/FrontendMessageHandlerTest.kt Adds handler mapping tests for TagWriteMessageWriteNfcTag
app/src/test/kotlin/io/homeassistant/companion/android/frontend/navigation/FrontendEventHandlerTest.kt Adds event-handler tests for LaunchNfcWrite routing
app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt Adds ViewModel tests for emitting LaunchNfcWrite and sending ResultMessage on completion

* @param messageId Correlation id from the originating `tag/write` request.
* @param tagId Optional pre-filled tag identifier.
*/
data class LaunchNfcWrite(val messageId: Int, val tagId: String?) : FrontendEvent
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency with other events: why not navigate to / open NFC write?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Addressed

@jpelgrom
Copy link
Copy Markdown
Member

we might need to think about how we can limit that a first "easy" thing might be using delegation, but let see how it goes.

Did you already have something specific in mind for delegation/do you have an example?

@TimoPtr
Copy link
Copy Markdown
Member Author

TimoPtr commented Apr 20, 2026

we might need to think about how we can limit that a first "easy" thing might be using delegation, but let see how it goes.

Did you already have something specific in mind for delegation/do you have an example?

Not yet TBH, I need to have another example to brainstorm about what I could do.

@TimoPtr TimoPtr requested a review from jpelgrom April 20, 2026 10:34
@jpelgrom
Copy link
Copy Markdown
Member

OK. I also don't have another good approach at this point so let's continue with this for now.

@jpelgrom jpelgrom merged commit 47a575e into main Apr 21, 2026
25 checks passed
@jpelgrom jpelgrom deleted the feature/nfc_write branch April 21, 2026 20:11
@TimoPtr TimoPtr added the WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav. label Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed WebViewActivity replacement Ongoing work to replace the WebViewActivity in favor of a well tested compose screen using nav.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants