Show overlaps in public keys of repeaters by wel97459 · Pull Request #218 · zjs81/meshcore-open

wel97459 · 2026-02-23T00:03:08Z

Show overlaps in public keys of repeaters on the map.
Added a function to handle filtering markers placed on the map.

…ization support

Copilot

Pull request overview

This PR adds functionality to visualize overlaps in public keys of repeaters on the map, where overlaps are detected by comparing the first byte of public keys. The feature helps identify potential routing conflicts when multiple repeaters/rooms share the same first byte in their public key. Additionally, the PR includes UI improvements to the path trace functionality, converting text buttons to icon buttons and adding a separate "return path" option.

Changes:

Added mapShowOverlaps setting to show only nodes with overlapping public key first bytes
Refactored marker filtering logic into a dedicated _filterContactsBySettings method
Enhanced path trace UI with icon buttons and separate forward/return path options

Reviewed changes

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

Show a summary per file

File	Description
lib/services/app_settings_service.dart	Added `setMapShowOverlaps` method to update the new overlap display setting
lib/screens/map_screen.dart	Core changes: new filtering logic, overlap detection, updated path trace UI with icon buttons, and overlap visualization (red markers with key prefix labels)
lib/models/app_settings.dart	Added `mapShowOverlaps` boolean field with serialization support
lib/l10n/app_*.arb	Added localized strings for "Repeater Key Overlaps" across all 15 supported languages
lib/l10n/app_localizations*.dart	Generated localization code for the new string key

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

Copilot · 2026-02-23T00:09:24Z

lib/screens/map_screen.dart

+      final hasOverlap = contacts
+          .where(
+            (c) =>
+                c.publicKeyHex != contact.publicKeyHex &&
+                c.publicKey.first == contact.publicKey.first &&
+                (c.type == advTypeRepeater || c.type == advTypeRoom) &&
+                (contact.type == advTypeRepeater ||
+                    contact.type == advTypeRoom),
+          )
+          .firstOrNull;


The overlap detection only compares the first byte of the public key using c.publicKey.first == contact.publicKey.first. According to the code at line 1676, path traces use only the first byte of public keys. However, this is a very weak collision detection - with only 256 possible values for the first byte, false positives are highly likely in any reasonably sized network.

Consider whether this is intentional (to match the path trace behavior) or if a more robust comparison is needed. If intentional, this should be documented in a comment explaining why only the first byte is compared.

Copilot · 2026-02-23T00:09:24Z

lib/screens/map_screen.dart

+                (c.type == advTypeRepeater || c.type == advTypeRoom) &&
+                (contact.type == advTypeRepeater ||
+                    contact.type == advTypeRoom),


The condition checks if both contacts are either advTypeRepeater OR advTypeRoom. However, based on the PR title "Show overlaps in public keys of repeaters" and the English localization "Repeater Key Overlaps", this feature appears to be specifically for repeaters. Including rooms in the overlap detection may be intentional, but it's inconsistent with the user-facing messaging which only mentions repeaters. Consider either:

Updating the UI text to mention both repeaters and rooms

Restricting the check to only repeaters if rooms were included by mistake

Copilot · 2026-02-23T00:09:24Z

lib/screens/map_screen.dart

+      final hasOverlap = contacts
+          .where(
+            (c) =>
+                c.publicKeyHex != contact.publicKeyHex &&
+                c.publicKey.first == contact.publicKey.first &&
+                (c.type == advTypeRepeater || c.type == advTypeRoom) &&
+                (contact.type == advTypeRepeater ||
+                    contact.type == advTypeRoom),
+          )
+          .firstOrNull;


This code has O(n²) time complexity because the overlap detection runs a full scan of the contacts list for every contact being filtered. For large contact lists, this could cause noticeable performance degradation.

Consider optimizing by:

Pre-computing overlaps once before the filtering loop

Building a map of first-byte to contacts for O(1) lookup

Or moving the overlap computation outside the filter function and storing it as metadata on contacts

Copilot · 2026-02-23T00:09:25Z

lib/screens/map_screen.dart

+                      tooltip: "Path Trace",
+                      icon: const Icon(Icons.arrow_forward_outlined),
+                    ),
+                  if (_pathTrace.isNotEmpty)
+                    IconButton(
                      onPressed: () {
                        Navigator.push(
                          context,
                          MaterialPageRoute(
                            builder: (context) => PathTraceMapScreen(
                              title: l10n.contacts_pathTrace,
                              path: Uint8List.fromList(_pathTrace),
+                              flipPathRound: true,
                            ),
                          ),
                        );
                        setState(() {
                          _isBuildingPathTrace = false;
                        });
                      },
-                      child: Text(l10n.map_runTrace),
+                      tooltip: "Build Return Path",
+                      icon: const Icon(Icons.replay),
                    ),
                  if (_pathTrace.isNotEmpty)
