Skip to content

Comments

feat: [DIM-530] AGIbot nav test blueprint using ROSNav bridge#1288

Open
spomichter wants to merge 3 commits intodevfrom
stash/dim-530-agibot-nav-v2
Open

feat: [DIM-530] AGIbot nav test blueprint using ROSNav bridge#1288
spomichter wants to merge 3 commits intodevfrom
stash/dim-530-agibot-nav-v2

Conversation

@spomichter
Copy link
Contributor

Composes ROSNav with explicit transport assignments:

  • ros_* In ports use ROSTransport (subscribe from AGIbot ROS topics)
  • DimOS Out ports use LCMTransport (publish to lcmspy/rerun)
  • ROSNav bridges internally: ROS → handler code → LCM

Registered as 'agibot-nav-test' in all_blueprints.py

Composes ROSNav with explicit transport assignments:
- ros_* In ports use ROSTransport (subscribe from AGIbot ROS topics)
- DimOS Out ports use LCMTransport (publish to lcmspy/rerun)
- ROSNav bridges internally: ROS → handler code → LCM

Registered as 'agibot-nav-test' in all_blueprints.py
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 18, 2026

Greptile Summary

This PR adds a new AGIbot navigation test blueprint (agibot_nav_test) that composes the existing ROSNav module with explicit transport assignments. ROS In ports subscribe from the AGIbot's ROS navigation topics via ROSTransport, while DimOS Out ports publish to lcmspy/rerun via LCMTransport. The blueprint also configures ROS Out ports for publishing goals, cancellation, and joystick commands back to the AGIbot stack.

  • The blueprint file itself (agibot_nav_test.py) follows established patterns well, using autoconnect(ros_nav()).transports({...}) with clear transport mappings.
  • Critical issue: The blueprint is registered in all_modules instead of all_blueprints in the auto-generated all_blueprints.py. This will cause dimos run agibot-nav-test to fail at runtime because the CLI resolves the primary blueprint name via get_blueprint_by_name(), which only checks all_blueprints. Additionally, the all_modules loader uses the dict key as an getattr identifier, but "agibot-nav-test" contains hyphens which are invalid in Python identifiers.
  • The fix is to not manually edit all_blueprints.py — instead, run pytest dimos/robot/test_all_blueprints_generation.py to auto-generate the correct entry in all_blueprints.

Confidence Score: 2/5

  • This PR has a runtime-breaking registration bug that will prevent the blueprint from being runnable via dimos run.
  • The blueprint code itself is well-structured, but it is registered in the wrong dictionary (all_modules instead of all_blueprints) in a file that is auto-generated and should not be manually edited. The current registration will cause a runtime failure when attempting to run the blueprint.
  • dimos/robot/all_blueprints.py — the entry needs to be moved from all_modules to all_blueprints (ideally by running the auto-generator).

Important Files Changed

Filename Overview
dimos/robot/agibot/blueprints/test/agibot_nav_test.py New AGIbot navigation test blueprint composing ROSNav with explicit transport assignments. Well-structured, follows existing patterns for blueprint files.
dimos/robot/all_blueprints.py Blueprint registered in all_modules instead of all_blueprints, and this auto-generated file was manually edited. The entry will cause a runtime failure.
dimos/robot/agibot/init.py Empty init file for new agibot package. No issues.
dimos/robot/agibot/blueprints/init.py Empty init file for new blueprints package. No issues.
dimos/robot/agibot/blueprints/test/init.py Empty init file for new test blueprints package. No issues.
dimos/robot/agibot/modules/init.py Empty init file for future agibot modules. No issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph ROS["AGIbot ROS Navigation Stack"]
        R1["/goal_reached"]
        R2["/cmd_vel"]
        R3["/way_point"]
        R4["/registered_scan"]
        R5["/terrain_map_ext"]
        R6["/path"]
        R7["/tf"]
        R8["/goal_pose"]
        R9["/cancel_goal"]
        R10["/stop"]
        R11["/joy"]
    end

    subgraph Blueprint["agibot_nav_test Blueprint"]
        subgraph ROSIn["ROS In Ports (ROSTransport)"]
            RI1["ros_goal_reached"]
            RI2["ros_cmd_vel"]
            RI3["ros_way_point"]
            RI4["ros_registered_scan"]
            RI5["ros_global_pointcloud"]
            RI6["ros_path"]
            RI7["ros_tf"]
        end
        NAV["ROSNav Module"]
        subgraph ROSOut["ROS Out Ports (ROSTransport)"]
            RO1["ros_goal_pose"]
            RO2["ros_cancel_goal"]
            RO3["ros_soft_stop"]
            RO4["ros_joy"]
        end
        subgraph DimOut["DimOS Out Ports (LCMTransport)"]
            D1["pointcloud → /lidar"]
            D2["global_pointcloud → /map"]
            D3["goal_req → /goal_req"]
            D4["goal_active → /goal_active"]
            D5["path_active → /path_active"]
            D6["cmd_vel → /cmd_vel"]
        end
    end

    subgraph LCM["DimOS Consumers"]
        L1["lcmspy / rerun"]
    end

    R1 -->|ROSTransport| RI1
    R2 -->|ROSTransport| RI2
    R3 -->|ROSTransport| RI3
    R4 -->|ROSTransport| RI4
    R5 -->|ROSTransport| RI5
    R6 -->|ROSTransport| RI6
    R7 -->|ROSTransport| RI7

    RI1 --> NAV
    RI2 --> NAV
    RI3 --> NAV
    RI4 --> NAV
    RI5 --> NAV
    RI6 --> NAV
    RI7 --> NAV

    NAV --> RO1
    NAV --> RO2
    NAV --> RO3
    NAV --> RO4
    NAV --> D1
    NAV --> D2
    NAV --> D3
    NAV --> D4
    NAV --> D5
    NAV --> D6

    RO1 -->|ROSTransport| R8
    RO2 -->|ROSTransport| R9
    RO3 -->|ROSTransport| R10
    RO4 -->|ROSTransport| R11

    D1 -->|LCMTransport| L1
    D2 -->|LCMTransport| L1
    D3 -->|LCMTransport| L1
    D4 -->|LCMTransport| L1
    D5 -->|LCMTransport| L1
    D6 -->|LCMTransport| L1
Loading

Last reviewed commit: 6173b70

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

"replanning_a_star_planner": "dimos.navigation.replanning_a_star.module",
"rerun_bridge": "dimos.visualization.rerun.bridge",
"ros_nav": "dimos.navigation.rosnav",
"agibot-nav-test": "dimos.robot.agibot.blueprints.test.agibot_nav_test:agibot_nav_test",
Copy link
Contributor

Choose a reason for hiding this comment

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

Blueprint registered in wrong dictionary

agibot_nav_test is a blueprint (created via autoconnect(...).transports(...)) but is registered in all_modules instead of all_blueprints. This will cause a runtime failure when running dimos run agibot-nav-test:

  1. get_blueprint_by_name() looks up the name in all_blueprints — it won't find "agibot-nav-test" there, so it raises ValueError("Unknown blueprint set name: agibot-nav-test").

  2. Even if this were somehow resolved to get_module_by_name(), that function does getattr(python_module, name)() where name is the dict key "agibot-nav-test". Since Python identifiers cannot contain hyphens, getattr(module, "agibot-nav-test") would raise AttributeError. The actual exported variable is agibot_nav_test (underscores).

  3. all_blueprints.py is auto-generated (line 15: "Do not edit manually"). The auto-scanner in test_all_blueprints_generation.py would classify agibot_nav_test as a blueprint (because the assignment ends with .transports(), a blueprint method), so running the generator should produce the correct entry automatically.

Remove this manual edit and instead run pytest dimos/robot/test_all_blueprints_generation.py to regenerate. The expected auto-generated entry would be in all_blueprints:

Suggested change
"agibot-nav-test": "dimos.robot.agibot.blueprints.test.agibot_nav_test:agibot_nav_test",

With the auto-generated entry in all_blueprints being:

"agibot-nav-test": "dimos.robot.agibot.blueprints.test.agibot_nav_test:agibot_nav_test",

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