Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature foxglove bridge #607

Merged
merged 14 commits into from
Dec 5, 2023
Merged

Feature foxglove bridge #607

merged 14 commits into from
Dec 5, 2023

Conversation

victorjarlow
Copy link
Collaborator

@victorjarlow victorjarlow commented Oct 13, 2023

  • Updated docs to point users toward the foxglove studio docker implementation instead of studio.foxglove.dev.
  • Adopted foxglove_bridge as the default bridge, instead of ros_bridge (see motivation here: https://github.com/foxglove/ros-foxglove-bridge#motivation).
  • Added options to launch_basic such that bridge security and implementation can be controlled.
  • Updated install instructions and install script
  • Logs end up in ~/.ros/logs/date-xx-xxx/launch.log on my system (I have not set ROS_HOME or ROS_LOG_DIR)

example: ros2 launch atos launch_basic.py insecure:=True foxbridge:=True

If no input arguments are specified, the options are set to insecure=False, foxbridge=True.

Motivation for this PR:

  • The PR removes the need for accepting https://localhost:3443 certificate
  • The need for accepting the https://localhost:8765 certificate still remains..
  • We can change the standard ros launch command to "ros2 launch atos launch_basic insecure:=True" to highlight that the bridge protocol is insecure and that the user can do extra steps (accept cert at https://localhost:8765) in order to simply use the default launch command, but this requires extra steps.

install_atos.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@Robert108 Robert108 left a comment

Choose a reason for hiding this comment

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

Really nice that we can avoid the problems a lot of people are having with the certificates with this.

docker-compose.yml Outdated Show resolved Hide resolved
docs/Usage/GUI/foxglove.md Show resolved Hide resolved
launch/launch_utils/launch_base.py Show resolved Hide resolved
launch/launch_utils/launch_base.py Show resolved Hide resolved
Copy link
Contributor

@samuelthoren samuelthoren left a comment

Choose a reason for hiding this comment

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

I agree with Robert's comments, otherwise it looks good to merge!

@victorjarlow
Copy link
Collaborator Author

@Robert108 requesting re-review

@victorjarlow victorjarlow merged commit 4e2e495 into dev Dec 5, 2023
0 of 2 checks passed
@victorjarlow victorjarlow deleted the feature_foxglove_bridge branch December 5, 2023 13:52
@victorjarlow victorjarlow restored the feature_foxglove_bridge branch December 5, 2023 13:57
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