Skip to content

Inheritance and abstract class fixes for stereo#963

Merged
spomichter merged 5 commits intodevfrom
alexl_stereo_camera_inheritance_fix
Jan 8, 2026
Merged

Inheritance and abstract class fixes for stereo#963
spomichter merged 5 commits intodevfrom
alexl_stereo_camera_inheritance_fix

Conversation

@alexlin2
Copy link
Copy Markdown
Contributor

@alexlin2 alexlin2 commented Jan 8, 2026

-Fixed Stash and Ivan's comments about stereo camera inheritance
-renamed "stereo" to "depth" camera since most depth cameras are not stereo

@alexlin2 alexlin2 requested review from a team, leshy and spomichter January 8, 2026 03:18
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jan 8, 2026

Greptile Summary

Renamed stereo camera classes to depth camera classes and fixed inheritance structure by adding perception.DepthCamera protocol to RealSenseCamera and ZEDCamera.

Major Changes:

  • Renamed StereoCameraDepthCameraHardware and StereoCameraConfigDepthCameraConfig to better reflect that most depth cameras are not stereo cameras
  • Added perception.DepthCamera protocol with depth_image and depth_camera_info outputs
  • Updated RealSenseCamera and ZEDCamera to inherit from DepthCameraHardware, Module, and perception.DepthCamera
  • Standardized camera config parameters: frequencyfps, frame_width/frame_heightwidth/height
  • Moved width, height, and fps fields up to base CameraConfig protocol
  • Added proper handling for fps <= 0 case in webcam capture loop

Issues Found:

  • DepthCameraConfig should be marked as a Protocol since it inherits from CameraConfig (which is a Protocol) and only defines structural requirements without implementations

Confidence Score: 4/5

  • This PR is safe to merge with minor type-checking concerns
  • The refactoring properly addresses the inheritance issues and renames stereo to depth camera throughout the codebase. The changes are consistent and well-structured. However, there is one syntax issue where DepthCameraConfig should be marked as a Protocol to maintain proper type-checking behavior, which prevents a perfect score.
  • Pay close attention to dimos/hardware/sensors/camera/spec.py - the DepthCameraConfig class needs the Protocol designation

Important Files Changed

Filename Overview
dimos/hardware/sensors/camera/spec.py Renamed StereoCamera to DepthCamera classes, added standardized config fields (width, height, fps) to CameraConfig, but DepthCameraConfig lost Protocol designation which could cause type checking issues
dimos/hardware/sensors/camera/realsense/camera.py Updated imports and class names from Stereo to Depth, added perception.DepthCamera to inheritance chain for proper type support
dimos/hardware/sensors/camera/zed/camera.py Updated imports and class names from Stereo to Depth, added perception.DepthCamera to inheritance chain for proper type support
dimos/spec/perception.py Added DepthCamera protocol with depth_image and depth_camera_info outputs, extending Camera protocol
dimos/hardware/sensors/camera/webcam.py Standardized config parameters (frequency→fps, frame_width/frame_height→width/height) and added proper handling for fps<=0 case

Sequence Diagram

sequenceDiagram
    participant User
    participant RealSenseCamera
    participant DepthCameraHardware
    participant Module
    participant DepthCamera as perception.DepthCamera
    
    Note over RealSenseCamera,DepthCamera: Inheritance Chain
    RealSenseCamera->>DepthCameraHardware: inherits abstract methods
    RealSenseCamera->>Module: inherits module functionality
    RealSenseCamera->>DepthCamera: implements protocol
    
    User->>RealSenseCamera: instantiate with config
    Note over RealSenseCamera: RealSenseCameraConfig extends<br/>ModuleConfig + DepthCameraConfig
    
    User->>RealSenseCamera: start()
    RealSenseCamera->>RealSenseCamera: initialize pipeline
    RealSenseCamera->>RealSenseCamera: build camera info
    RealSenseCamera->>RealSenseCamera: start capture thread
    
    loop Capture Loop
        RealSenseCamera->>RealSenseCamera: wait_for_frames()
        RealSenseCamera->>DepthCamera: publish(color_image)
        RealSenseCamera->>DepthCamera: publish(depth_image)
        RealSenseCamera->>Module: publish(tf transforms)
    end
    
    User->>RealSenseCamera: get_color_camera_info()
    RealSenseCamera-->>User: CameraInfo
    User->>RealSenseCamera: get_depth_camera_info()
    RealSenseCamera-->>User: CameraInfo
    User->>RealSenseCamera: get_depth_scale()
    RealSenseCamera-->>User: float
Loading

Copy link
Copy Markdown
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.

9 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile


class StereoCameraConfig(Protocol):
"""Protocol for stereo camera configuration."""
class DepthCameraConfig(CameraConfig):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

syntax: DepthCameraConfig should be marked as a Protocol since it inherits from CameraConfig (which is a Protocol) and only defines structural requirements without implementations

Suggested change
class DepthCameraConfig(CameraConfig):
class DepthCameraConfig(CameraConfig, Protocol):

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@spomichter spomichter merged commit 888999f into dev Jan 8, 2026
13 checks passed
@spomichter spomichter deleted the alexl_stereo_camera_inheritance_fix branch January 8, 2026 08:33
spomichter pushed a commit that referenced this pull request Jan 8, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 888999f
spomichter pushed a commit that referenced this pull request Jan 8, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 888999f
spomichter pushed a commit that referenced this pull request Jan 8, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 35ea42c [formerly 888999f]
Former-commit-id: c23b8bc
spomichter pushed a commit that referenced this pull request Jan 8, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 35ea42c [formerly 888999f]
Former-commit-id: c23b8bc
paul-nechifor pushed a commit that referenced this pull request Jan 8, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 35ea42c [formerly 888999f]
Former-commit-id: 528c6d6
jeff-hykin pushed a commit that referenced this pull request Jan 9, 2026
* make inheritance and abstract class fixes

* CI code cleanup

* renamed stereo to depth camera since most depth cameras are not stereo cameras

* bug fix

* fix mypy

Former-commit-id: 35ea42c [formerly 888999f]
Former-commit-id: 528c6d6
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