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

Multiple cameras + compression #248

Merged
merged 5 commits into from
Jul 22, 2023
Merged

Conversation

brendoncintas
Copy link

I noticed that for the demo_launch, the support is mostly for one USB camera. I've modified the launch file to include other cameras, and remapped each image_raw and image_compressed topic to be on a per-camera basis rather than all of them being on one topic.

A note about this:

  • This only works for ROS2.
  • I've commented out the extra usb_cam nodes as I'll be needing 4 for my robot, but you can add or remove as needed.
  • For my application, I didn't need the show_image.py script, so I removed it from the launch file. This could be added back if needed.

Also, I've cleaned up the code in the launch file to make each camera an individual node within a group, then launch the whole group with LaunchDescription, rather than each node being launched individually. Should make the code look a little cleaner.

FROM #247. I didn't PR the right branch. Whoops.

@flynneva
Copy link
Collaborator

@brendoncintas I like this idea of making it easier for users to launch multiple cameras at once. I've taken your branch and made one small improvement to it by making the config into a class that should make it dead simple for people to add new cameras to the launch file.

You can see the changes I made here.

To add new cameras to the launch file, now all that is needed is to add a few lines here that adds a new CameraConfig class to the list and thats it:

CameraConfig(
  name="camera1",
  param_path=Path(path, to, param file),
)

If you like them as well, then I can add them to this branch I think and we can go ahead with this PR.

Let me know what you think!

@brendoncintas
Copy link
Author

@brendoncintas I like this idea of making it easier for users to launch multiple cameras at once. I've taken your branch and made one small improvement to it by making the config into a class that should make it dead simple for people to add new cameras to the launch file.

You can see the changes I made here.

To add new cameras to the launch file, now all that is needed is to add a few lines here that adds a new CameraConfig class to the list and thats it:

CameraConfig(
  name="camera1",
  param_path=Path(path, to, param file),
)

If you like them as well, then I can add them to this branch I think and we can go ahead with this PR.

Let me know what you think!

That is an incredible idea, and I'm all for it. Go ahead and add them to the branch and go ahead with the PR.

@flynneva flynneva self-requested a review July 22, 2023 19:56
config/params_2.yaml Outdated Show resolved Hide resolved
config/params_2.yaml Outdated Show resolved Hide resolved
config/params_2.yaml Show resolved Hide resolved
Copy link
Author

@brendoncintas brendoncintas left a comment

Choose a reason for hiding this comment

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

This should be good to go!

@flynneva
Copy link
Collaborator

@brendoncintas thanks, I cleaned up the commits and fixed the linter errors. Lets wait for CI to run again as it should pass now

Signed-off-by: Evan Flynn <evanflynn.msu@gmail.com>
Signed-off-by: Evan Flynn <evanflynn.msu@gmail.com>
@flynneva flynneva merged commit e79853f into ros-drivers:ros2 Jul 22, 2023
@flynneva
Copy link
Collaborator

@brendoncintas thanks for the contribution! 🙏🏼

@brendoncintas
Copy link
Author

Glad to have been a part of it!

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