Skip to content

New g1 blueprint runfiles with ruff#707

Closed
paul-nechifor wants to merge 9 commits intodevfrom
new-g1-blueprint-runfiles-with-ruff
Closed

New g1 blueprint runfiles with ruff#707
paul-nechifor wants to merge 9 commits intodevfrom
new-g1-blueprint-runfiles-with-ruff

Conversation

@paul-nechifor
Copy link
Contributor

No description provided.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

def _deploy_connection(self) -> None:
"""Deploy and configure the connection module."""
self.connection = self._dimos.deploy(G1ConnectionModule, self.ip)
# Configure LCM transports
self.connection.movecmd.transport = core.LCMTransport("/cmd_vel", TwistStamped)
self.connection.odom_in.transport = core.LCMTransport("/state_estimation", Odometry)

P1 Badge G1 connection still expects TwistStamped despite interface change

G1ConnectionModule was updated to accept In[Twist] and forwards the Twist directly to UnitreeWebRTCConnection.move, but _deploy_connection still connects the input to an LCM transport that deserializes TwistStamped. At runtime the connection’s move method will receive a TwistStamped instance (which lacks .linear/.angular at the top level), raising attribute errors and preventing motion commands. The transport should now publish Twist messages to match the updated module signature.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +431 to +441
def deploy(dimos: DimosCluster):
nav = dimos.deploy(ROSNav)
nav.pointcloud.transport = pSHMTransport("/lidar")
nav.global_pointcloud.transport = pSHMTransport("/map")
# nav.pointcloud.transport = LCMTransport("/lidar", PointCloud2)
# nav.global_pointcloud.transport = LCMTransport("/map", PointCloud2)

nav.goal_req.transport = LCMTransport("/goal_req", PoseStamped)
nav.goal_active.transport = LCMTransport("/goal_active", PoseStamped)
nav.path_active.transport = LCMTransport("/path_active", Path)
nav.cmd_vel.transport = LCMTransport("/cmd_vel", TwistStamped)

Choose a reason for hiding this comment

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

P1 Badge Misconfigured nav deploy uses inert base class and wrong message type

The new deploy helper instantiates ROSNav and wires its transports, but ROSNav is just a stub with empty start/stop methods. None of the ROS2 subscriptions or conversions implemented in ROSNavigationModule are created, so calling this function produces a module that never bridges the robot topics. In the same block cmd_vel is bound to LCMTransport(..., TwistStamped) even though ROSNavigationModule.cmd_vel publishes Twist, which will cause serialization/type errors when a command is emitted. Use ROSNavigationModule here and keep the transport type consistent with the module’s Out[Twist] output.

Useful? React with 👍 / 👎.

@paul-nechifor
Copy link
Contributor Author

Closed in favor of #724 . That PR contains the g1 blueprints, some fixes and adds the G1 sim.

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.

3 participants