frontend: detect and list other vehicles on blueos #3517
+401
−11
Conversation
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
Reviewer's GuideThis PR adds automatic discovery of peer vehicles via mDNS in the backend and a new VehiclePicker widget in the frontend, integrating it into the VehicleBanner with lifecycle-managed Zeroconf listeners and a Sequence diagram for vehicle discovery and listing in frontendsequenceDiagram
actor User
participant VehiclePicker
participant Frontend
participant Backend
participant ZeroconfListener
participant OtherVehicle
User->>VehiclePicker: Clicks VehiclePicker button
VehiclePicker->>Frontend: Request /beacon/v1.0/discovered_services
Frontend->>Backend: GET /discovered_services
Backend->>ZeroconfListener: get_discovered_services()
ZeroconfListener->>Backend: Return discovered vehicles
Backend->>Frontend: Return discovered vehicles
Frontend->>VehiclePicker: Display list of vehicles
User->>VehiclePicker: Clicks on vehicle IP
VehiclePicker->>OtherVehicle: Open vehicle UI in new tab
Class diagram for BeaconServiceListener and Beacon service changesclassDiagram
class BeaconServiceListener {
+discovered_services: Dict[str, str]
+add_service(zc, type_, name)
+remove_service(zc, type_, name)
+update_service(zc, type_, name)
+_process_service(zc, type_, name)
+get_discovered_services() Dict[str, str]
}
class Beacon {
+zeroconf: Optional[Zeroconf]
+service_listener: Optional[BeaconServiceListener]
+service_browser: Optional[ServiceBrowser]
+start_zeroconf_listener()
+stop_zeroconf_listener()
+get_discovered_services() Dict[str, str]
}
Beacon "1" -- "1" BeaconServiceListener : manages
Beacon "1" -- "1" ServiceBrowser : manages
BeaconServiceListener <|-- ServiceListener
Class diagram for VehiclePicker Vue componentclassDiagram
class VehiclePicker {
+expanded: boolean
+loading: boolean
+error: string | null
+availableVehicles: Vehicle[]
+toggleDropdown()
+fetchDiscoveredServices()
+refreshServices()
+navigateToVehicle(ip: string)
+allMyIps: string[]
}
class Vehicle {
+ips: string[]
+hostname: string
+imagePath: string
}
VehiclePicker "*" -- "*" Vehicle : displays
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In fetchDiscoveredServices, consider replacing the manual completedRequests counter with Promise.all (or Promise.allSettled) over your per-IP requests so you can more cleanly handle the loading state once all calls finish.
- BeaconServiceListener currently only pulls the first IPv4 address—think about extending it to support IPv6 addresses (and possibly multiple addresses per service) for more complete network discovery.
- Right now discovered services only disappear when an explicit remove event fires; you might want to add a TTL or timestamp-based cleanup so stale entries don’t linger indefinitely if a service goes silent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In fetchDiscoveredServices, consider replacing the manual completedRequests counter with Promise.all (or Promise.allSettled) over your per-IP requests so you can more cleanly handle the loading state once all calls finish.
- BeaconServiceListener currently only pulls the first IPv4 address—think about extending it to support IPv6 addresses (and possibly multiple addresses per service) for more complete network discovery.
- Right now discovered services only disappear when an explicit remove event fires; you might want to add a TTL or timestamp-based cleanup so stale entries don’t linger indefinitely if a service goes silent.
## Individual Comments
### Comment 1
<location> `core/services/beacon/main.py:50-56` </location>
<code_context>
+ logger.info(f"Service {name} added")
+ self._process_service(zc, type_, name)
+
+ def _process_service(self, zc: Zeroconf, type_: str, name: str) -> None:
+ """Process a service and extract its IP address"""
+ try:
+ info = zc.get_service_info(type_, name)
+ if info and info.addresses:
+ # Convert the first IP address from bytes to string
+ ip_address = socket.inet_ntoa(info.addresses[0])
+ self.discovered_services[name] = ip_address
+ logger.info(f"Service {name} at {ip_address}")
</code_context>
<issue_to_address>
**issue:** IPv6 addresses are not handled in service discovery.
Currently, only IPv4 addresses are processed via socket.inet_ntoa, so services with IPv6 addresses are ignored. Please add IPv6 support for full compatibility.
</issue_to_address>
### Comment 2
<location> `core/frontend/src/components/app/VehiclePicker.vue:168-177` </location>
<code_context>
+ ips.forEach(async (ip) => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Mixing async/await with forEach may cause race conditions.
forEach does not handle async functions as expected, which can result in unpredictable increments of completedRequests. Use Promise.all or a for...of loop with await to ensure proper sequencing and concurrency control.
</issue_to_address>
### Comment 3
<location> `core/frontend/src/components/app/VehiclePicker.vue:195` </location>
<code_context>
+ )
+
+ if (existingVehicleIndex >= 0) {
+ this.availableVehicles[existingVehicleIndex].ips.push(...newVehicle.ips)
+ if (newVehicle.imagePath && !this.availableVehicles[existingVehicleIndex].imagePath) {
+ this.availableVehicles[existingVehicleIndex].imagePath = newVehicle.imagePath
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Duplicate IPs may be added to availableVehicles.
Currently, IPs are merged without checking for duplicates. Use a Set or filter out existing IPs before adding to prevent redundant entries.
```suggestion
const existingIps = new Set(this.availableVehicles[existingVehicleIndex].ips)
const uniqueNewIps = newVehicle.ips.filter(ip => !existingIps.has(ip))
this.availableVehicles[existingVehicleIndex].ips.push(...uniqueNewIps)
```
</issue_to_address>
### Comment 4
<location> `core/services/beacon/main.py:293-303` </location>
<code_context>
def stop_zeroconf_listener(self) -> None:
"""
Stop the zeroconf listener and clean up resources
"""
if self.zeroconf is not None:
try:
if self.service_browser is not None:
self.service_browser.cancel()
self.service_browser = None
self.zeroconf.close()
self.zeroconf = None
self.service_listener = None
logger.info("Zeroconf listener stopped")
except Exception as e:
logger.warning(f"Error stopping zeroconf listener: {e}")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Add guard clause ([`last-if-guard`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/last-if-guard/))
```suggestion
if self.zeroconf is None:
return
try:
if self.service_browser is not None:
self.service_browser.cancel()
self.service_browser = None
self.zeroconf.close()
self.zeroconf = None
self.service_listener = None
logger.info("Zeroconf listener stopped")
except Exception as e:
logger.warning(f"Error stopping zeroconf listener: {e}")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+50
to
+56
| def _process_service(self, zc: Zeroconf, type_: str, name: str) -> None: | ||
| """Process a service and extract its IP address""" | ||
| try: | ||
| info = zc.get_service_info(type_, name) | ||
| if info and info.addresses: | ||
| # Convert the first IP address from bytes to string | ||
| ip_address = socket.inet_ntoa(info.addresses[0]) |
There was a problem hiding this comment.
issue: IPv6 addresses are not handled in service discovery.
Currently, only IPv4 addresses are processed via socket.inet_ntoa, so services with IPv6 addresses are ignored. Please add IPv6 support for full compatibility.
Comment on lines
+293
to
+303
| if self.zeroconf is not None: | ||
| try: | ||
| if self.service_browser is not None: | ||
| self.service_browser.cancel() | ||
| self.service_browser = None | ||
| self.zeroconf.close() | ||
| self.zeroconf = None | ||
| self.service_listener = None | ||
| logger.info("Zeroconf listener stopped") | ||
| except Exception as e: | ||
| logger.warning(f"Error stopping zeroconf listener: {e}") |
There was a problem hiding this comment.
suggestion (code-quality): Add guard clause (last-if-guard)
Suggested change
| if self.zeroconf is not None: | |
| try: | |
| if self.service_browser is not None: | |
| self.service_browser.cancel() | |
| self.service_browser = None | |
| self.zeroconf.close() | |
| self.zeroconf = None | |
| self.service_listener = None | |
| logger.info("Zeroconf listener stopped") | |
| except Exception as e: | |
| logger.warning(f"Error stopping zeroconf listener: {e}") | |
| if self.zeroconf is None: | |
| return | |
| try: | |
| if self.service_browser is not None: | |
| self.service_browser.cancel() | |
| self.service_browser = None | |
| self.zeroconf.close() | |
| self.zeroconf = None | |
| self.service_listener = None | |
| logger.info("Zeroconf listener stopped") | |
| except Exception as e: | |
| logger.warning(f"Error stopping zeroconf listener: {e}") |
patrickelectric
requested changes
Oct 24, 2025
Member
patrickelectric
left a comment
patrickelectric
left a comment
There was a problem hiding this comment.
It's not clear how this discovery BlueOS on the network since we do not have an global identifier for BlueOS in mdns, besides the mavlink one. IIRC.
| this.availableVehicles = [] | ||
| try { | ||
| const response = await axios.get('/beacon/v1.0/discovered_services', { |
Member
There was a problem hiding this comment.
Why is this here and not in the beacon store ?
Williangalvani
force-pushed
the
vehicle-picker
branch
from
a3d254f to
aa0918f
Compare
Member
@Williangalvani can you answer this comment ? |
Member
Author
I'm not done with this PR yet, I probably pushed it to build on CI (due to 429 isssues) and switched tasks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Helps manage multiple vehicles on the same network. requires that the vehicles are able to see each other
Screen.Recording.2025-08-30.at.18.28.53.mov
Summary by Sourcery
Enable automatic discovery of other vehicles on the local network via mDNS in the backend and add a front-end widget for listing and navigating to those vehicles
New Features:
Enhancements:
Summary by Sourcery
Enable automatic discovery of other vehicles on the network and provide a front-end widget for listing and navigating to those vehicles.
New Features:
Enhancements: