Implement PathTraceMapScreen and refactor path tracing functionality by wel97459 · Pull Request #141 · zjs81/meshcore-open

wel97459 · 2026-02-08T19:35:25Z

This replaces the path tracing UI, with a map to show the path. a long with a list of the hops and there signal reports

…across screens

Copilot

Pull request overview

This PR replaces the existing path tracing dialog UI with a dedicated map-based screen that visualizes the traced route and provides a hop/SNR list, and updates entry points across the app to navigate to the new screen.

Changes:

Remove the legacy PathTraceDialog implementation.
Add PathTraceMapScreen using flutter_map, including hop markers, polylines, and hop/SNR legend.
Update contacts/chat/channel-path screens to navigate to the new map-based path trace UI.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
lib/widgets/path_trace_dialog.dart	Removes the old dialog-based path trace UI.
lib/screens/path_trace_map.dart	Adds the new map-based path trace screen and trace parsing logic.
lib/screens/contacts_screen.dart	Replaces dialog open with navigation to `PathTraceMapScreen`.
lib/screens/chat_screen.dart	Adds a “Path Trace” action that navigates to `PathTraceMapScreen`.
lib/screens/channel_message_path_screen.dart	Adds navigation to `PathTraceMapScreen` and enhances map UI with self marker + distance display.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T19:39:48Z