-                    ElevatedButton(
+                    IconButton(
                      onPressed: _removePath,
-                      child: Text(l10n.map_removeLast),
+                      tooltip: "Remove Last Point",
+                      icon: const Icon(Icons.delete),


The tooltip strings "Path Trace", "Build Return Path", and "Remove Last Point" are hardcoded in English instead of using localization. This is inconsistent with the rest of the codebase which uses context.l10n for all user-facing strings. These should use localized strings from the l10n files.

Copilot · 2026-02-23T00:09:25Z

lib/screens/map_screen.dart

                style: TextStyle(fontSize: 18),
              ),
-              const SizedBox(height: 6),
+              // const SizedBox(height: 6),


This line is commented out but left in the code. If this spacing adjustment is no longer needed due to the button changes, the commented line should be removed entirely to keep the code clean. If it might be needed later, add a comment explaining why it's kept.

Copilot · 2026-02-23T00:09:25Z

lib/screens/map_screen.dart

+          .where(
+            (c) =>
+                c.publicKeyHex != contact.publicKeyHex &&
+                c.publicKey.first == contact.publicKey.first &&


The code accesses publicKey.first without checking if the Uint8List is non-empty. While publicKey is a required non-nullable field, an empty Uint8List could still be passed, which would cause a runtime error when accessing .first. Consider adding a safety check or documenting that publicKey must never be empty.

Copilot · 2026-02-23T00:09:25Z

lib/screens/map_screen.dart

+            label: settings.mapShowOverlaps && !_isBuildingPathTrace
+                ? "${contact.publicKeyHex.substring(0, 2)}:${contact.name}"


The code calls substring(0, 2) on publicKeyHex without checking its length. If publicKeyHex is shorter than 2 characters, this will throw a RangeError. While public keys are typically of a fixed length, consider adding a safety check or using a safer substring method like substring(0, min(2, publicKeyHex.length)).

Copilot · 2026-02-23T00:09:26Z

lib/screens/map_screen.dart

+    List<Contact> filtered = [];
+    bool addContact = false;
    for (final contact in contacts) {
+      addContact = false;
      if (!contact.hasLocation) continue;

      // Apply node type filters
      if (contact.type == advTypeRepeater &&
-          (!settings.mapShowRepeaters && !_isBuildingPathTrace)) {
-        continue;
+          (settings.mapShowRepeaters ||
+              _isBuildingPathTrace ||
+              settings.mapShowOverlaps)) {
+        addContact = true;
      }
      if (contact.type == advTypeChat &&
-          !(settings.mapShowChatNodes && !_isBuildingPathTrace)) {
-        continue;
+          (settings.mapShowChatNodes || _isBuildingPathTrace)) {
+        addContact = true;
      }
      if (contact.type != advTypeChat &&
          contact.type != advTypeRepeater &&
-          (!settings.mapShowOtherNodes && !_isBuildingPathTrace)) {
-        continue;
+          (settings.mapShowOtherNodes ||
+              _isBuildingPathTrace ||
+              settings.mapShowOverlaps)) {
+        addContact = true;
+      }
+
+      final hasOverlap = contacts
+          .where(
+            (c) =>
+                c.publicKeyHex != contact.publicKeyHex &&
+                c.publicKey.first == contact.publicKey.first &&
+                (c.type == advTypeRepeater || c.type == advTypeRoom) &&
+                (contact.type == advTypeRepeater ||
+                    contact.type == advTypeRoom),
+          )
+          .firstOrNull;
+
+      if (hasOverlap == null &&
+          settings.mapShowOverlaps &&
+          !_isBuildingPathTrace) {
+        addContact = false;
      }

+      if (addContact) {
+        filtered.add(contact);
+      }
+    }


The filtering logic has several design issues:

When mapShowOverlaps is enabled, it overrides individual node type filters by including repeaters and other nodes even if those filters are disabled (lines 530, 541). This means users will see repeaters/other nodes even if they've explicitly disabled those checkboxes in the UI. This creates a confusing user experience.

The addContact boolean is set by multiple independent conditions, making the logic hard to follow. A contact is added if ANY of the node type conditions are true, which isn't immediately clear from the structure.

The overlap detection at lines 556-559 acts as an exclusive filter - when enabled with no overlap found, it overrides the earlier addContact = true decisions. This means mapShowOverlaps acts as "show ONLY overlaps" rather than "highlight overlaps".

Consider refactoring to make the intent clearer, perhaps separating "which nodes to consider" from "which nodes to display" logic. Also consider whether overriding individual node type filters is the desired UX.

Add show overlaps in public keys of repeaters functionality and local…

ebe490b

…ization support

Copilot AI review requested due to automatic review settings February 23, 2026 00:03

Copilot started reviewing on behalf of wel97459 February 23, 2026 00:03 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Show overlaps in public keys of repeaters#218

Show overlaps in public keys of repeaters#218
wel97459 wants to merge 1 commit intomainfrom
dev-mapOverlap

wel97459 commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		label: settings.mapShowOverlaps && !_isBuildingPathTrace
		? "${contact.publicKeyHex.substring(0, 2)}:${contact.name}"

Comments

Conversation

wel97459 commented Feb 23, 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 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant