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.

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

New ball shooter plugin for the "Scan and Dock and Deliver" task #311

Merged
merged 7 commits into from
Aug 11, 2021

Conversation

caguero
Copy link
Contributor

@caguero caguero commented Aug 10, 2021

See issue #280.

How to test it:

Terminal 1:

roslaunch vrx_gazebo vrx.launch

Terminal 2:

rostopic pub /wamv/shooters/ball_shooter/fire std_msgs/Empty "{}" --once

You should observe how the blue ball is fired.
ball_shooter

Feel free to play with the plugin parameters in ball_shooter.xacro for testing.

Note about the ball shooter model: For now, the model is just a box. This will be replaced in the future when a nicer model is ready. It shouldn't be any functional difference in any case.

caguero and others added 5 commits August 5, 2021 22:24
Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: Carlos Agüero <[email protected]>
@M1chaelM
Copy link
Collaborator

The gazebo 7 test is failing. Since this is very old now, I've changed the merge requirements to make it optional and added the gazebo 11 test to the list of required checks. I'm also updating the test images just to make sure they are correct.

@M1chaelM M1chaelM self-requested a review August 11, 2021 18:43
Copy link
Collaborator

@M1chaelM M1chaelM left a comment

Choose a reason for hiding this comment

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

I think this looks great! Thanks for putting it together. I added one suggestion about putting the shot_force explicitly in the ball_shooter.xacro, if you agree that could make things easier for users.

A couple other notes:

  • I tested varying the shot force and number of shots. These seemed to be working as expected.
  • I see what you mean about re-using the same ball. I think this is fine. I guess it's possible that teams will want to use a strategy of shooting a bunch of shots in rapid succession if they are running out of time, and this might be OK to allow if we decide to limit the number of shots (say, to 3 or whatever would reasonably fit). I think we can adjust this later if needed though.

///
/// * Optional parameters:
/// <num_shots> - Number of shots allowed. Default to UINT_MAX.
/// <shot_force> - Force (N) applied to the projectile. Default to 250 N.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about including this tag explicitly in the example and also the .xacro file we are providing ? Out of all the parameters, it seems like the one that users are most likely to want to adjust. Even if it is redundantly set to 250 it might make it easier to quickly pick up how to use this plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Added in 07609f6.

<frame>wamv/ball_shooter_link</frame>
<pose>0.2 0 0 0 0 0</pose>
</projectile>
<topic>${namespace}/${shooter_namespace}${name}/fire</topic>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is where I was thinking we could change to something like:

       <shot_force>250</shot_force>
       <topic>${namespace}/${shooter_namespace}${name}/fire</topic>

@M1chaelM
Copy link
Collaborator

One more note: after updating the test images this is still not building on Gazebo 7. I'm not sure whether we should try to fix this or not. Thoughts?

@caguero
Copy link
Contributor Author

caguero commented Aug 11, 2021

One more note: after updating the test images this is still not building on Gazebo 7. I'm not sure whether we should try to fix this or not. Thoughts?

I needed to add a few ifdefs to the code (462cdc9). Not very elegant but it should work. Waiting for CI...

@caguero caguero merged commit ae1823d into master Aug 11, 2021
@caguero caguero deleted the ball_shooter branch August 26, 2021 15:08
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.

2 participants