Skip to content

feat(drone): modernize drone for CLI + Rerun + replay#1520

Merged
spomichter merged 19 commits intodevfrom
feat/drone-modernize
Mar 11, 2026
Merged

feat(drone): modernize drone for CLI + Rerun + replay#1520
spomichter merged 19 commits intodevfrom
feat/drone-modernize

Conversation

@spomichter
Copy link
Contributor

Summary

This PR modernizes the drone codebase to align with current DimOS patterns (Go2, G1) and enables CLI-based operation.

Changes

Phase 1: CLI Registration + Rerun Integration

  • ✅ Created blueprint directory structure (dimos/robot/drone/blueprints/)
  • ✅ Added drone_basic blueprint (DroneConnectionModule + DroneCameraModule + RerunBridge)
  • ✅ Added drone_agentic blueprint (full stack with tracking + agent + web)
  • ✅ Registered both blueprints in all_blueprints.py
  • ✅ Replaced FoxgloveBridge with RerunBridgeModule
  • ✅ Added replay support via global_config.replay flag

Phase 2: Module Cleanup

  • ✅ Removed main() function from drone.py (CLI now handles this)
  • ✅ Removed __main__ block
  • ✅ Added deprecation warning to old Drone(Robot) class
  • ✅ Existing stream names preserved for backward compatibility

Phase 3: Replay Integration

  • ✅ Wired replay through --replay flag
  • ✅ Existing FakeMavlinkConnection and FakeDJIVideoStream work with connection_string="replay"
  • ✅ Data at data/drone/ ready for replay (6,246 frames)

Usage

# Basic drone with visualization
dimos run drone-basic

# Replay mode
dimos run drone-basic --replay

# Full agentic drone with web UI + AI agent
dimos run drone-agentic

# Agentic replay
dimos run drone-agentic --replay

Testing Notes

  • Unit tests require pymavlink dependency (not in core requirements)
  • Blueprint registration verified: both drone-basic and drone-agentic discoverable
  • Replay data verified: data/drone/mavlink/ (4,098 frames) and data/drone/video/ (2,148 frames)
  • Pre-commit hooks passing

Migration Path

Existing code using Drone() class will continue to work but should migrate to:

  • Simple use cases → drone_basic blueprint
  • Autonomous missions → drone_agentic blueprint

Related

  • Assessment: engineering/drone-assessment.md
  • Reference implementation: Go2 blueprints (dimos/robot/unitree/go2/blueprints/)

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR modernizes the drone subsystem to align with the DimOS blueprint pattern (as used in Go2/G1), enabling CLI-based operation via dimos run drone-basic and dimos run drone-agentic, replacing the old main() entry point. The two new blueprints are cleanly registered in all_blueprints.py and correctly use lazy loading to defer global_config evaluation until the blueprint is actually invoked.

Key issues found:

  • Name conflict in drone.py: The old drone_agentic() factory function (lines 394–424) was not removed. There are now two things named drone_agentic — one a callable function in drone.py, one a pre-built Blueprint object in blueprints/agentic/drone_agentic.py. Code that imports directly from drone.py will silently get the old function, not the new blueprint.
  • outdoor parameter silently dropped: The new drone_agentic blueprint hardcodes outdoor=False, removing a previously configurable parameter. Users running outdoor GPS-only missions will get incorrect behavior with no error or warning.
  • Loss of DRONE_CONNECTION / DRONE_VIDEO_PORT env var support: The deleted main() function honored these environment variables; both new blueprints hardcode the defaults with only --replay carried over. Existing deployments that set these env vars will break silently.
  • Hardcoded office GPS coordinates: DRONE_SYSTEM_PROMPT in drone_agentic.py embeds specific coordinates for Dimensional Inc.'s SF office, duplicating the same constant already in drone.py. These should live in config rather than source code.

