Skip to content

Update to Jetty, introduce flowstate_ros_gz_bridge#105

Open
luca-della-vedova wants to merge 21 commits intomainfrom
luca/jetty_with_vendored_bridge
Open

Update to Jetty, introduce flowstate_ros_gz_bridge#105
luca-della-vedova wants to merge 21 commits intomainfrom
luca/jetty_with_vendored_bridge

Conversation

@luca-della-vedova
Copy link
Copy Markdown
Contributor

Overview

This PR updates the version of the vendored gz packages to target Gazebo Jetty, as well as introduce a vendored ros_gz_bridge (I believe we need to vendor it because we want to build against custom versions of gz_transport, gz_msgs), as well as a flowstate_ros_gz_bridge service built on top of it that can be used to bridge between the Gazebo running in Flowstate and the user's machine.
I had to update the dockerfile for services slightly to make sure that system dependencies installed through rosdep install are available in the image that is used to run the service. I did so by doing dpkg --get-selections as suggested by @wjwwood .
The bridge has a default configuration to bridge the clock into ROS to run node in simulation time, but the configuration can be set through the proto when instancing it.

Test it!

The testing involves building the flowstate_ros_gz_bridge service and checking that the clock is successfully bridged:

# Build and bundle the service
$ ./src/sdk-ros/scripts/build_container.sh --service_package flowstate_ros_gz_bridge --service_name flowstate_ros_gz_bridge_service
$ ./src/sdk-ros/scripts/build_bundle.sh --service_package flowstate_ros_gz_bridge --service_name flowstate_ros_gz_bridge_service
# Install it
# SERVICE_BUNDLE points to the bundle generated above, set the organization and context to point to your VM
$ inctl asset install --org $INTRINSIC_ORGANIZATION --cluster $INTRINSIC_CONTEXT $SERVICE_BUNDLE

Then instantiate it either from a terminal or the flowstate UI, on a third machine:

# Make sure all the ROS shells are running with RMW_IMPLEMENTATION=rmw_zenoh_cpp
# Run the zenoh router
$ ZENOH_CONFIG_OVERRIDE='connect/endpoints=["tcp/127.0.0.1:17447"]' ros2 run rmw_zenoh_cpp rmw_zenohd
# SSH and port forward
$ inctl ssh --project $INTRINSIC_ORGANIZATION --context $INTRINSIC_CONTEXT --no-tofu -- -N -L 17080:localhost:17080 -L 17447:localhost:17447
# Last terminal
$ ros2 topic echo /clock --no-daemon

You should see the clock topic being bridged to your local machine!

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@google.com>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Copy link
Copy Markdown

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

Nice! I was able to follow the instructions to sideload it into a flowstate solution and confirmed that messages are coming through on the /clock.

I left some minor comments. I think it also helps to add the instructions you have in the PR description to a README file?

Comment thread flowstate_ros_gz_bridge/src/flowstate_ros_gz_bridge.cc
Comment thread flowstate_ros_gz_bridge/src/flowstate_ros_gz_bridge.cc Outdated
Comment thread flowstate_ros_gz_bridge/package.xml Outdated
<version>0.0.1</version>
<description>Version of ROS GZ bridge to bridge with Flowstate Gazebo</description>
<maintainer email="lucadv@intrinsic.ai">Luca Della Vedova</maintainer>
<license>Intrinsic License</license>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see Intrinsic License used in different package.xml files in this PR. should Apache-2.0 be used instead?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@wjwwood, we're talking about the license offline. This package uses sim_connection.cc and sim_connection.hh which are adapted from code in insrc (which we eventually want to make public). Do you know if there is process to go through to ensure we can use Apache-2.0, or if there is an appropriate license to use?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't know what process you need to go through. We just got permission to change the license at some point for all of sdk-ros. @tfoote would know maybe?

I don't think it should be merged here until the license is apache-2.0.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should also have a LICENSE file in each new package.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@tfoote, any advice on how we can proceed here? Is it ok to use Apache-2.0 license?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated to Apache 2.0 since it's a fairly trivial piece of code that we intend to make public anyway in dd84f41

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, please use Apache-2.0 for this packages and files to be consistent with the rest of the repo.

Comment thread flowstate_ros_gz_bridge/src/sim_connection.h Outdated
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
@luca-della-vedova
Copy link
Copy Markdown
Contributor Author

I think it also helps to add the instructions you have in the PR description to a README file?

Yap good point, added in 0f48753

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Copy link
Copy Markdown
Collaborator

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Need to resolve the license before merging.

Comment thread flowstate_ros_gz_bridge/package.xml Outdated
<version>0.0.1</version>
<description>Version of ROS GZ bridge to bridge with Flowstate Gazebo</description>
<maintainer email="lucadv@intrinsic.ai">Luca Della Vedova</maintainer>
<license>Intrinsic License</license>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should also have a LICENSE file in each new package.

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
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.

4 participants