Skip to content

Conversation

@suteny0r
Copy link

#2653

Implemented %GPS variable for use in quick chat. First run inserts quick chat item for dropping location pin in message. It can be edited or deleted. On first run, if there are no entries in the quick chat table, this entry will be recreated. This also allows the user to change the URL to use any other mapping service instead of Google Maps.

This PR supersedes #3533

Thanks @deeno, good idea.

@deeno
Copy link

deeno commented Oct 24, 2025

@suteny0r Uh thanks, though I'm not sure what for?. 🧐🤨

@DaneEvans
Copy link
Collaborator

AI crap, detekt. Then you can consider trying to hurry us up

@DaneEvans DaneEvans self-requested a review October 24, 2025 12:38
Enables QuickChat messages to include dynamic GPS coordinates using %GPS variable.
When a QuickChat action contains %GPS, it is replaced with the current device
coordinates in format: latitude,longitude (7 decimal places).

Adds default location pin (📍) button as second quick chat action that inserts
https://maps.google.com/?q=%GPS which expands to actual coordinates.

Changes:
- Add userLatitude/userLongitude parameters to QuickChatRow and handleQuickChatAction
- Replace %GPS (case-insensitive) with lat,lon when coordinates available
- Add locationAction button (position=-2) that appears when GPS valid
- Pass GPS coordinates from ourNode validPosition to action handler

Example: https://maps.google.com/?q=%GPS becomes
         https://maps.google.com/?q=25.7987898,-80.3939333
Fixes crash 'Key "0" was already used' when showing QuickChat.

Both alertAction and locationAction have default uuid=0L, causing
key collision in LazyRow. Changed to use position which is guaranteed
unique: bell=-1, location=-2, user actions=0,1,2...
Remove hardcoded location action. Users can now create their own
QuickChat action with %GPS variable which will be replaced with
actual coordinates.

Example: Create QuickChat action with message 'https://maps.google.com/?q=%GPS'
and it expands to 'https://maps.google.com/?q=25.7987898,-80.3939333'

This allows users to:
- Edit the action name (emoji)
- Change between Append/Instant mode
- Modify the URL or message format
- Reorder actions
- Delete if not needed
Initialize database with default location pin (📍) QuickChat action
when no actions exist. Creates entry with:
- Name: 📍
- Message: https://maps.google.com/?q=%GPS
- Mode: Append
- Position: 0

Users can edit, delete, or reorder this action via QuickChat settings.
Move default location action initialization from QuickChatViewModel
to MessageViewModel. MessageViewModel is instantiated when opening a
channel, ensuring the default action appears immediately without
needing to first open QuickChat settings.
@suteny0r
Copy link
Author

@suteny0r Uh thanks, though I'm not sure what for?. 🧐🤨

You brought #2653 to my attention (on discord). It was a superior method of achieving the location pin drop that I had submitted, so I rewrote it and resubmitted it. The new method allows the user to modify the map URL to use any map service provider they like. It allows them access to the immediate send selector.

In contrast to waypoints, this method allows the user to send to private channels, and to DM individual nodes. It also doesn't have the 'anyone can delete the waypoint I just created' problem. Waypoints in general are just a bad way of communicating position. They are static. If you are travelling, the waypoint you create is a marker showing where you were, at one time, not where you are. Messages are ephemeral. You don't have to go back and clean up the ones you send. Its obvious that a location you drop in a message isn't meant to indicate permanence.

@DaneEvans
Copy link
Collaborator

I'm Deeno over there, he's someone else.

You still aren't getting green ticks here. Read the contributing.md, run spotless and detekt. delete the unnecessary AI junk, then I'll review it.

And stop trying to speed things up while not doing your part.

suteny0r and others added 2 commits October 25, 2025 13:24
Applied automatic code formatting fixes to pass CI checks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Remove unused userLatitude and userLongitude parameters from QuickChatRow function
- Update detekt baseline to allow 8 parameters in MessageViewModel constructor (needed for quickChatActionRepository dependency injection)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@suteny0r
Copy link
Author

I'm Deeno over there, he's someone else.

You still aren't getting green ticks here. Read the contributing.md, run spotless and detekt. delete the unnecessary AI junk, then I'll review it.

And stop trying to speed things up while not doing your part.

Thanks for bringing that to my attention.

action = action,
messageInputState = messageInputState,
userLatitude = ourNode?.takeIf { it.validPosition != null }?.latitude,
userLongitude = ourNode?.takeIf { it.validPosition != null }?.longitude,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this not a named tuple?

Copy link
Author

Choose a reason for hiding this comment

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

Now it is.

onSendMessage: (String) -> Unit,
) {
val processedMessage =
if (userLatitude != null && userLongitude != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be checking for the presence of the string before it does any other logic.

if message.lower.contains ('%gps') 
     if <valid gps data> 
         <do the replace> 

if <any other quick message params we want?> 




Copy link
Author

Choose a reason for hiding this comment

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

Good catch. Now it does.

Copy link
Author

Choose a reason for hiding this comment

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

and now it does, properly.


LazyRow(modifier = modifier.padding(vertical = 4.dp), horizontalArrangement = Arrangement.spacedBy(4.dp)) {
items(allActions, key = { it.uuid }) { action ->
items(allActions, key = { it.position }) { action ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we changing this key?

Copy link
Author

Choose a reason for hiding this comment

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

The key was changed from uuid to position to fix a crash: Key "0" was already used.

Both the alert bell action and the default location pin action are created with uuid=0L (the default
value). When both are present in the LazyRow, this causes a key collision that crashes the app.

The position field is guaranteed unique:

Bell action: position = -1
Location pin action: position = -2 (or could be 0 if it's user-editable)
User-created actions: position = 0, 1, 2...
This ensures each item in the LazyRow has a unique, stable key for proper Compose recomposition.

quickChatActionRepository.upsert(
org.meshtastic.core.database.entity.QuickChatAction(
name = "📍",
message = "https://maps.google.com/?q=%GPS",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're not going to serve a google link to the mesh by default.

Copy link
Author

@suteny0r suteny0r Oct 26, 2025

Choose a reason for hiding this comment

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

Good point. I'll make it conditional on whether it's the f-droid or google build to use google maps or openstreetmaps.

Copy link
Author

Choose a reason for hiding this comment

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

The layout of the OSM URL required that I separate the %GPS variable into %LAT and %LON. The app now constructs the appropriate URL, depending on whether it is the fdroid or google build.

suteny0r and others added 3 commits October 25, 2025 20:24
Removes the Claude Code settings file from version control and adds
.claude/ directory to .gitignore to prevent future commits.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Remove accidentally committed image file from repository.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
…URLs

Add support for %LAT and %LON variables in QuickChat messages, with
automatic substitution of user coordinates. Map URLs are now configured
per build variant to respect F-Droid preferences.

Changes:
- Replace single %GPS variable with %LAT and %LON variables
- Add DEFAULT_MAP_URL BuildConfig field with variant-specific URLs:
  * Google: https://maps.google.com/?q=%LAT,%LON
  * F-Droid: https://www.openstreetmap.org/?mlat=%LAT&mlon=%LON#map=13/%LAT/%LON
- Refactor to use MeshProtos.Position instead of separate lat/lng parameters
- Check for variable presence before processing GPS substitution
- Update existing location pin URLs when switching between build variants
- Add core.common dependency to feature:messaging module
- Update detekt baseline for additional MessageViewModel parameter

Addresses reviewer feedback:
- Use named tuple (Position) instead of separate primitive parameters
- Check for variable presence before GPS data lookup
- No hardcoded Google Maps URLs in F-Droid builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants