Skip to content

Conversation

@leshy
Copy link
Contributor

@leshy leshy commented Jan 9, 2026

No description provided.

@leshy leshy requested a review from a team January 9, 2026 09:19
Copy link

@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.

Greptile Overview

Greptile Summary

This PR introduces a new visual navigation module (visual_nav2.py) and integrates it with the Unitree GO2 robot blueprints. The changes migrate from ObjectDBModule to Detection3DModule for 3D detection processing.

Key Changes

New Visual Navigation Module (visual_nav2.py)

  • Creates VisNavSkills module for visual navigation using vision-language models (Moondream)
  • Defines inputs for color_image and detections streams
  • Implements visual_navigation() method to query VLM for target objects
  • Includes commented-out detections_stream() method for aligning image and detection data

Blueprint Integration (unitree_go2_blueprints.py)

  • Migrates from ObjectDBModule/detectionDB_module to Detection3DModule/detection3d_module
  • Adds vis_nav_skills() to the detection pipeline via autoconnect()
  • Updates all module references in remappings and transports from ObjectDBModule to Detection3DModule
  • Comments out unused detection transport (line 109-111)
  • Increases Dask worker count from 6 to 8 for the detection configuration

Critical Issues Found

The new visual_nav2.py module has several blocking errors that will prevent runtime execution:

  1. Missing imports: field and VlModel are not imported but used in the code
  2. Broken start() method: Calls non-existent detections_stream() method that is commented out
  3. Non-functional visual_navigation(): Creates an observable but doesn't subscribe to it, so no processing occurs

These issues are consistent with the "WIP" (Work In Progress) status of this PR.

Confidence Score: 1/5

  • This PR has critical syntax and logic errors that will cause immediate runtime failures
  • The new visual_nav2.py module contains multiple blocking issues: missing imports (field, VlModel) will cause NameError exceptions, the start() method calls a non-existent method causing AttributeError, and the visual_navigation() method is non-functional. These are not minor issues - the module will fail immediately upon instantiation. The blueprint changes appear structurally sound but the integration with the broken VisNavSkills module makes the entire detection pipeline unstable. Given the WIP status, this is expected, but the code is definitely not merge-ready.
  • dimos/skills/visual_nav2.py requires immediate attention - all import errors must be fixed and the start() method needs to be corrected before this module can function

Important Files Changed

File Analysis

Filename Score Overview
dimos/skills/visual_nav2.py 1/5 New visual navigation module with critical import errors and incomplete implementation - start() method calls non-existent detections_stream(), visual_navigation() creates unused observable
dimos/robot/unitree_webrtc/unitree_go2_blueprints.py 3/5 Refactored detection module from ObjectDBModule to Detection3DModule, integrated new vis_nav_skills, updated remappings and transports, increased dask workers from 6 to 8

Sequence Diagram

sequenceDiagram
    participant GO2 as GO2Connection
    participant Det3D as Detection3DModule
    participant VisNav as VisNavSkills
    participant VLM as MoondreamVLModel
    
    Note over GO2,VLM: Visual Navigation System Flow
    
    GO2->>Det3D: color_image (In)
    GO2->>Det3D: pointcloud (In)
    
    Det3D->>Det3D: Process 3D detections
    Det3D->>Det3D: Generate 2D detections
    
    Det3D->>VisNav: color_image (forwarded)
    Det3D->>VisNav: detections (Out->In)
    
    Note over VisNav: start() called on initialization
    VisNav->>VisNav: detections_stream() [BROKEN - method commented out]
    
    Note over VisNav: visual_navigation(target) method
    VisNav->>VLM: query_detections(image, target)
    VLM-->>VisNav: ImageDetections2D
    Note over VisNav: Observable created but not subscribed [BROKEN]
    
    Det3D->>Det3D: Output visualization streams
    Det3D-->>GO2: detected_pointcloud_0/1/2
    Det3D-->>GO2: detected_image_0/1/2
    Det3D-->>GO2: annotations
Loading

# See the License for the specific language governing permissions and
# limitations under the License.

from dataclasses import dataclass
Copy link

Choose a reason for hiding this comment

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

Missing import for field from dataclasses. Line 31 uses field(default_factory=MoondreamVLModel) but field is not imported.

Suggested change
from dataclasses import dataclass
from dataclasses import dataclass, field

from reactivex.observable import Observable

from dimos.core import In, Module, ModuleConfig, Out, rpc
from dimos.models.vl.moondream import MoondreamVLModel
Copy link

Choose a reason for hiding this comment

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

Missing import for VlModel. The type annotation on line 31 (vlmodel: VlModel) and line 41 (vlmodel: VlModel) reference VlModel but it's not imported.

Suggested change
from dimos.models.vl.moondream import MoondreamVLModel
from dimos.models.vl.base import VlModel

self.vlmodel = self.config.vlmodel()

def start(self) -> None:
self._disposables.add(self.detections_stream().subscribe(print))
Copy link

Choose a reason for hiding this comment

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

The start() method calls self.detections_stream() but this method is commented out (lines 58-74). This will cause an AttributeError at runtime when the module starts.

Either:

  1. Uncomment the detections_stream() method if it's needed, or
  2. Remove or comment out this call if the functionality is not yet ready

Since this is a WIP PR, the call should likely be commented out until detections_stream() is implemented.

Comment on lines +50 to +56
def visual_navigation(self, target: str) -> None:
self.color_image.observable().pipe(
ops.map(lambda img: self.vlmodel.query_detections(img, target)),
ops.filter(lambda d: d.detections_length > 0),
)

print(f"Navigating to {target} using visual navigation.")
Copy link

Choose a reason for hiding this comment

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

The visual_navigation() method creates an observable pipeline but doesn't subscribe to it or return it. The pipeline result is unused, which means:

  1. No actual processing will happen when this method is called
  2. The detections query won't be executed
  3. Only the print statement will execute

The observable should either be:

  1. Subscribed to (e.g., .subscribe(...)) so it executes, or
  2. Returned so the caller can subscribe to it, or
  3. Stored as an instance variable for later use

Based on similar patterns in the codebase, this method likely needs to be completed before it's functional.

@leshy leshy closed this Jan 25, 2026
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.

2 participants