-
-
Notifications
You must be signed in to change notification settings - Fork 340
Variable for %GPS in quick chat. Feature request #2653 #3535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 15 commits
8e1b191
d28de35
a775e0e
9edd293
a7bb96d
e32eee6
2d29fab
31e4af6
e6cb702
603e745
28f269b
6ae5878
62e1831
976e170
0e43566
7c27419
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,3 +41,6 @@ keystore.properties | |
|
|
||
| /build-logic/convention/build/* | ||
| /build-logic/build/ | ||
|
|
||
| # Claude Code | ||
| .claude/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,6 +106,7 @@ import org.meshtastic.core.ui.component.SecurityIcon | |
| import org.meshtastic.core.ui.component.SharedContactDialog | ||
| import org.meshtastic.core.ui.theme.AppTheme | ||
| import org.meshtastic.proto.AppOnlyProtos | ||
| import org.meshtastic.proto.MeshProtos | ||
| import java.nio.charset.StandardCharsets | ||
|
|
||
| private const val MESSAGE_CHARACTER_LIMIT_BYTES = 200 | ||
|
|
@@ -299,10 +300,13 @@ fun MessageScreen( | |
| QuickChatRow( | ||
| enabled = connectionState.isConnected(), | ||
| actions = quickChatActions, | ||
| userPosition = ourNode?.validPosition, | ||
| onClick = { action -> | ||
| handleQuickChatAction( | ||
| action = action, | ||
| messageInputState = messageInputState, | ||
| userPosition = ourNode?.validPosition, | ||
| ourNode = ourNode, | ||
| onSendMessage = { text -> onEvent(MessageScreenEvent.SendMessage(text)) }, | ||
| ) | ||
| }, | ||
|
|
@@ -422,25 +426,49 @@ private fun String.ellipsize(maxLength: Int): String = if (length > maxLength) " | |
| * | ||
| * @param action The [QuickChatAction] to handle. | ||
| * @param messageInputState The [TextFieldState] of the message input field. | ||
| * @param userPosition Current user position (lat/lng), if available. | ||
| * @param onSendMessage Lambda to call when a message needs to be sent. | ||
| */ | ||
| private fun handleQuickChatAction( | ||
| action: QuickChatAction, | ||
| messageInputState: TextFieldState, | ||
| userPosition: MeshProtos.Position?, | ||
| ourNode: Node?, | ||
| onSendMessage: (String) -> Unit, | ||
| ) { | ||
| val processedMessage = | ||
| if ( | ||
| action.message.contains("%LAT", ignoreCase = true) || | ||
| action.message.contains("%LON", ignoreCase = true) || | ||
| action.message.contains("%SNAME", ignoreCase = true) | ||
| ) { | ||
| var result = action.message | ||
| userPosition?.let { | ||
| val latitude = "%.7f".format(it.latitudeI * 1e-7) | ||
| val longitude = "%.7f".format(it.longitudeI * 1e-7) | ||
| result = | ||
| result.replace("%LAT", latitude, ignoreCase = true).replace("%LON", longitude, ignoreCase = true) | ||
| } | ||
| ourNode?.user?.shortName?.let { shortName -> | ||
| result = result.replace("%SNAME", shortName, ignoreCase = true) | ||
| } | ||
| result | ||
| } else { | ||
| action.message | ||
| } | ||
|
|
||
| when (action.mode) { | ||
| QuickChatAction.Mode.Append -> { | ||
| val originalText = messageInputState.text.toString() | ||
| // Avoid appending if the exact message is already present (simple check) | ||
| if (!originalText.contains(action.message)) { | ||
| if (!originalText.contains(processedMessage)) { | ||
| val newText = | ||
| buildString { | ||
| append(originalText) | ||
| if (originalText.isNotEmpty() && !originalText.endsWith(' ')) { | ||
| append(' ') | ||
| } | ||
| append(action.message) | ||
| append(processedMessage) | ||
| } | ||
| .limitBytes(MESSAGE_CHARACTER_LIMIT_BYTES) | ||
| messageInputState.setTextAndPlaceCursorAtEnd(newText) | ||
|
|
@@ -449,7 +477,7 @@ private fun handleQuickChatAction( | |
|
|
||
| QuickChatAction.Mode.Instant -> { | ||
| // Byte limit for 'Send' mode messages is handled by the backend/transport layer. | ||
| onSendMessage(action.message) | ||
| onSendMessage(processedMessage) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -683,13 +711,15 @@ private fun OverFlowMenu( | |
| * | ||
| * @param enabled Whether the buttons should be enabled. | ||
| * @param actions The list of [QuickChatAction]s to display. | ||
| * @param userPosition Current user position, used to determine if location-based actions can be enabled. | ||
| * @param onClick Callback when a quick chat button is clicked. | ||
| */ | ||
| @Composable | ||
| private fun QuickChatRow( | ||
| modifier: Modifier = Modifier, | ||
| enabled: Boolean, | ||
| actions: List<QuickChatAction>, | ||
| userPosition: MeshProtos.Position?, | ||
| onClick: (QuickChatAction) -> Unit, | ||
| ) { | ||
| val alertActionMessage = stringResource(R.string.alert_bell_text) | ||
|
|
@@ -707,8 +737,11 @@ private fun QuickChatRow( | |
| val allActions = remember(alertAction, actions) { listOf(alertAction) + actions } | ||
|
|
||
| LazyRow(modifier = modifier.padding(vertical = 4.dp), horizontalArrangement = Arrangement.spacedBy(4.dp)) { | ||
| items(allActions, key = { it.uuid }) { action -> | ||
| Button(onClick = { onClick(action) }, enabled = enabled) { Text(text = action.name) } | ||
| items(allActions, key = { it.position }) { action -> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we changing this key?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 The position field is guaranteed unique: Bell action: position = -1 |
||
| val requiresPosition = | ||
| action.message.contains("%LAT", ignoreCase = true) || action.message.contains("%LON", ignoreCase = true) | ||
| val isEnabled = enabled && (!requiresPosition || userPosition != null) | ||
| Button(onClick = { onClick(action) }, enabled = isEnabled) { Text(text = action.name) } | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're going to have an issue with shortnames with a space, among other cases.
The space will break your new regex, and other items like
shortName = "<')<"or">))>"(my poor attempt at a four char ascii art fish), will break the Linkify URL parsing.You're going to need URI encoding for the label:
But this URI-encodes all uses of
%SNAME.Some options:
%SNAMEENC,which URI encodes%GEOtoken which directly get replaced with the full"geo:(${latitude),${longitude}(${Uri.encode(label)})""geo:47.62148,-122.34814(FriendXYZ+@+2025-11-09+16:40)", which would be from hypothetical future tokens of"geo:%LAT,%LON(%LNAMEENC+@+$DATE+$TIME)"(timestamp is so folks don't confuse old map pins)geo:47.62148,-122.34814, auto-append a label of who+when, if a label doesn't exist, before sending to LinkifyUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I applied your URI encoding change. (I can't believe I didn't think about that before)
I implemented %LNAME (but I did not add it to the template, I like the shorter label on maps)
The pin created is ephemeral (unlike waypoint pins) so adding date/timestamps seems unnecessary, as they only exist for the lifetime of the view session, and the messages they appear in already have a timestamp.
In the context of quick chat messages, it seems redundant to want to include an unencode version of the node name in the message, because every message will already have that (sender).
What sort of failure/fallback cases are you thinking about for the simpler %GEO message use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might I suggest %F0%9F%90%A0 for your fish? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No failure cases, just mentioning that you can use a
%GEOtoken to produce the full message, and you wouldn't have to worry about when to encode%SNAME.In your updated code, you're always encoding the
%SNAME& the now added%LNAMEtoo. This has the issue of URI encoding when not needed, like with a quick message of"Test message seen by %LNAME in SOMA"; sending%SNAMEor%LNAMEis useful as most nodes in my list don't have a name, so I assume others likewise don't know my node's name (even more likely for new nodes):The pins can be non-ephemeral once in the map apps, directly so if bookmarked, seen in search history, or used as a routing destination. I certainly have a ton of old bookmarked temporary locations.
Message timestamps are lost in store-and-forward; so if you're disconnected from the mesh when someone sends their location, you'll get the stored message automatically when you reconnect but it will have the current timestamp displayed on an hours old message, implying the sender is at that location now. Granted that should be fixed in store-and-forward.
I could make a case for spiky boi %F0%9F%90%A1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get it now. People may want to use the variable expansion in quick chat messages other than pin drops. Now I see why the *ENC variants would be useful.
I started out with this being a quick chat mod. currently the variable expansions only happen when quick chat messages are triggered. Should I expand it so that any manually typed messages in the message window can use variable expansions? I don't like making changes to areas outside of the scope of the enhancement, so I'm inclined not to do that.
I noticed that the other variables from your example, $date and $time don't actually work. Were they hypothetical or am I using them wrong?
I see now that it could be useful to ALSO provide a single %GEO expansion with the simple, non labeled version of the geo: URI
I'll go add that back in %GEO for single term expansion, in addition to the current set.
Do you think the URI encoding overhead is significant enough to warrant the complexity of only doing it when the variable is used?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a feel for the Meshtastic repos, but it's likely easier to put in a MVP with a constrained set of tokens, like (
%LAT,%LON) or%GEO, touching a minimal area of code, in the first PR. Letting users test the feature, and setting the basis for a discussion on what is useful and a followup of adding those tokens and further functionality.Couple examples as precedent of a thought-though replacement token & operator scheme:
$token_name|u$w/ the|uafterwards to indicate to URL encode the token value. In splunk,$env:user_realname|u$would be the user's full username with URL encoding operator added.https://help.splunk.com/en/splunk-cloud-platform/create-dashboards-and-reports/simple-xml-dashboards/10.1.2507/drilldown-and-dashboard-interactivity/token-usage-in-dashboards#token-filters-0
*|FNAME|*for the user's firstname and*|URL:FNAME|*as an operator to URL encode the first namehttps://mailchimp.com/help/all-the-merge-tags-cheat-sheet
Takeaways from looking at replacement token implementations:
#{TOKEN}#(Azure Pipelines),$TOKEN$(splunk),*|FNAME|*(mailchimp)$server.serverName$(splunk),*|USER:COMPANY|*(Mailchimp)$token_name|u$(splunk), and*|URL:FNAME|*(Mailchimp)<dyn type="mapFrame" name="MapFrameName" property="lowerLeft.x" units="dd.deg" decimalPlaces="2" showDirections="True"/>to get the longitude of the left side of the map rounded to two decimal places"122.31"ROUND($LAT$, 2))for its operatorsI might recommend:
E.g.
$POSITION:LATITUDE$and$USER:LONGNAME|U$for a URL encoded long name. Though something more terse is nice too, like$LNAME|U$.Hypothetical future tokens.
Not sure what you're asking.