Confidence Score: 3/5

  • Safe to merge for non-outdoor drone use, but the stale drone_agentic function in drone.py and the dropped outdoor flag are regressions that should be resolved before merging.
  • The blueprint registration, lazy loading, and replay wiring are correct and consistent with the rest of the codebase. However, the leftover drone_agentic factory function in drone.py creates a genuine naming conflict that will confuse callers, the outdoor mode regression silently breaks a valid use case, and the removed env-var configuration is a behavioral change that isn't documented as intentional.
  • dimos/robot/drone/drone.py (stale drone_agentic function) and dimos/robot/drone/blueprints/agentic/drone_agentic.py (hardcoded outdoor=False, hardcoded GPS coordinates).

Important Files Changed

Filename Overview
dimos/robot/drone/drone.py Deprecation notice added to Drone class and main() removed, but the old drone_agentic factory function (lines 394–424) was not removed, creating a name conflict with the new Blueprint object in blueprints/agentic/.
dimos/robot/drone/blueprints/agentic/drone_agentic.py New agentic blueprint wires up the full drone stack correctly, but silently drops the outdoor parameter (hardcoded to False) and embeds hardcoded office GPS coordinates in the system prompt that are location-specific and duplicated from drone.py.
dimos/robot/drone/blueprints/basic/drone_basic.py Clean basic blueprint for connection + camera + visualization. Connection string and video port are hardcoded, dropping the DRONE_CONNECTION/DRONE_VIDEO_PORT env var support from the deleted main(). rerun_config and viewer-selection logic are duplicated from the agentic blueprint.
dimos/robot/all_blueprints.py Both drone-basic and drone-agentic correctly registered in alphabetical order with string-based lazy references.
dimos/robot/drone/blueprints/init.py Properly uses lazy_loader to defer module-level execution until the blueprint is accessed, consistent with the pattern used elsewhere.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["dimos run drone-basic / drone-agentic"]
    CLI -->|"--replay"| GC["global_config.replay = True"]
    CLI --> AB["all_blueprints.py\nlookup by name"]

    AB -->|"drone-basic"| DB["drone_basic.py\n(module-level eval)"]
    AB -->|"drone-agentic"| DA["drone_agentic.py\n(module-level eval)"]

    GC -.->|"read at import time"| DB
    GC -.->|"read at import time"| DA

    DB --> VIS_B{"global_config.viewer?"}
    DA --> VIS_A{"global_config.viewer?"}

    VIS_B -->|"foxglove"| FB_B["foxglove_bridge()"]
    VIS_B -->|"rerun*"| RR_B["rerun_bridge()"]
    VIS_B -->|"else"| NONE_B["no bridge"]

    VIS_A -->|"foxglove"| FB_A["foxglove_bridge()"]
    VIS_A -->|"rerun*"| RR_A["rerun_bridge()"]
    VIS_A -->|"else"| NONE_A["no bridge"]

    DB -->|"replay?"| CS_B{"connection_string"}
    CS_B -->|"yes"| R1["replay"]
    CS_B -->|"no"| U1["udp:0.0.0.0:14550"]

    DA -->|"replay?"| CS_A{"connection_string"}
    CS_A -->|"yes"| R2["replay"]
    CS_A -->|"no"| U2["udp:0.0.0.0:14550"]

    DB --> BASIC["autoconnect:\nDroneConnectionModule\nDroneCameraModule\nwebsocket_vis"]
    DA --> AGENTIC["autoconnect:\nDroneConnectionModule\nDroneCameraModule\nDroneTrackingModule\nwebsocket_vis\nGoogleMapsSkill\nOsmSkill\nagent(gpt-4o)\nweb_input"]

    BASIC --> BRUN["Blueprint.build().loop()"]
    AGENTIC --> ARUN["Blueprint.build().loop()"]
Loading

Comments Outside Diff (1)

  1. dimos/robot/drone/drone.py, line 394-424 (link)

    Stale drone_agentic function creates a name conflict

    The old drone_agentic() factory function (lines 394–424) was never removed from drone.py. The PR now also introduces a drone_agentic Blueprint object in dimos/robot/drone/blueprints/agentic/drone_agentic.py.

    This means there are two things with the same name exported from the dimos.robot.drone package:

    • dimos.robot.drone.drone.drone_agentic → a callable factory function returning a Blueprint
    • dimos.robot.drone.blueprints.agentic.drone_agentic.drone_agentic → a pre-built Blueprint object

    Any code that does from dimos.robot.drone.drone import drone_agentic will get the old factory function, not the new blueprint. This is especially confusing since both share the exact same identifier. The old function should either be removed or explicitly deprecated/re-exported to alias the new blueprint.

Last reviewed commit: 02aaf30

Comment on lines +84 to +89
connection_string = "udp:0.0.0.0:14550"
video_port = 5600
outdoor = False

if global_config.replay:
connection_string = "replay"
Copy link
Contributor

Choose a reason for hiding this comment

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

outdoor mode silently dropped — regression from old API

The previous drone_agentic() function in drone.py accepted outdoor as a configurable parameter and threaded it through to both DroneConnectionModule and DroneTrackingModule. The new blueprint hardcodes outdoor = False with no way to override it via CLI or config.

Users running outdoor GPS-only missions (the documented use case) will silently get incorrect indoor/velocity-integration behavior. At minimum, this should read from global_config (if that field exists) or be documented as a known limitation.

# Determine connection string and outdoor mode based on config
connection_string = "udp:0.0.0.0:14550"
video_port = 5600
outdoor = False  # ← no outdoor config hookup; was configurable in the old API

Comment on lines +36 to +40
Here are some GPS locations to remember
6th and Natoma intersection: 37.78019978319006, -122.40770815020853,
454 Natoma (Office): 37.780967465525244, -122.40688342010769
5th and mission intersection: 37.782598539339695, -122.40649441875473
6th and mission intersection: 37.781007204789354, -122.40868447123661"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Hardcoded office GPS coordinates in system prompt

The DRONE_SYSTEM_PROMPT embeds specific GPS coordinates for the Dimensional Inc. SF office (454 Natoma, 6th & Natoma, etc.). These are:

  1. Location-specific and will be confusing or misleading for anyone running the blueprint elsewhere.
  2. Potentially sensitive if the repo is public.

These deployment-specific waypoints should live in a config file, environment variable, or separate "locations" config rather than being baked into the source code. The same prompt is also duplicated verbatim in drone.py (lines 382–391), meaning any update must be applied in two places.

Comment on lines +67 to +71
# Determine connection string based on replay flag
connection_string = "udp:0.0.0.0:14550"
video_port = 5600
if global_config.replay:
connection_string = "replay"
Copy link
Contributor

Choose a reason for hiding this comment

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

DRONE_CONNECTION / DRONE_VIDEO_PORT env vars no longer honored

The deleted main() function in drone.py read DRONE_CONNECTION and DRONE_VIDEO_PORT from environment variables, allowing users to configure the drone endpoint without modifying code:

connection = os.getenv("DRONE_CONNECTION", "udp:0.0.0.0:14550")
video_port = int(os.getenv("DRONE_VIDEO_PORT", "5600"))

Both drone_basic.py and drone_agentic.py now hardcode these values. Any user or deployment that previously relied on those environment variables will silently get the default values instead of their configured ones, with no warning. The same applies to video_port — only --replay is carried forward via global_config. Consider reading these from global_config or falling back to environment variables to preserve the previous behavior.

Phase 1: CLI Registration + Rerun Integration

- Create blueprint directory structure (dimos/robot/drone/blueprints/)
- Add drone_basic blueprint (DroneConnectionModule + DroneCameraModule + RerunBridge)
- Add drone_agentic blueprint (full stack with tracking + agent + web)
- Register both blueprints in all_blueprints.py
- Replace FoxgloveBridge with RerunBridgeModule
- Add replay support via global_config.replay flag
- Maintain existing stream names for backward compatibility

Blueprints now support:
- dimos run drone-basic [--replay]
- dimos run drone-agentic [--replay]
…ated

Phase 2: Module Cleanup

- Remove main() function from drone.py (CLI now handles this)
- Remove __main__ block
- Add deprecation warning to old Drone(Robot) class
- Recommend using drone_basic or drone_agentic blueprints instead

The old class-based Robot pattern is being phased out in favor of
blueprint composition. Existing code using Drone() will still work
but should migrate to the new blueprints.
- Remove deprecated Drone(Robot) class entirely from drone.py
- Delete drone.py file (functionality moved to blueprints)
- Update __init__.py to remove Drone export
- Add ClockSyncConfigurator to both blueprints (fixes LCM autoconf errors)
- Update test_drone.py to skip TestDroneFullIntegration (tested deprecated class)
- All remaining tests pass (22 passed, 1 skipped)

The Drone class-based pattern is fully deprecated. Use drone_basic or
drone_agentic blueprints instead.
- Changed LCM(autoconf=True) to LCM() in both blueprints
- Matches Go2 blueprint pattern
- Fixes TypeError when loading blueprints
- Verified: dimos --replay run drone-basic works (modules deploy successfully)
…() signature

- Replace conditional viewer logic with direct FoxgloveBridge.blueprint()
- Use WebsocketVisModule.blueprint() instead of websocket_vis() alias
- Preserve exact module composition: DroneConnectionModule, DroneCameraModule,
  DroneTrackingModule, WebsocketVisModule, FoxgloveBridge, GoogleMapsSkillContainer,
  OsmSkill, agent, web_input
- Preserve exact remappings: (DroneTrackingModule, video_input → video),
  (DroneTrackingModule, cmd_vel → movecmd_twist)
- Keep function-based _make_drone_agentic() with all original default params
- Module-level drone_agentic instance for CLI registry compatibility
- Export DRONE_SYSTEM_PROMPT in __all__
Opens camera feed automatically on startup instead of requiring
manual panel navigation. Horizontal split: Camera (1/3) + 3D world (2/3).
DroneCameraModule subscribes on 'video' stream, not 'color_image'.
The Rerun entity path is world/video.
GoogleMapsSkillContainer now logs a warning instead of crashing
when GOOGLE_MAPS_API_KEY is not set. The module deploys but
returns 'not configured' for skill calls.
Some Rerun versions don't render BGR color_model correctly,
causing blue-tinted video. Convert BGR→RGB and BGRA→RGBA in
_format_to_rerun() with numpy channel swap.
- FakeDJIVideoStream: re-tag replay frames from BGR to RGB (GStreamer
  outputs RGB but Aug 2025 recording used default BGR label)
- OsmSkill: guard .subscribe() with hasattr check for RemoteIn
  compatibility when distributed across workers
…ompat

Replace Vector3 parameter with x/y/z floats so Agent.on_system_modules()
can generate JSON schema for the skill. Vector3 is an IsInstanceSchema
which Pydantic cannot serialize.
…blueprint

- Remove wrapper function, flatten to module-level autoconnect
- Remove .global_config(n_workers=4) that caused OsmSkill RemoteIn crash
- Remove .configurators(ClockSyncConfigurator())
- Add conditional viz: Rerun when --viewer rerun, Foxglove when --viewer foxglove
- Add --replay support (connection_string='replay')
Clean autoconnect() composition with _vis sub-blueprint for conditional
viewer selection. No _modules list, no .insert() hack.
- drone_agentic now imports drone_basic and layers tracking + skills + agent
- Removed n_workers=4 and ClockSyncConfigurator from drone_basic
- Removed duplicate vis/replay logic from drone_agentic
- Removed fill_mode='wireframe' (invalid Rerun FillMode)
- Matches Go2 composition pattern
- Rewrite README for CLI-based usage (dimos run drone-basic/agentic)
- Document blueprint composition pattern
- Add indoor/outdoor mode, replay, Rerun/Foxglove visualization
- Keep RosettaDrone setup verbatim
- Add return type annotations to fix mypy no-untyped-def
- Lazy-init _last_log in _on_message instead of start() so tests can
  call _on_message without calling start() first
- Fix test mock to use spec=RerunConvertible so messages pass through
  the visual override pipeline
- Add bridge.stop() cleanup to prevent thread leak warnings
These changes (rate limiter + test) were incorrectly added by the
subagent in the drone modernization branch. They belong in a separate PR.
@spomichter spomichter force-pushed the feat/drone-modernize branch from 9280def to a62d0ae Compare March 11, 2026 19:50
@spomichter spomichter merged commit 9b9c817 into dev Mar 11, 2026
12 checks passed
@spomichter spomichter deleted the feat/drone-modernize branch March 11, 2026 20:40
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