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

[sensors] Launch vectornav in bringup.launch.py #229

Merged
merged 13 commits into from
Nov 24, 2024
Merged

Conversation

mazam32
Copy link
Contributor

@mazam32 mazam32 commented Nov 1, 2024

Description

This PR does the following:

  • Changed the bringup launch file so that it launches vectornav launch file implicitly
  • Provides a new file in documents folder to fix issues with laptops with access denied issues.

Copy link
Member

@davidcalderon03 davidcalderon03 left a comment

Choose a reason for hiding this comment

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

Please remove the build, install, and log files from this PR. You need to build the repo in rover-colon, not src

Copy link
Member

@davidcalderon03 davidcalderon03 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@yambati03 yambati03 left a comment

Choose a reason for hiding this comment

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

The changes to bringup.launch.py look good, but had a few comments about the other checked in code. Also, we'll likely want a way to quickly zero the IMU (not sure if the vectornav already comes with a service to do this). I'd also add some instructions for calibrating the IMU and setting the correct magnetic declination.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file supposed to be here? It is empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this IMU parser node anymore. There should be an option in the config file for the vectornav package that will automatically publish a ROS 2 sensor_msgs/Imu message.

@yambati03 yambati03 changed the title Feat/vectornav [sensors] launch vectornav in bringup.launch.py Nov 3, 2024
@yambati03 yambati03 changed the title [sensors] launch vectornav in bringup.launch.py [sensors] Launch vectornav in bringup.launch.py Nov 3, 2024
@davidcalderon03 davidcalderon03 merged commit efb99ed into master Nov 24, 2024
3 of 4 checks passed
@davidcalderon03 davidcalderon03 deleted the feat/vectornav branch November 24, 2024 21:48
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