Open
Conversation
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Reviewer's GuideImplements a new Leaflet-based MiniMap widget that displays the vehicle’s live GPS position and trail using MAVLink GLOBAL_POSITION_INT data, wires it into the main view layout, configures proxying for cached map tiles, and adds the necessary Leaflet dependencies. Sequence diagram for MiniMap live GPS position updates via MAVLink2RestsequenceDiagram
participant Autopilot
participant MAVLink2Rest
participant MiniMap as MiniMap_component
participant AutopilotStore as autopilot_data
participant AutopilotManager as autopilot_manager
participant LeafletMap as Leaflet_map
Autopilot->>MAVLink2Rest: Send GLOBAL_POSITION_INT
MiniMap->>MAVLink2Rest: startListening(GLOBAL_POSITION_INT)
MAVLink2Rest-->>MiniMap: listener
MiniMap->>MAVLink2Rest: listener.setCallback(message)
MAVLink2Rest-->>MiniMap: Invoke_callback_on_each_message
loop For_each_GLOBAL_POSITION_INT_message
MAVLink2Rest->>MiniMap: message
MiniMap->>AutopilotStore: Read system_id
MiniMap->>MiniMap: Check system_id_and_component_id
alt Not_target_system_or_component
MiniMap-->>MAVLink2Rest: Ignore_message
else Matching_vehicle
MiniMap->>MiniMap: Decode lat, lon, hdg
MiniMap->>MiniMap: Update latitude, longitude, heading
MiniMap->>MiniMap: has_position = true
MiniMap->>LeafletMap: updateVehicle()
LeafletMap->>LeafletMap: set marker position
LeafletMap->>LeafletMap: rotate marker icon
LeafletMap->>LeafletMap: append_to_trail_and_update_polyline
end
end
MiniMap->>MAVLink2Rest: listener.discard() on_beforeDestroy
MiniMap->>LeafletMap: map.remove() on_beforeDestroy
Class diagram for the new MiniMap Vue componentclassDiagram
class MiniMap {
+string name
+L_Map map
+L_Marker vehicle_marker
+L_Polyline trail
+L_LatLng[] trail_points
+number latitude
+number longitude
+number heading
+boolean has_position
+number max_trail_points
+ListenerHandle listener
+mounted()
+beforeDestroy()
+initMap()
+startListening()
+updateVehicle()
}
class mavlink2rest {
+startListening(message_name) ListenerHandle
}
class ListenerHandle {
+setCallback(message)
+setFrequency(hz)
+discard()
}
class autopilot_data {
+number system_id
}
class autopilot_manager {
+string vehicle_type
}
class Leaflet_Map {
+setView(center, zoom)
+invalidateSize()
+panTo(center)
}
class Leaflet_Marker {
+setLatLng(position)
+getElement() HTMLElement
}
class Leaflet_Polyline {
+setLatLngs(points)
}
class GlobalPositionInt {
+number lat
+number lon
+number hdg
}
MiniMap --> mavlink2rest : uses
MiniMap --> ListenerHandle : holds
MiniMap --> autopilot_data : reads
MiniMap --> autopilot_manager : reads
MiniMap --> Leaflet_Map : wraps
MiniMap --> Leaflet_Marker : wraps
MiniMap --> Leaflet_Polyline : wraps
MiniMap --> GlobalPositionInt : decodes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
patrickelectric
requested review from
Williangalvani,
joaoantoniocardoso and
joaomariolago
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Panning the map on every position update with
this.map.panTo(pos)can cause jittery UX at higher update rates; consider only recentering when the vehicle nears the edge of the viewport or behind a user-controlled “follow” toggle. - Importing
leaflet/dist/leaflet.cssinside theMiniMapcomponent will pull the stylesheet every time the component is used; it may be cleaner to move this CSS import to a global entry point (e.g., main.ts) so it’s loaded once. - Using
latitude === 0 && longitude === 0to detect lack of GPS can incorrectly hide valid positions at (0,0); if possible, key thehas_positionlogic off an explicit validity/fix indicator from MAVLink instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Panning the map on every position update with `this.map.panTo(pos)` can cause jittery UX at higher update rates; consider only recentering when the vehicle nears the edge of the viewport or behind a user-controlled “follow” toggle.
- Importing `leaflet/dist/leaflet.css` inside the `MiniMap` component will pull the stylesheet every time the component is used; it may be cleaner to move this CSS import to a global entry point (e.g., main.ts) so it’s loaded once.
- Using `latitude === 0 && longitude === 0` to detect lack of GPS can incorrectly hide valid positions at (0,0); if possible, key the `has_position` logic off an explicit validity/fix indicator from MAVLink instead.
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/map/MiniMap.vue:131-134` </location>
<code_context>
+ }).addTo(this.map)
+ },
+ startListening() {
+ this.listener = mavlink2rest.startListening('GLOBAL_POSITION_INT')
+ this.listener.setCallback((message) => {
+ if (message?.header.system_id !== autopilot_data.system_id || message?.header.component_id !== 1) {
+ return
+ }
+
+ const pos = message?.message as GlobalPositionInt
+ this.latitude = pos.lat / 1e7
+ this.longitude = pos.lon / 1e7
+ this.heading = pos.hdg / 100
+
+ if (this.latitude === 0 && this.longitude === 0) return
+
+ const was_positioned = this.has_position
+ this.has_position = true
+
+ this.updateVehicle()
+
+ if (!was_positioned) {
+ this.$nextTick(() => {
+ this.map?.invalidateSize()
+ this.map?.setView([this.latitude, this.longitude], 18)
+ })
+ }
+ }).setFrequency(1)
+ },
+ updateVehicle() {
</code_context>
<issue_to_address>
**suggestion:** Heading value is used without guarding against the MAVLink "unknown" sentinel value.
For `GLOBAL_POSITION_INT`, `hdg` is a `uint16` in cdeg where `65535` indicates "unknown". Here we always use `heading` to rotate the icon, so an unknown heading still results in a rotation (655.35°). If you want to respect the sentinel, consider checking for `65535` and either skipping rotation or retaining the previous heading in that case.
```suggestion
const pos = message?.message as GlobalPositionInt
this.latitude = pos.lat / 1e7
this.longitude = pos.lon / 1e7
// hdg is uint16 in cdeg; 65535 indicates "unknown" and should not be used
if (pos.hdg !== 65535) {
this.heading = pos.hdg / 100
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
patrickelectric
force-pushed
the
map-visualizer
branch
from
ee77795 to
e87334a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #3738
Summary by Sourcery
Add a vehicle position minimap widget using Leaflet and integrate it into the main view layout.
New Features:
Enhancements: