Skip to content

[voicevox] add support for device param#546

Merged
k-okada merged 2 commits intojsk-ros-pkg:masterfrom
sawada10:voicevox-support-for-device-param
Jul 31, 2025
Merged

[voicevox] add support for device param#546
k-okada merged 2 commits intojsk-ros-pkg:masterfrom
sawada10:voicevox-support-for-device-param

Conversation

@sawada10
Copy link
Contributor

@k-okada @iory

I updated the voicevox launch file to allow specifying the speaker device name.
Here is the example.

<launch>
  <include file="$(find voicevox)/launch/voicevox_texttospeech.launch"> 
    <arg name="use_docker" value="true"/>
    <arg name="device" value="respeaker"/>
  </include>
</launch>

Please review.

@k-okada
Copy link
Member

k-okada commented Jul 30, 2025

thank you
Should we include <arg name="device" default="???" doc="???" /> at the top of the file?

@mqcmd196
Copy link
Member

Should we include at the top of the file?

This already exists.

Copy link
Member

@mqcmd196 mqcmd196 left a comment

Choose a reason for hiding this comment

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

Is this param really used?

❯ git rev-parse HEAD
48c605596a8b39c913c2d49c4869476f3422a524
❯ pwd
/home/obinata/ros/review_ws/src/jsk-ros-pkg/jsk_3rdparty/3rdparty/voicevox
❯ git grep device
launch/voicevox_texttospeech.launch:  <arg name="device" default="" />
node_scripts/list_speakers.py:        # check device
node_scripts/list_speakers.py:        for device in await client.http.request("GET", "/supported_devices"):
node_scripts/list_speakers.py:            print("Device: {}".format(device))

@sawada10
Copy link
Contributor Author

The device argument for voicevox_texttospeech.launch is set in the following line,
<arg name="device" default="" />

but I added them because there was no part to read them in the soundplay_node.py.
<param name="device" value="$(arg device)" />

I tried it with USB speakers connected, and without this line, specifying the device as an argument did not work, but by adding it, I was able to specify the device.

Copy link
Member

@mqcmd196 mqcmd196 left a comment

Choose a reason for hiding this comment

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

Ah, I'm super sorry, but I misunderstood something. You specified the param in soundplay_node.py . That's reasonable. LGTM

@mqcmd196
Copy link
Member

@k-okada
Could you merge

and kick the CI again?

@k-okada k-okada merged commit 74c32a7 into jsk-ros-pkg:master Jul 31, 2025
16 checks passed
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