+      // Check if it's a binary response
+      if (code == pushCodeTraceData &&
+          listEquals(frame.sublist(4, 8), tagData)) {
+        _timeoutTimer?.cancel();
+        if (!mounted) return;
+        frameBuffer.skipBytes(3); //reserved + path length + flag
+        if (listEquals(frameBuffer.readBytes(4), tagData)) {
+          _handleTraceResponse(frame);
+        }


frame.sublist(4, 8) will throw a RangeError if a malformed/short frame arrives (only frame.isEmpty is checked). Add a minimum-length guard (e.g., if (frame.length < 8) return;) before slicing or reading fields from the frame buffer.

Copilot · 2026-02-08T19:39:48Z

+      _points = <LatLng>[];
+      _points.add(LatLng(connector.selfLatitude!, connector.selfLongitude!));
+      for (final hop in _traceData!.pathData) {
+        final contact = _traceData!.pathContacts[hop];
+        if (contact != null && contact.hasLocation) {
+          _points.add(LatLng(contact.latitude!, contact.longitude!));
+        }


connector.selfLatitude! / selfLongitude! are nullable in MeshCoreConnector, so forcing them with ! can crash if the user/device hasn't set a location yet. Consider only adding the self point/marker when both coordinates are non-null, and show the existing “no repeater locations”/“not available” UI otherwise.

Copilot · 2026-02-08T19:39:48Z

+      for (final hop in pathData)
+        if (_traceData!.pathContacts[hop]!.hasLocation)
+          Marker(
+            point: LatLng(
+              _traceData!.pathContacts[hop]!.latitude!,
+              _traceData!.pathContacts[hop]!.longitude!,
+            ),


_traceData!.pathContacts[hop]! can be null when a hop byte doesn’t match any known contact (you already handle this case earlier when building _points). This will crash when building markers. Use a null-safe lookup (final contact = _traceData?.pathContacts[hop]; if (contact?.hasLocation == true) ...) instead of force-unwrapping.

Copilot · 2026-02-08T19:39:48Z

+    final frame = buildTraceReq(
+      DateTime.now().millisecondsSinceEpoch ~/ 1000,
+      0, //flags
+      0, //auth


The buildTraceReq signature is (tag, auth, flag, ...), but the call site comments label the second argument as //flags and third as //auth. This is misleading and could cause a real bug if either value becomes non-zero. Update the argument comments (or use named parameters / local vars) so auth and flag are unambiguous.

Suggested change

final frame = buildTraceReq(

DateTime.now().millisecondsSinceEpoch ~/ 1000,

0, //flags

0, //auth

final int auth = 0;

final int flag = 0;

final frame = buildTraceReq(

DateTime.now().millisecondsSinceEpoch ~/ 1000,

auth,

flag,

Copilot · 2026-02-08T19:39:49Z

+        final timeoutSeconds = frameBuffer.readUInt32LE();
+
+        // Start timeout timer for trace response
+        _timeoutTimer?.cancel();
+        _timeoutTimer = Timer(Duration(milliseconds: timeoutSeconds), () {


timeoutSeconds is read from the RESP_CODE_SENT frame and is logged elsewhere as milliseconds (e.g., debug log prints timeout=...ms). The variable name here is misleading since it’s passed to Duration(milliseconds: ...). Rename it to timeoutMs (and adjust any UI text accordingly) to avoid confusion.

Suggested change

final timeoutSeconds = frameBuffer.readUInt32LE();

// Start timeout timer for trace response

_timeoutTimer?.cancel();

_timeoutTimer = Timer(Duration(milliseconds: timeoutSeconds), () {

final timeoutMs = frameBuffer.readUInt32LE();

// Start timeout timer for trace response

_timeoutTimer?.cancel();

_timeoutTimer = Timer(Duration(milliseconds: timeoutMs), () {

Copilot · 2026-02-08T19:39:49Z

+            points.add(hop.position!);
+          }
+        }
+        points.add(LatLng(connector.selfLatitude!, connector.selfLongitude!));


connector.selfLatitude! / selfLongitude! are nullable, but this code force-unwraps them when building points. This introduces a new crash path (previous code only used hop locations). Only append the self point when both coordinates are non-null, or provide a safe fallback.

Suggested change

points.add(LatLng(connector.selfLatitude!, connector.selfLongitude!));

if (connector.selfLatitude != null && connector.selfLongitude != null) {

points.add(LatLng(connector.selfLatitude!, connector.selfLongitude!));

}

Copilot · 2026-02-08T19:39:49Z

+        final mapKey = ValueKey(
+          '${_formatPathPrefixes(selectedPath)},${context.l10n.pathTrace_you}',
+        );
+        _pathDistance = _getPathDistance(points);


Avoid mutating state (_pathDistance = ...) during build(). This can lead to hard-to-reason-about rebuild behavior and makes the value dependent on build order. Compute the distance as a local final pathDistance = ... and pass it down, or update state in setState when the selected path changes.

Suggested change

_pathDistance = _getPathDistance(points);

final pathDistance = _getPathDistance(points);

if (_pathDistance != pathDistance) {

WidgetsBinding.instance.addPostFrameCallback((_) {

if (!mounted) return;

setState(() {

_pathDistance = pathDistance;

});

});

}

Copilot · 2026-02-08T19:39:49Z

+      Marker(
+        point: LatLng(
+          context.read<MeshCoreConnector>().selfLatitude!,
+          context.read<MeshCoreConnector>().selfLongitude!,
+        ),


This marker force-unwraps selfLatitude! / selfLongitude! even though self location is nullable. If the device/user hasn’t set a location, the map screen will crash. Make the self marker conditional on non-null coordinates (or derive it from an always-present position source).

Copilot

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T20:31:17Z

+        final timeoutSeconds = frameBuffer.readUInt32LE();
+
+        // Start timeout timer for trace response
+        _timeoutTimer?.cancel();
+        _timeoutTimer = Timer(Duration(milliseconds: timeoutSeconds), () {
+          if (!mounted) return;


timeoutSeconds is read from the RESP_CODE_SENT frame and then used as Duration(milliseconds: ...). Elsewhere in the codebase this field is treated as milliseconds, so the variable name is misleading and makes it easy to introduce unit bugs. Rename to something like timeoutMs (or convert to seconds and use Duration(seconds: ...) consistently).

Copilot · 2026-02-08T20:31:17Z

+      for (final hop in pathData)
+        if (_traceData!.pathContacts[hop]!.hasLocation)
+          Marker(
+            point: LatLng(
+              _traceData!.pathContacts[hop]!.latitude!,
+              _traceData!.pathContacts[hop]!.longitude!,
+            ),


_traceData!.pathContacts[hop]! assumes every hop byte maps to a known Contact. That mapping is built opportunistically from connector.contacts, so it can legitimately be missing (e.g., unknown hop prefix / contacts not loaded yet), which would throw at runtime. Guard for a null contact before accessing .hasLocation/.latitude/.longitude and skip markers for unknown hops.

Copilot · 2026-02-08T20:31:18Z

+        point: LatLng(
+          context.read<MeshCoreConnector>().selfLatitude!,
+          context.read<MeshCoreConnector>().selfLongitude!,
+        ),


The "You" marker force-unwraps selfLatitude/selfLongitude, which are nullable on MeshCoreConnector. This can crash when the device location isn't available. Use a null-safe fallback or omit the marker until self location is known.

Copilot · 2026-02-08T20:31:18Z

+                child: Text(
+                  '${l10n.channelPath_repeaterHops} (${(_pathDistance / 1609.34).toStringAsFixed(2)} Miles / ${(_pathDistance / 1000).toStringAsFixed(2)} Km)',
+                  style: const TextStyle(fontWeight: FontWeight.w600),


The distance label is built with hard-coded English units ("Miles / Km") and hard-coded conversion constants. This won't localize correctly and is hard to maintain. Consider moving the full label (including units) into l10n and/or using locale-aware formatting (and potentially a user preference for metric/imperial).

Copilot · 2026-02-08T20:31:18Z

+          }
+        }
+        points.add(LatLng(connector.selfLatitude!, connector.selfLongitude!));
+


selfLatitude/selfLongitude are nullable on MeshCoreConnector, but they are force-unwrapped when appending the "self" point. This can crash the map screen when location is unknown. Handle nulls (e.g., only add the self point when both values are present).

Copilot · 2026-02-08T20:31:18Z

+        final mapKey = ValueKey(
+          '${_formatPathPrefixes(selectedPath)},${context.l10n.pathTrace_you}',
+        );
+        _pathDistance = _getPathDistance(points);



_pathDistance is mutated inside build(). Updating State fields during build makes rebuild behavior harder to reason about and can lead to subtle UI inconsistencies. Compute the distance as a local final and pass it into _buildLegendCard (or store it via setState in response to path selection changes).

Copilot · 2026-02-08T20:31:18Z

+                if (_noLocationErr)
+                  Center(
+                    child: Card(
+                      color: Colors.red,
+                      child: Padding(
+                        padding: EdgeInsets.all(12),
+                        child: Text(
+                          context.l10n.pathTrace_someHopsNoLocation,
+                          style: TextStyle(color: Colors.white),
+                        ),
+                      ),
+                    ),
+                  ),
+                if (!_hasData && _noLocationErr)
+                  Center(
+                    child: Column(
+                      mainAxisSize: MainAxisSize.min,
+                      children: [
+                        if (_isLoading) const CircularProgressIndicator(),
+                        const SizedBox(height: 16),
+                        if (!_isLoading && _failed2Loaded)
+                          Text(context.l10n.pathTrace_notAvailable),
+                      ],
+                    ),
+                  ),
+                if (_hasData && !_noLocationErr)
+                  FlutterMap(


When any hop is missing location, _noLocationErr is set to true and the UI condition if (_hasData && !_noLocationErr) prevents the map and legend from rendering at all, even though _points still contains the hops that do have locations. This makes the "some hops missing location" state effectively hide the main content. Consider always rendering the map/legend when at least 2 points are available, and show the warning as an overlay instead of a hard gate.

Copilot · 2026-02-08T20:31:19Z

                child: Text(
-                  l10n.channelPath_repeaterHops,
+                  '${l10n.channelPath_repeaterHops} (${(_pathDistance / 1609.34).toStringAsFixed(2)} Miles / ${(_pathDistance / 1000).toStringAsFixed(2)} Km)',
                  style: const TextStyle(fontWeight: FontWeight.w600),


This UI string hard-codes "Miles / Km" and conversion constants directly in the widget text, which won't localize and is difficult to maintain. Prefer a localized message (and locale-aware number formatting / unit selection) instead of embedding units and conversion math in the UI.

Copilot · 2026-02-08T20:31:19Z

  "pathTrace_failed": "Path trace failed.",
  "pathTrace_notAvailable": "Path trace not available.",
  "pathTrace_refreshTooltip": "Refresh Path Trace.",
+  "pathTrace_someHopsNoLocation": "One or more of the hops is missing a location!",


Grammar: "One or more of the hops is missing a location!" should use a plural verb ("are missing") to agree with "one or more".

Suggested change

"pathTrace_someHopsNoLocation": "One or more of the hops is missing a location!",

"pathTrace_someHopsNoLocation": "One or more of the hops are missing a location!",

446564 · 2026-02-08T21:52:47Z

I think my last ask would be for single repeater ping have the window just show both there and back. But that ca be later as it's still alpha

Implement PathTraceMapScreen and refactor path tracing functionality …

98e0b05

…across screens

Copilot AI review requested due to automatic review settings February 8, 2026 19:35

Copilot started reviewing on behalf of wel97459 February 8, 2026 19:35 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

wel97459 added 2 commits February 8, 2026 11:57

Add localization for missing location error in path tracing

2f4b230

Add scrollbar to path trace details list for improved navigation

0d8801f

Copilot AI review requested due to automatic review settings February 8, 2026 20:26

Copilot started reviewing on behalf of wel97459 February 8, 2026 20:26 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

446564 approved these changes Feb 8, 2026

View reviewed changes

wel97459 merged commit c365b78 into main Feb 9, 2026
18 checks passed

-        _pathDistance = _getPathDistance(points);
+        final pathDistance = _getPathDistance(points);
+        if (_pathDistance != pathDistance) {
+          WidgetsBinding.instance.addPostFrameCallback((_) {
+            if (!mounted) return;
+            setState(() {
+              _pathDistance = pathDistance;
+            });
+          });
+        }

	"pathTrace_someHopsNoLocation": "One or more of the hops is missing a location!",
	"pathTrace_someHopsNoLocation": "One or more of the hops are missing a location!",

Conversation

wel97459 commented Feb 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

446564 commented Feb 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants