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.

Sign up for GitHub

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

Jump to bottom

[develop/fetch] Add nodes to adjust the volume of the speaking #1746

Merged
merged 2 commits into from
Dec 4, 2022
Merged

[develop/fetch] Add nodes to adjust the volume of the speaking #1746

merged 2 commits into from
Dec 4, 2022

Conversation

Kanazawanaoaki
Copy link
Contributor

develop/fetch version of #1715
This PR adds Apps to adjust the volume of Fetch audible_warning and tweet.

@Kanazawanaoaki Kanazawanaoaki mentioned this pull request Dec 2, 2022
Open
4 tasks
@Kanazawanaoaki Kanazawanaoaki force-pushed the develop/fetch-add-speck-volume-lower branch 2 times, most recently from 7e63d15 to e23baa3 Compare December 2, 2022 05:10
@mqcmd196
Copy link
Member

mqcmd196 commented Dec 2, 2022

Could you push only commits for Fetch? It seems it includes PR2's ones

@Kanazawanaoaki
Copy link
Contributor Author

Understood.
I have removed the changes not related to fetch.

knorth55
knorth55 requested changes Dec 2, 2022
View reviewed changes
jsk_fetch_robot/jsk_fetch_startup/apps/volume_zero/volume_zero.app Outdated
@@ -0,0 +1,6 @@
display: Fetch Volume Zero
description: Fetch speaks zero volume
platform: fetch
Copy link
Member

Choose a reason for hiding this comment

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

we can use platform: all.
with that, we can move the app to jsk_robot_startup and use the same app for fetch, pr2 and other robotcs.

@knorth55
Copy link
Member

knorth55 commented Dec 2, 2022 •
edited
Loading

I also want to make an icon.
You can make an icon from free images like いらすとや.

@tkmtnt7000
Copy link
Member

tkmtnt7000 commented Dec 2, 2022 •
edited
Loading

I'm not sure, but I have a little concern that icon made from いらすとや may have to be changed when merging to master branch because of some policies...

@knorth55
Copy link
Member

knorth55 commented Dec 2, 2022

how about these ?
https://pixabay.com/images/search/mute%20sound/?manual_search=1

@Kanazawanaoaki
Copy link
Contributor Author

Thanks for the review and comments.
As I mentioned in #1715 (comment), I also thought it would be a good idea to put the app in jsk_robot_startup and create icons, so I'll work on it!

@Kanazawanaoaki Kanazawanaoaki force-pushed the develop/fetch-add-speck-volume-lower branch from 847b552 to 7f22892 Compare December 2, 2022 18:08
@Kanazawanaoaki Kanazawanaoaki force-pushed the develop/fetch-add-speck-volume-lower branch from 7f22892 to ef7200f Compare December 3, 2022 09:36
@Kanazawanaoaki
Copy link
Contributor Author

The icon image has been rebased as follows. This PR was rebased to resolve conflicts and tested with fetch15.

Volume lower Volume reset Volume zero
volume_lower volume_reset volume_zero

@Kanazawanaoaki Kanazawanaoaki mentioned this pull request Dec 3, 2022
Open
@mqcmd196 mqcmd196 self-requested a review December 4, 2022 04:09
mqcmd196
mqcmd196 approved these changes Dec 4, 2022
View reviewed changes
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.

LGTM

@knorth55 knorth55 merged commit 722b449 into jsk-ros-pkg:develop/fetch Dec 4, 2022
@Kanazawanaoaki Kanazawanaoaki deleted the develop/fetch-add-speck-volume-lower branch December 4, 2022 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mqcmd196 mqcmd196 mqcmd196 approved these changes

@knorth55 knorth55 knorth55 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Kanazawanaoaki @mqcmd196 @knorth55 @tkmtnt7000