search

ros-drivers / audio_common

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

[Refactor #214] converted nodes to components #215

Closed knorth55 closed 1 year ago

knorth55 commented 1 year ago

Hmm. I have a problem. In my environment, node name does not change with launch file. Can't we change the node name from launch in ROS2?

$ ros2 param list
/audio/audio_capture_node:
  bitrate
  channels
  depth
  device
  dst
  format
  sample_format
  sample_rate
  use_sim_time
/launch_ros_22823:
  use_sim_time

$ ros2 node list
/audio/audio_capture_node
/launch_ros_22823
knorth55 commented 1 year ago

I found out that my ROS2 distro is too old, Eloquent. I need to set node-name instead of name.

knorth55 commented 1 year ago

@weeshal can you check if this PR works?

weeshal commented 1 year ago

@knorth55

+1 on the change to python launch files!

I'm using eloquent as well, and I seem to be facing a similar issue with the arguments passed into launch_ros.Node. How would you like to go about solving this - retain the changes necessary for Eloquent or target this towards Humble? This would be difficult without version branching like you have for ros1.

weeshal commented 1 year ago

Some findings:

Here is a PR with the functional changes: https://github.com/knorth55/audio_common/pull/2

knorth55 commented 1 year ago

Currently some of the namespacing across launch files is inconsistent - the capture.launch.xml uses /audio whereas the other 2 in that package have no namespace.

What do you mean? All the launch files have the same namespace /audio.

The .launch.xml files seem to use the value set in audio_capture_node.cpp field to name the node whereas the .launch.py files use the node_name(eloquent)/ name(later) field provided in the launch file. That should be kept consistent (you can decide whether that is suffixed with _node or not, as maintainer)

eloquent is EOL, we should use name, I think.

Default value for audio filename is output.mp3 but default format is wave. The audio file plays fine, but could be kept consistent.

Thank you, I will fix it.

I am unable to save/play files through the audio_play_node, I had this issue even before changes. If you are able to play audio, then it is some configuration error but if you are unable too, I can try to debug what might be going on. EDIT: looks like the messages are not being received.

You can just try running audio_capture and audio_play on one laptop with microphone and speaker. You can hear the sound from microphone. I recommend to use earphones because speaker causes howling.

weeshal commented 1 year ago

I mean the capture_wave.launch.xml does not have the arguments as the other 2 launch files. So it does not publish to /audio/audio as intended.

sounds good regarding not supporting eloquent.

and I tried both nodes with headphones and verified that messages don’t seem to be coming through (subscriber callback is not being called even when capture node is publishing). This seems out of scope for this PR though.

knorth55 commented 1 year ago

I mean the capture_wave.launch.xml does not have the arguments as the other 2 launch files. So it does not publish to /audio/audio as intended.

I see, I updated.

knorth55 commented 1 year ago

and I tried both nodes with headphones and verified that messages don’t seem to be coming through (subscriber callback is not being called even when capture node is publishing). This seems out of scope for this PR though.

This is weird. It works in my environment, and topic is correctly published. Can you check if the topic is correctly passed by ros2 topic info /audio/audio?

weeshal commented 1 year ago

I’m not currently at my computer, but if it works in your environment then it could be because of my running on eloquent. Either way, I’m not using the audio play node

knorth55 commented 1 year ago

OK, so can you review this PR?

knorth55 commented 1 year ago

@weeshal thank you for your help!

weeshal commented 1 year ago

@knorth55 thanks for the prompt review and collaboration! 😄