ros-drivers / audio_common

Common code for working with audio in ROS
BSD 3-Clause "New" or "Revised" License
86 stars 151 forks source link

Converted nodes to components #214

Closed weeshal closed 1 year ago

weeshal commented 1 year ago

Made changes to audio_capture and audio_play nodes:

This PR closes #209

weeshal commented 1 year ago

This change will allow the ability to load an unload these nodes (utilizing the filesink more effectively), here is an example:

  1. Running the following command in a terminal will launch a generic component container. ros2 run rclcpp_components component_container
  2. In a second terminal, running ros2 component list will show the available containers (/ComponentManager by default) and ros2 component types will produce the available components image
  3. ros2 component load /ComponentManager audio_capture audio_transport::RosGstCapture -p dst:=audio4.mp3 && sleep 10 && ros2 component unload /ComponentManager 1 will record a 10s mp3 file. (NOTE: each component loaded has a unique uid - the next time you run this, you should use ros2 component unload /ComponentManager 2)
weeshal commented 1 year ago

@knorth55 would you be able to test the audio_play node with this change? I wasn't able to get audio playing even before this change.

knorth55 commented 1 year ago

currently i dont have much time so i will check it later. but i have one things i want to tell you.

this pr includes several changes which are not related to components. please split these changes into other PR? PR should be compact for the review.

for example... do you really need to change the audio_capture to audio_capture_node?

weeshal commented 1 year ago

Certainly! The convention that I have followed is naming the libraries with the project name, and the executable to project_name_node. I can maybe do project_name_libraries and project_name so that all the launch changes are removed. EDIT: this should be better

knorth55 commented 1 year ago

You don't need to change the node executable name, but I think we should keep the node name in launch file, I mean.

knorth55 commented 1 year ago

this PR is moved to #215