Skip to content

Conversation

@mdecourcy
Copy link
Contributor

  • Implemented tap action on “New Node Seen” notifications to open the specific node’s Details screen.
  • Added deep link meshtastic://meshtastic/node/{destNum} to the notification PendingIntent.
  • MainActivity forwards unhandled deep links to the UI; MainScreen parses /node/{destNum} and navigates to NodesRoutes.NodeDetailGraph(destNum) so ViewModels receive the param.
  • Matches existing message notification deep-link behavior; if parsing fails, it safely falls back to opening the app.

@github-actions github-actions bot added the enhancement New feature or request label Nov 13, 2025
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 0% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.51%. Comparing base (65775fd) to head (5b617d3).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
app/src/main/java/com/geeksville/mesh/ui/Main.kt 0.00% 18 Missing ⚠️
...ville/mesh/service/MeshServiceNotificationsImpl.kt 0.00% 9 Missing ⚠️
...src/main/java/com/geeksville/mesh/model/UIState.kt 0.00% 5 Missing ⚠️
.../src/main/java/com/geeksville/mesh/MainActivity.kt 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #3684      +/-   ##
========================================
- Coverage   0.51%   0.51%   -0.01%     
========================================
  Files        374     377       +3     
  Lines      21159   21328     +169     
  Branches    2538    2588      +50     
========================================
  Hits         109     109              
- Misses     21030   21199     +169     
  Partials      20      20              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@jamesarich jamesarich left a comment

Choose a reason for hiding this comment

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

@mdecourcy - we've already got deeplink routes defined in navigation.. we should be handling this similarly to how we're currently navigating to the correct Conversation from the message notif.

see: https://github.com/meshtastic/Meshtastic-Android/blob/main/app/src/main/java/com/geeksville/mesh/navigation/NodesNavigation.kt#L90-L96

@mdecourcy mdecourcy requested a review from jamesarich November 13, 2025 23:33
Copy link
Collaborator

@DaneEvans DaneEvans left a comment

Choose a reason for hiding this comment

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

Looks fine to me once James' happy

val topLevelDestination = TopLevelDestination.fromNavDestination(currentDestination)

// Handle pending deep links (e.g., from notifications) once NavHost is ready
LaunchedEffect(pendingDeepLink) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The current message notification routes to the message screen appropriately without this, using the Navigation sdk deeplinking that's already in place. Why do we need additional handling here?

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 honestly wasnt able to get it working without the extra handling, just kept opening the node lsit. I can take another stab at it later today when I'm at the airport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants