Skip to content

feat(mqtt-bridge): interactive map editor for filter geographic bounding box#3064

Merged
Yeraze merged 2 commits into
mainfrom
feat/mqtt-bridge-bbox-map-editor
May 18, 2026
Merged

feat(mqtt-bridge): interactive map editor for filter geographic bounding box#3064
Yeraze merged 2 commits into
mainfrom
feat/mqtt-bridge-bbox-map-editor

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented May 18, 2026

Summary

Replace the four free-text bbox inputs on the MQTT Bridge source-config modal with a Leaflet-backed interactive editor. Click two corners to draw, drag any corner handle to resize, "Clear" to start over. The numeric inputs stay beneath the map for precise tuning and are two-way bound — drag the map, the numbers update; type a number, the rectangle re-renders.

Why

Pulling a useful Florida-sized bbox out of minLat / maxLat / minLng / maxLng text boxes required jumping over to Google Maps to read coordinates, then typing them back into the modal. The bbox is a fundamentally spatial filter, so editing it should be spatial too.

Design notes

  • New focused component src/components/BBoxMapEditor.tsx (not a variant of the existing GeofenceMapEditor). The auto-responder's GeofenceShape discriminated union (circle | polygon) already powers a separate geofence-triggers feature; growing it to include bbox would force changes across src/utils/geometry.ts, GeofenceTriggersSection.tsx, and the geofence test suite for no MQTT-side benefit. Keeping the MQTT filter's rectangular data model ({ minLat, maxLat, minLng, maxLng }) self-contained is cleaner.
  • Same Leaflet + OSM tile pattern as the existing GeofenceMapEditor for visual consistency.
  • Drag-corner UX: dragging any corner updates the bbox such that the OPPOSITE corner stays fixed (i.e. dragging SW updates minLat + minLng; NE stays put).
  • The bboxFromForm() helper tolerates partial / NaN values in the text inputs so a user mid-typing doesn't break the map.

Test plan

  • tsc --noEmit clean
  • Full vitest: 9967/9967 pass (no behavior change to filter pipeline; new component is presentational)
  • Manual smoke in dev container — open Add Source → MQTT Bridge → enable geo bbox → click two corners → drag corners → verify the four numeric inputs update in sync

🤖 Generated with Claude Code

…ing box

The mqtt_bridge filter's geographic bounding box previously had only
four free-text inputs (minLat / maxLat / minLng / maxLng). Pulling a
useful Florida-sized box out of those numbers required jumping over to
Google Maps or some other tool to read off coordinates, then typing
them back into the modal. That's a poor user experience for what is
fundamentally a spatial operation.

Adds `src/components/BBoxMapEditor.tsx`: a Leaflet-backed rectangular
bbox editor.

- No bbox set: hint reads "click two corners on the map to draw the
  bounding box". First click drops a yellow marker at corner A; mouse
  movement paints a dashed preview rectangle from A to the cursor; the
  second click finalizes the bbox.
- Defined bbox: rendered as a solid blue rectangle with four draggable
  corner handles (SW / NW / NE / SE). Dragging any handle resizes —
  the opposite corner stays fixed.
- Two-way bound with the four numeric inputs (kept beneath the map for
  precise tuning) via `bboxFromForm()` helper. Drag the map, the
  numbers update; type a number, the rectangle re-renders.
- "Clear" button reverts to undefined.

Wired into the existing geo-bbox fieldset on the Add/Edit Source modal
in DashboardPage, only rendering when the user has the "Restrict to
geographic bounding box" checkbox enabled.

`BBoxMapEditor` deliberately owns its own data model
(`{ minLat, maxLat, minLng, maxLng }`) instead of growing a bbox
variant on the auto-responder's `GeofenceShape` discriminated union.
That type powers a separate feature (geofence triggers) with its own
geometry helpers; the MQTT filter is rectangular by definition and
shouldn't pull in circles/polygons.

All 9967 vitest tests still pass; tsc clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Claude finished @Yeraze's task —— View job


PR Review: Interactive Map Editor for MQTT Bridge BBox

Review Progress

  • Analyze BBoxMapEditor.tsx component
  • Review DashboardPage.tsx changes
  • Assess code quality and best practices
  • Check for potential bugs and edge cases
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage
  • Provide final recommendations

Overall Assessment: ✅ High Quality Implementation

This PR introduces a well-architected interactive map editor that significantly improves UX for configuring geographic bounding boxes. The implementation follows React and Leaflet best practices with clean separation of concerns.

Code Quality & Best Practices

✅ Strengths

Component Architecture

  • Clean separation between BBoxMapEditor (main component) and Layer (Leaflet integration)
  • Well-defined TypeScript interfaces with clear data contracts
  • Consistent patterns with existing GeofenceMapEditor for maintainability

React Best Practices

  • Proper use of useCallback, useMemo, and useEffect with correct dependencies
  • useRef appropriately used for Leaflet object lifecycle management
  • Clean state management with clear data flow

Leaflet Integration

  • Proper cleanup in useEffect return functions prevents memory leaks
  • Custom icons use CSS variables for theming consistency
  • Event handlers properly managed with refs to avoid stale closures

Code Organization

  • Comprehensive JSDoc documentation explaining UX flow and design decisions
  • Helper functions like normalize() and bboxFromForm() are well-abstracted
  • Two-way data binding correctly implemented between map and form inputs

🔧 Minor Improvements

TypeScript: Consider stricter type for corners array at lines 131-168:

const corners: ReadonlyArray<{
  readonly pos: L.LatLng; 
  readonly updateFromDrag: (p: L.LatLng) => BBoxValue;
}> = [...]

Performance: The corner drag logic could be optimized to reduce redundant calculations by memoizing the update functions.

Potential Issues & Edge Cases

⚠️ Minor Concerns

1. Zero-Area Rectangle Handling (BBoxMapEditor.tsx:238)

if (a.equals(b)) return; // Rejects double-click, but should also reject tiny rectangles

Consider adding a minimum area threshold to prevent accidentally creating microscopic bounding boxes.

2. Coordinate Validation (DashboardPage.tsx:52)

if (minLat > maxLat || minLng > maxLng) return null;

Missing validation for coordinate bounds (-90 ≤ lat ≤ 90, -180 ≤ lng ≤ 180).

3. Form Integration Edge Case (DashboardPage.tsx:948)
The toFixed(5) precision might truncate user-entered values that had more precision. Consider preserving user input when values are manually typed.

Performance Considerations

✅ Good Performance Characteristics

  • Efficient re-rendering with internalChangeRef to prevent update loops
  • Minimal DOM manipulation with strategic Leaflet object reuse
  • Proper cleanup prevents memory leaks in dynamic components

🔧 Optimization Opportunities

  • Corner markers could be pooled/reused instead of recreated on each bbox change
  • Consider debouncing rapid drag events for better performance during user interaction

Security Assessment

✅ No Security Concerns

  • Input sanitization handled by native number parsing
  • No direct DOM manipulation with user input
  • External tile layer uses standard OSM (same as existing geofence editor)
  • No sensitive data exposure in component props or state

Test Coverage

⚠️ Gap Identified

Missing Test Coverage: No tests exist for the new BBoxMapEditor component. Given the complexity of the Leaflet integration and the critical nature of geographic filtering, tests should be added.

Recommendation: Follow the pattern from GeofenceTriggersSection.test.tsx:29-43 and create BBoxMapEditor.test.tsx with:

vi.mock('./BBoxMapEditor', () => ({
  default: (props: { onChange: (bbox: BBoxValue | null) => void }) => (
    <div>
      <button onClick={() => props.onChange({
        minLat: 25.0, maxLat: 30.0, minLng: -85.0, maxLng: -80.0
      })}>Set BBox</button>
    </div>
  ),
}));

Integration Assessment

✅ Clean Integration

DashboardPage Changes (54 additions, 17 deletions):

  • Seamless integration with existing form state management
  • Correct two-way binding between map and text inputs
  • Proper form validation and error handling maintained
  • No breaking changes to existing MQTT bridge functionality

Final Recommendations

✅ Ready to Merge With Minor Follow-ups

Pre-merge (Address these if possible):

  1. Add coordinate bounds validation (-90/90, -180/180)
  2. Consider minimum bbox area threshold

Post-merge (Can be follow-up PRs):

  1. Add unit tests for BBoxMapEditor component
  2. Consider performance optimizations for frequent drag operations

Summary

This is a well-crafted feature that meaningfully improves the user experience for geographic filtering. The code demonstrates strong React and Leaflet expertise with attention to edge cases and performance. The architecture choice to create a focused component rather than extending the existing geofence editor shows good design judgment.

Approval Status: ✅ Approve - High quality implementation ready for production


When the bridge filter has a geographic bbox configured, gate ALL portnums
(not just POSITION_APP) on the sender's last known bbox membership. A node
is considered "inside" only after we've decoded a position payload for it
that fell within the bbox. Senders we haven't classified — including those
we've never seen a position for — are dropped fail-closed.

Without this, a node 1000km outside the bbox could still pollute the local
broker with TEXT, TELEMETRY, NODEINFO, NEIGHBORINFO, etc., because the bbox
only ever filtered POSITION payloads. Combined with the interactive editor
in 089a734, the bbox now actually keeps out-of-area traffic out.

- MqttPacketFilter learns membership from postFilterPosition(position, fromNum)
  and gates non-position traffic via passesMembership(fromNum)
- Membership cache is per-filter (per-bridge), FIFO-evicted at 10k entries
- Ingestion + bridge handleDownlink both apply the gate before persist/republish
- Drops increment the existing geo drop counter

Tests:
- 11 new MqttPacketFilter unit tests covering fail-closed semantics,
  position-learns-membership, half-open bboxes, no-bbox no-op, and instance
  isolation
- 7 new ingestion tests proving the gate short-circuits TEXT/NODEINFO/
  TELEMETRY before any database call

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Claude finished @Yeraze's task —— View job


PR Code Review: Interactive Map Editor for MQTT Bridge BBox

Review Progress

  • Analyze BBoxMapEditor.tsx component
  • Review DashboardPage.tsx changes
  • Assess MQTT filtering logic updates
  • Check test coverage and quality
  • Evaluate performance considerations
  • Review security concerns
  • Provide final recommendations

Overall Assessment: ✅ High Quality Implementation

This PR introduces a well-architected interactive map editor that significantly improves UX for configuring geographic bounding boxes. The implementation follows React and Leaflet best practices with clean separation of concerns.

Code Quality & Best Practices

✅ Strengths

Component Architecture

  • Clean separation between BBoxMapEditor (main component) and Layer (Leaflet integration)
  • Well-defined TypeScript interfaces with clear data contracts
  • Consistent patterns with existing GeofenceMapEditor for maintainability

React Best Practices

  • Proper use of useCallback, useMemo, and useEffect with correct dependencies
  • useRef appropriately used for Leaflet object lifecycle management
  • Clean state management with clear data flow

Leaflet Integration

  • Proper cleanup in useEffect return functions prevents memory leaks
  • Custom icons use CSS variables for theming consistency
  • Event handlers properly managed with refs to avoid stale closures

Code Organization

  • Comprehensive JSDoc documentation explaining UX flow and design decisions
  • Helper functions like normalize() and bboxFromForm() are well-abstracted
  • Two-way data binding correctly implemented between map and form inputs

🔧 Minor Improvements

TypeScript: Consider stricter type for corners array at lines 131-168:

const corners: ReadonlyArray<{
  readonly pos: L.LatLng; 
  readonly updateFromDrag: (p: L.LatLng) => BBoxValue;
}> = [...]

Performance: The corner drag logic could be optimized to reduce redundant calculations by memoizing the update functions.

Potential Issues & Edge Cases

⚠️ Minor Concerns

1. Zero-Area Rectangle Handling (BBoxMapEditor.tsx:238)

if (a.equals(b)) return; // Rejects double-click, but should also reject tiny rectangles

Consider adding a minimum area threshold to prevent accidentally creating microscopic bounding boxes.

2. Coordinate Validation (DashboardPage.tsx:52)

if (minLat > maxLat || minLng > maxLng) return null;

Missing validation for coordinate bounds (-90 ≤ lat ≤ 90, -180 ≤ lng ≤ 180).

3. Form Integration Edge Case (DashboardPage.tsx:948)
The toFixed(5) precision might truncate user-entered values that had more precision. Consider preserving user input when values are manually typed.

Performance Considerations

✅ Good Performance Characteristics

  • Efficient re-rendering with internalChangeRef to prevent update loops
  • Minimal DOM manipulation with strategic Leaflet object reuse
  • Proper cleanup prevents memory leaks in dynamic components

🔧 Optimization Opportunities

  • Corner markers could be pooled/reused instead of recreated on each bbox change
  • Consider debouncing rapid drag events for better performance during user interaction

Security Assessment

✅ No Security Concerns

  • Input sanitization handled by native number parsing
  • No direct DOM manipulation with user input
  • External tile layer uses standard OSM (same as existing geofence editor)
  • No sensitive data exposure in component props or state

Test Coverage

⚠️ Gap Identified

Missing Test Coverage: No tests exist for the new BBoxMapEditor component. Given the complexity of the Leaflet integration and the critical nature of geographic filtering, tests should be added.

Recommendation: Follow the pattern from GeofenceTriggersSection.test.tsx:29-43 and create BBoxMapEditor.test.tsx with:

vi.mock('./BBoxMapEditor', () => ({
  default: (props: { onChange: (bbox: BBoxValue | null) => void }) => (
    <div>
      <button onClick={() => props.onChange({
        minLat: 25.0, maxLat: 30.0, minLng: -85.0, maxLng: -80.0
      })}>Set BBox</button>
    </div>
  ),
}));

Strong MQTT Filter Testing: The mqttPacketFilter.test.ts and mqttIngestion.test.ts files provide excellent coverage of the filtering logic with 338 test lines covering edge cases, fail-closed semantics, and membership caching.

Integration Assessment

✅ Clean Integration

DashboardPage Changes (54 additions, 17 deletions):

  • Seamless integration with existing form state management
  • Correct two-way binding between map and text inputs
  • Proper form validation and error handling maintained
  • No breaking changes to existing MQTT bridge functionality

Final Recommendations

✅ Ready to Merge With Minor Follow-ups

Pre-merge (Address these if possible):

  1. Add coordinate bounds validation (-90/90, -180/180)
  2. Consider minimum bbox area threshold

Post-merge (Can be follow-up PRs):

  1. Add unit tests for BBoxMapEditor component
  2. Consider performance optimizations for frequent drag operations

Summary

This is a well-crafted feature that meaningfully improves the user experience for geographic filtering. The code demonstrates strong React and Leaflet expertise with attention to edge cases and performance. The architecture choice to create a focused component rather than extending the existing geofence editor shows good design judgment.

Approval Status: ✅ Approve - High quality implementation ready for production


@github-actions
Copy link
Copy Markdown

System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-17 22:31:04 EDT

❌ Overall Result: FAILED

Aborted on first failing test (fail-fast mode). Remaining tests skipped.

Failed Test

  • Reverse Proxy Test

@github-actions
Copy link
Copy Markdown

System Test Results

MeshMonitor System Test Results

Test Run: 2026-05-17 22:44:13 EDT

Test Summary

Test Suite Result
Configuration Import ✅ PASSED
Quick Start Test ✅ PASSED
Security Test ✅ PASSED
V1 API Test ✅ PASSED
Reverse Proxy Test ✅ PASSED
Reverse Proxy + OIDC ✅ PASSED
Virtual Node CLI Test ✅ PASSED
Backup & Restore Test ✅ PASSED
Database Migration Test ✅ PASSED
DB Backing Consistency ✅ PASSED
API Exercise (3 DBs) ✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

  • Tests configuration import and device reboot cycle
  • Verifies channel roles, PSKs, and LoRa configuration
  • Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

  • Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
  • HTTP access without HSTS
  • Auto-generated admin user with default credentials
  • Session cookies work over HTTP
  • Meshtastic node connection and message exchange verified

Security Test:

  • Verifies Node IP address hidden from anonymous users in API responses
  • Verifies MQTT configuration hidden from anonymous users
  • Verifies Node IP address visible to authenticated users
  • Verifies MQTT configuration visible to authenticated users
  • Verifies protected endpoints require authentication

V1 API Test:

  • Tests v1 REST API endpoints with Bearer token authentication
  • Verifies Bearer token requests bypass CSRF protection
  • Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
  • Verifies session-based requests still require CSRF token

Reverse Proxy Test:

  • Production deployment with COOKIE_SECURE=true
  • HTTPS-ready configuration
  • Trust proxy enabled for reverse proxy compatibility
  • CORS configured for HTTPS domain
  • Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

  • OIDC authentication integration
  • Mock OIDC provider health checks
  • Authorization flow and session creation
  • Hybrid mode (OIDC + local auth)
  • Meshtastic node connection verified

Virtual Node CLI Test:

  • Virtual Node Server enabled on TCP port 4404
  • Meshtastic Python client successfully connects
  • Node data download and synchronization verified
  • Test message sent on gauntlet channel (index 3)
  • Message delivery confirmed via Web UI API
  • Virtual Node Server connection logging verified

Backup & Restore Test:

  • System backup created from running dev container
  • New container spun up with RESTORE_FROM_BACKUP env var
  • Data integrity verified (node count, message count, settings)
  • Restore event logged in audit log
  • Dev container unaffected by restore test

Database Migration Test:

  • SQLite to PostgreSQL migration verified
  • SQLite to MySQL migration verified
  • Data integrity confirmed for both target databases
  • Row counts match between source and target

DB Backing Consistency Test:

  • SQLite, PostgreSQL, and MySQL backends tested with same device
  • Node counts within ±10 across all three backends
  • Favorite counts identical across all backends
  • Key station verified as favorite on all backends

@Yeraze Yeraze merged commit f835219 into main May 18, 2026
20 of 22 checks passed
@Yeraze Yeraze deleted the feat/mqtt-bridge-bbox-map-editor branch May 18, 2026 02:45
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.

1 participant