Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd support for defining and playing songs #37
Merged
Conversation
|
Thanks for the PR! |
ca_driver/src/create_driver.cpp
Outdated
| @@ -222,6 +224,16 @@ void CreateDriver::undockCallback(const std_msgs::EmptyConstPtr& msg) | |||
| robot_->setMode(create::MODE_FULL); | |||
| } | |||
|
|
|||
| void CreateDriver::defineSongCallback(const ca_msgs::DefineSongConstPtr& msg) | |||
| { | |||
| robot_->defineSong(msg->song, msg->length, &(msg->notes.front()), &(msg->durations.front())); | |||
This comment has been minimized.
This comment has been minimized.
jacobperron
Mar 27, 2017
Member
This function returns a bool. At the very least we can check the return type and output an error message on failure. Something like ROS_ERROR_STREAM("[CREATE] Failed to define song " << msg->song << " of length " << msg->length);
This comment has been minimized.
This comment has been minimized.
clydemcqueen
Mar 27, 2017
Author
Contributor
Done. It would be nice to add more error checking to libcreate create::Create::defineSong. Perhaps that is a 2nd PR.
ca_driver/src/create_driver.cpp
Outdated
|
|
||
| void CreateDriver::playSongCallback(const ca_msgs::PlaySongConstPtr& msg) | ||
| { | ||
| robot_->playSong(msg->song); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ca_msgs/CMakeLists.txt
Outdated
| @@ -11,6 +11,8 @@ add_message_files( | |||
| Bumper.msg | |||
| ChargingState.msg | |||
| Mode.msg | |||
| PlaySong.msg | |||
| DefineSong.msg | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ca_msgs/msg/DefineSong.msg
Outdated
| @@ -0,0 +1,5 @@ | |||
| Header header # ignored | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ca_msgs/msg/PlaySong.msg
Outdated
| @@ -0,0 +1,2 @@ | |||
| Header header # ignored | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Looks great Finally, could you squash the latest commits and update the README (adding the new topics to the subscribers section). Cheers! |
|
OK! README updated and commits squashed. Thanks. |
6aea09b
into
AutonomyLab:indigo-devel
1 check passed
1 check passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
clydemcqueen commentedMar 27, 2017
Add support for defining and playing songs. Tested on a Create 2.
The Create 2 Open Interface Spec is somewhat confusing: it says that you can create "up to four songs," but then suggests that song numbers can be "0 - 4". In my tests the only valid song numbers are 0, 1, 2 and 3.