Skip to content

Remove redundant map refresh calls and optimize refresh triggers#161

Merged
MrAlders0n merged 3 commits into
devfrom
copilot/remove-redundant-map-refresh
Dec 30, 2025
Merged

Remove redundant map refresh calls and optimize refresh triggers#161
MrAlders0n merged 3 commits into
devfrom
copilot/remove-redundant-map-refresh

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 30, 2025

After PR #160 implemented the Map Refresh Service as an always-on background service (5s timer + 25m movement triggers), redundant scheduleCoverageRefresh() calls remained in the TX ping flow, causing duplicate map refreshes. Additionally, the 5-second timer-based refresh was inefficient and not aligned with actual data updates.

Changes

Phase 1: Remove Redundant Map Refresh Calls

Removed redundant map refresh calls from:

  • postApiInBackground() - Eliminated setTimeout wrapper with conditional refresh after API post
  • postApiAndRefreshMap() - Eliminated setTimeout wrapper with conditional refresh after message queueing
  • primeGpsOnce() - Eliminated conditional refresh after initial GPS acquisition

Before:

// TX ping completes → triggers immediate refresh
if (state.capturedPingCoords) {
  scheduleCoverageRefresh(lat, lon, MAP_REFRESH_DELAY_MS);
}

After:

// Map refresh is handled by the Map Refresh Service (25m movement or API flush)
// No manual refresh needed here

Phase 2: Optimize Map Refresh Triggers

Changed map refresh triggers from:

  • ❌ 5-second timer (arbitrary polling)
  • ✅ 25-meter movement

To:

  • ✅ 25-meter movement (GPS-based)
  • ✅ API flush completion (data-driven)

Implementation:

  • Removed MAP_REFRESH_INTERVAL_MS constant and timer management
  • Added checkAndRefreshMap('api_flush') trigger after successful API batch post
  • Updated trigger types: 'initial' | 'gps' | 'api_flush'
  • Simplified Map Refresh Service (no setInterval timer needed)

Benefits

  • Eliminates duplicate refreshes in the TX ping lifecycle
  • More efficient - map updates only when data is posted or user moves significantly
  • Better UX - map shows latest coverage aligned with actual API data updates
  • Simpler code - removed timer management complexity

Net: -35 lines total (66 deletions, 31 insertions)

Original prompt

Problem

After merging PR #160 (Phase 1: UI Refactor + RX Auto Mode + Always-On Services), the Map Refresh Service was implemented as an "always-on" background service that refreshes the coverage map based on:

  • Timer trigger: Every 5 seconds (MAP_REFRESH_INTERVAL_MS = 5000)
  • GPS trigger: When user moves 25 meters (MAP_REFRESH_DISTANCE_M = 25)

However, a redundant scheduleCoverageRefresh() call still exists in the sendPing() function in content/wardrive.js. This causes an immediate map refresh after every TX ping, which defeats the purpose of the background Map Refresh Service.

Current Behavior

After a TX ping completes, the following code triggers an immediate map refresh:

// In sendPing() function, after RX window completion:
if (state.capturedPingCoords) {
  const { lat, lon } = state.capturedPingCoords;
  debugLog(`[PING] Scheduling coverage map refresh for coordinates: lat=${lat.toFixed(5)}, lon=${lon.toFixed(5)}`);
  scheduleCoverageRefresh(lat, lon, MAP_REFRESH_DELAY_MS);
}

This results in duplicate map refreshes - one from the TX ping and one from the Map Refresh Service.

Expected Behavior

The Map Refresh Service should be the only mechanism for refreshing the coverage map. TX pings should not trigger map refreshes directly. The background service will handle map updates based on:

  1. 5-second interval timer
  2. 25-meter movement threshold

Fix Required

Remove the scheduleCoverageRefresh() call from the sendPing() function in content/wardrive.js. The block of code to remove should look like:

if (state.capturedPingCoords) {
  const { lat, lon } = state.capturedPingCoords;
  debugLog(`[PING] Scheduling coverage map refresh for coordinates: lat=${lat.toFixed(5)}, lon=${lon.toFixed(5)}`);
  scheduleCoverageRefresh(lat, lon, MAP_REFRESH_DELAY_MS);
}

Also remove the associated state.capturedPingCoords cleanup if it's no longer needed after this change.

Files to Modify

  • content/wardrive.js - Remove the redundant scheduleCoverageRefresh() call in sendPing() function

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Removed scheduleCoverageRefresh() calls from postApiInBackground()
- Removed scheduleCoverageRefresh() calls from postApiAndRefreshMap()
- Removed scheduleCoverageRefresh() call from primeGpsOnce()
- Map Refresh Service is now the only mechanism for map updates
- Service handles refreshes via 5s timer and 25m movement triggers

Co-authored-by: MrAlders0n <55921894+MrAlders0n@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove redundant map refresh from sendPing function Remove redundant map refresh calls after TX pings Dec 30, 2025
Copilot AI requested a review from MrAlders0n December 30, 2025 03:28
@MrAlders0n
Copy link
Copy Markdown
Collaborator

@copilot Will the map still be refresh on GPS fix. I liked the on connect immedaitly update GPS to map

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 30, 2025

@copilot Will the map still be refresh on GPS fix. I liked the on connect immedaitly update GPS to map

Yes, the map still refreshes immediately on GPS fix during connection. The sequence is:

  1. primeGpsOnce() waits for initial GPS fix and sets state.lastFix
  2. startMapRefreshService() is called immediately after, which does an initial refresh using the GPS coordinates

The map will update just as quickly as before. The difference is the refresh now happens via the Map Refresh Service's initial refresh (line 934 in startMapRefreshService()) instead of inside primeGpsOnce() itself.

The Map Refresh Service continues to refresh the map every 5 seconds and when you move 25 meters, so the map stays current throughout your wardrive session.

- Removed 5-second timer-based map refresh
- Map now refreshes on 25m movement or successful API flush
- Updated trigger types: 'initial' | 'gps' | 'api_flush'
- Simplified Map Refresh Service (no timer management needed)
- Map updates only when data is actually posted to API

Co-authored-by: MrAlders0n <55921894+MrAlders0n@users.noreply.github.com>
Copilot AI changed the title Remove redundant map refresh calls after TX pings Remove redundant map refresh calls and optimize refresh triggers Dec 30, 2025
@MrAlders0n MrAlders0n marked this pull request as ready for review December 30, 2025 03:47
Copilot AI review requested due to automatic review settings December 30, 2025 03:47
@MrAlders0n MrAlders0n merged commit 1128a4f into dev Dec 30, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the map refresh system by eliminating redundant refresh calls and replacing inefficient timer-based polling with event-driven triggers. The changes remove duplicate map refreshes that occurred during the TX ping lifecycle and replace a 5-second polling timer with targeted refresh triggers based on actual data updates (25m GPS movement and API flush completion).

Key Changes

  • Removed redundant scheduleCoverageRefresh() calls from TX ping flow (postApiInBackground, postApiAndRefreshMap, primeGpsOnce)
  • Eliminated 5-second timer-based map refresh in favor of GPS movement (25m) and API flush triggers
  • Improved UI responsiveness by removing unnecessary setTimeout delays when unlocking ping controls

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

@MrAlders0n MrAlders0n deleted the copilot/remove-redundant-map-refresh branch December 30, 2025 12:13
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