Location Sharing and marker deduplikation.#346
Location Sharing and marker deduplikation.#346ericszimmermann wants to merge 14 commits intozjs81:devfrom
Conversation
…verwrite existing GPS with null when the incoming frame had 0,0 coordinates. Now it also preserves prior coordinates when the incoming update omits location.
This adds Giphy page URLs and `media.giphy.com` URLs (with and without protocols) as *accepted* encodings for GIF messages, alongside the `g:` syntax. When someone posts such a URL by itself as a message, it will be rendered inline just like `g:` messages are now. This does not change the encoding that GIF messages are *sent* in; that is still the `g:` syntax.
Support receiving more formats of GIF message
Add LowMesh prefix and explain how to add more
Preserve Coordinates with contact.copyWith() function
Add additional device name prefixes to MeshCoreUuids
|
Testbuild with all four Pullrequest from me can be found here: |
There was a problem hiding this comment.
Pull request overview
This PR adds chat composer actions (GIF, emoji insertion, location sharing), introduces timed location-sharing via the connector, and changes map marker handling to deduplicate/update shared location markers rather than only appending new ones.
Changes:
- Added a “+” popup menu to chat/channel composers for GIFs, emoji insertion, and location sharing (single-shot or timed).
- Implemented timed location sharing in
MeshCoreConnectorand adjusted retry behavior to optionally clear paths on max retry. - Updated shared-marker collection to deduplicate markers by a stable key and display “shared at” timestamp; updated UTF-8 limiter to account for SMAZ compression.
Reviewed changes
Copilot reviewed 44 out of 46 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/widgets/emoji_picker.dart | Adds optional title to reuse picker for “insert emoji” vs “add reaction”. |
| lib/services/message_retry_service.dart | Adds forceClearPathOnMaxRetry tracking and applies it on max-retry failure. |
| lib/screens/map_screen.dart | Deduplicates shared markers by key, retains latest timestamp, shows “shared at”. |
| lib/screens/chat_screen.dart | Reworks composer into popup actions; adds emoji insertion + location sharing UI; updates byte limiting for SMAZ. |
| lib/screens/channel_chat_screen.dart | Mirrors composer popup actions for channels; adds location sharing flows and SMAZ-aware limiting. |
| lib/connector/meshcore_connector.dart | Adds timed location sharing state + periodic send logic; factors prepareChannelOutboundText. |
| lib/helpers/utf8_length_limiter.dart | Adds truncateToUtf8Bytes and supports optional encoder for effective byte length. |
| lib/helpers/reaction_helper.dart | Removes trailing whitespace (formatting-only). |
| lib/helpers/gif_helper.dart | Removes trailing whitespace (formatting-only). |
| lib/l10n/app_localizations.dart | Adds new localization keys for emoji insertion, location sharing, and “shared at”. |
| lib/l10n/app_localizations_en.dart | English strings for new chat/map labels. |
| lib/l10n/app_localizations_de.dart | German strings for new chat/map labels. |
| lib/l10n/app_localizations_es.dart | Spanish strings for new chat/map labels. |
| lib/l10n/app_localizations_fr.dart | French strings for new chat/map labels. |
| lib/l10n/app_localizations_hu.dart | Hungarian strings for new chat/map labels. |
| lib/l10n/app_localizations_it.dart | Italian strings for new chat/map labels. |
| lib/l10n/app_localizations_ja.dart | Japanese strings for new chat/map labels. |
| lib/l10n/app_localizations_ko.dart | Korean strings for new chat/map labels. |
| lib/l10n/app_localizations_nl.dart | Dutch strings for new chat/map labels. |
| lib/l10n/app_localizations_pl.dart | Polish strings for new chat/map labels. |
| lib/l10n/app_localizations_pt.dart | Portuguese strings for new chat/map labels. |
| lib/l10n/app_localizations_ru.dart | Russian strings for new chat/map labels. |
| lib/l10n/app_localizations_sk.dart | Slovak strings for new chat/map labels. |
| lib/l10n/app_localizations_sl.dart | Slovenian strings for new chat/map labels. |
| lib/l10n/app_localizations_sv.dart | Swedish strings for new chat/map labels. |
| lib/l10n/app_localizations_uk.dart | Ukrainian strings for new chat/map labels. |
| lib/l10n/app_en.arb | Adds new English ARB keys. |
| lib/l10n/app_de.arb | Adds new German ARB keys. |
| lib/l10n/app_es.arb | Adds new Spanish ARB keys. |
| lib/l10n/app_fr.arb | Adds new French ARB keys. |
| lib/l10n/app_hu.arb | Adds new Hungarian ARB keys. |
| lib/l10n/app_it.arb | Adds new Italian ARB keys. |
| lib/l10n/app_ja.arb | Adds new Japanese ARB keys. |
| lib/l10n/app_ko.arb | Adds new Korean ARB keys. |
| lib/l10n/app_nl.arb | Adds new Dutch ARB keys. |
| lib/l10n/app_pl.arb | Adds new Polish ARB keys. |
| lib/l10n/app_pt.arb | Adds new Portuguese ARB keys. |
| lib/l10n/app_ru.arb | Adds new Russian ARB keys. |
| lib/l10n/app_sk.arb | Adds new Slovak ARB keys. |
| lib/l10n/app_sl.arb | Adds new Slovenian ARB keys. |
| lib/l10n/app_sv.arb | Adds new Swedish ARB keys. |
| lib/l10n/app_uk.arb | Adds new Ukrainian ARB keys. |
| lib/l10n/app_bg.arb | Adds new Bulgarian ARB keys. |
| lib/l10n/app_zh.arb | Adds new Chinese ARB keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/screens/chat_screen.dart
Outdated
| final gpsInterval = | ||
| int.tryParse(connector.currentCustomVars?['gps_interval'] ?? '') ?? 900; | ||
| final minIntervals = (300.0 / gpsInterval).ceil().clamp(2, 9999); | ||
| final sliderMax = ((86400 / gpsInterval).floor() - minIntervals + 1).clamp( | ||
| 1, | ||
| 99999, | ||
| ); |
There was a problem hiding this comment.
gps_interval can parse to 0, which will cause a division-by-zero in (300.0 / gpsInterval) and (86400 / gpsInterval). Guard against gpsInterval <= 0 (e.g., fallback to 900 or clamp to >= 1) before doing these calculations.
| Future<void> sendMessageWithRetry({ | ||
| required Contact contact, | ||
| required String text, | ||
| String? originalText, | ||
| String? translatedLanguageCode, | ||
| String? translationModelId, | ||
| Uint8List? pathBytes, | ||
| int? pathLength, | ||
| bool forceClearPathOnMaxRetry = false, | ||
| }) async { |
There was a problem hiding this comment.
New behavior: forceClearPathOnMaxRetry changes failure handling by clearing the contact path even when the user setting is off. There are existing tests for MessageRetryService, but none appear to cover this new flag; adding a unit test that asserts clearContactPath is invoked when forceClearPathOnMaxRetry is true would help prevent regressions.
| final payload = _parseMarkerText(message.text); | ||
| if (payload == null) continue; | ||
| final fromName = message.isOutgoing ? selfName : contact.name; | ||
| final id = _buildMarkerId( | ||
| final key = _buildSharedMarkerKey( | ||
| sourceId: contact.publicKeyHex, | ||
| timestamp: message.timestamp, | ||
| text: message.text, | ||
| label: payload.label, | ||
| fromName: fromName, | ||
| flags: payload.flags, | ||
| isChannel: false, |
There was a problem hiding this comment.
fromName is used as part of the dedup key, but for room-server contacts (advTypeRoom) fromName is currently set to contact.name for all incoming messages. That can cause incorrect deduplication (and mislabeling) of markers from different room participants. Consider resolving the actual sender for room messages (e.g., via message.fourByteRoomContactKey, similar to ChatScreen) when building the shared-marker key/display name.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 46 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String _buildSharedMarkerKey({ | ||
| required String sourceId, | ||
| required DateTime timestamp, | ||
| required String text, | ||
| required String label, | ||
| required String fromName, | ||
| required String flags, |
There was a problem hiding this comment.
The shared-marker key currently depends on fromName (a display string that can change if a contact renames themselves or if the local selfName changes). Because marker.id is also used for _hiddenMarkerIds and persisted _removedMarkerIds, a name change will effectively change marker IDs and cause previously hidden/removed markers to reappear. Consider using a stable sender identifier in the key instead (e.g., for DMs use a fixed token like self vs contact:<pubKeyHex> based on message.isOutgoing; for channels prefer message.senderKeyHex when available, falling back to senderName only if the key is absent).
| String? contactKey, | ||
| int? channelIndex, | ||
| required Duration duration, | ||
| }) { |
There was a problem hiding this comment.
startLocationSharing accepts both contactKey and channelIndex as optional params but does not validate that exactly one is provided (or that duration is positive). Adding asserts/argument validation (XOR check + duration > Duration.zero) would prevent accidental misuse where both/neither targets are set, which would otherwise silently pick the contact branch or do nothing.
| }) { | |
| }) : assert( | |
| (contactKey != null) != (channelIndex != null), | |
| 'Exactly one of contactKey or channelIndex must be provided.', | |
| ), | |
| assert( | |
| duration > Duration.zero, | |
| 'duration must be greater than Duration.zero.', | |
| ) { | |
| if ((contactKey != null) == (channelIndex != null)) { | |
| throw ArgumentError( | |
| 'Exactly one of contactKey or channelIndex must be provided.', | |
| ); | |
| } | |
| if (duration <= Duration.zero) { | |
| throw ArgumentError.value( | |
| duration, | |
| 'duration', | |
| 'Must be greater than Duration.zero.', | |
| ); | |
| } |
| void startLocationSharing({ | ||
| String? contactKey, | ||
| int? channelIndex, | ||
| required Duration duration, | ||
| }) { | ||
| _locationSharingTimer?.cancel(); | ||
| _locationSharingEnd = DateTime.now().add(duration); | ||
| _locationSharingContactKey = contactKey; | ||
| _locationSharingChannelIndex = channelIndex; | ||
| _lastSharedLocationPositionKey = null; | ||
| final rawGpsInterval = | ||
| int.tryParse(_currentCustomVars?['gps_interval'] ?? '') ?? 900; | ||
| final gpsInterval = rawGpsInterval > 0 ? rawGpsInterval : 900; | ||
| _sendLocationOnce(contactKey, channelIndex); | ||
| _locationSharingTimer = Timer.periodic(Duration(seconds: gpsInterval), (_) { | ||
| if (!isConnected || DateTime.now().isAfter(_locationSharingEnd!)) { | ||
| stopLocationSharing(); | ||
| return; | ||
| } | ||
| _sendLocationOnce(contactKey, channelIndex); | ||
| }); | ||
| notifyListeners(); |
There was a problem hiding this comment.
Location sharing introduces timer-driven behavior with stop conditions (duration elapsed, disconnect) and suppression of repeated unchanged positions. Given there are already tests covering MeshCoreConnector behaviors, it would be good to add unit tests (e.g. with fake_async) verifying: (1) the timer stops at/after _locationSharingEnd, (2) stopLocationSharing() clears state + notifies listeners, and (3) _sendLocationOnce suppresses unchanged channel locations.
|
it might be better to break this into smaller changes. one commit per PR, one coherent change. so one for smaz aware utf length limit, one for share location, one for emoji picker etc etc. get the one any others rely on merged first then build on dev again for the next. it will make getting things merged in much quicker. |
|
I will try it, but it gets a little complicated because they depend on each other. |
ok maybe let's get the counter PR in. then maybe you can split the smaz aware part out and get that in, then the rest as one add new input to messages PR |
Created a Popup for the input field, where you can select to:
The Share Location is only enabled if GPS is enabled on companion.
It has two modes:
It only works for Contacts or PRIVATE channels (and would be best to limit per scope as soon as thats implemented):
For Private channels only sends when location has changed, and does no retry.
For Contacts it will try to use path and if this is not working use a single floot message in the next interval.
If you loose Bluetooth connection, sharing will be aborted. The label of the marker will always be the Companion name.
For this to work markers get now updated instead of only added (sender, label and flags have to be the same)
Since working on the input field, I also modified the length limitter to allow full size on smaz compression.
Sincerely Eric