-
Notifications
You must be signed in to change notification settings - Fork 73
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
Flying lead updates #161
Flying lead updates #161
Conversation
See dave_electrical_mating.world for an example. Allowing arbitrary names should make it possible to use the plugin for multiple plug-socket pairs. The plugin still pairs one socket to one plug, but it should be ok to use the same socket or plug with multiple plugin instances.
The plugin is associated with the socket model. Functionality is unchanged, and the plugin utilizes a "plug" model from the SDF world. Conversion will (hopefully) make the plugin easier to use and incorporate its capability into models using Xacro.
Plug and socket plugin updates to clean up the code and to base the alignment on the world pose of the plug and socket links. This should make it easier to add plug/socket pairs to arbitrary models. The platform model is just a box for now, but a better model will be incorporated once a mesh is available.
Updated alignment and joint functionality to work when the socket is mounted to another model and not aligned with the world X axis (previous version was using RelativePose when checking the alignment of the plug and socket). Also, changed the prismatic joint function to allow it to slide more reliably before it is locked.
Haven't looked but FYI the failing check is about style. |
I'll fix the style stuff before completing the merge (assuming the reviews are ok). Mostly blank spaces at the ends of lines. |
Alignment check now compares the pose of the socket and plug links more directly in a single function. Alignment is checked in 3 steps: Orientation: plug & socket link frame orientations match Lateral: plug position is along the socket link X axis Distance: plug is within 0.5 meters of the socket link If all three checks pass, the proximity check (which is unchanged) is used to determine when to create the prismatic joint.
Replaced ROS messaging with Gazebo messaging (there's no other ROS dependency, so it's really just a Gazebo plugin). Changed "verbose" variables to a DEBUG define instead. Changed the demos to set verbose to true when launching Gazebo so that the plugin info messages are printed to stdout (screen).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given my limited knowledge, I can only do a code-level review and nod at pictures, since I don't know enough about the technical / conceptual goals of this demo.
The new Electrical-Plug.dae
and Electrical-Socket.dae
are identical to the ones already existing in models/dave_object_models
. Are they intended to be replicas, or can the existing ones be reused?
New world looks as described. I never got good enough with the joystick to actually drive the robot to plug in the socket - someone better with the joystick should try that.
@@ -15,7 +15,7 @@ | |||
<arg name="gui" value="$(arg gui)"/> | |||
<arg name="headless" value="false"/> | |||
<arg name="debug" value="false"/> | |||
<arg name="verbose" value="false"/> | |||
<arg name="verbose" value="true"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this meant to be only a local change and not committed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's intentional. Since my last commit changed the messaging from ROS to Gazebo (there's no other ROS stuff in the plugin, so it's really a Gazebo plugin, not a ROS one), the messages do not go to the screen unless it's set to verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The duplicated meshes are intentional to avoid cross dependencies between the description package and the models package (e.g., if end users don't want the SDF models, they aren't needed). If the group feels strongly that they shouldn't be duplicated, the URDF (_description) packages can reference the dave_models package versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re duplicated meshes: I don't have a strong opinion. Sometimes duplicates create confusions, especially if someone starts modifying them and isn't aware there are two. I don't know if they will ever need to be modified. @M1chaelM opinions?
Also fixed a bug that kept the joint from being locked correctly.
Confirming the above behavior is not expected. After some troubleshooting with @dtdavi1 it appears that the ocean model from uuvsimulator is not loading on my machine for some reason. It looks like this is not specific to this PR: it's happening for all dave worlds, but I didn't realize it was happening because most worlds also include the local sand heightmap, which seems to come with its own ocean waves. The consequence is that I can't currently test this demo because without the ocean floor the objects fall off the world. Attempting to add the sand heightmap produces a crash (assertion error, not sure why). Our best guess is that my machine may have installed some update recently that broke this part of uuvsimulator. @dtdavi1 is going to try to create a new docker container with an update to check whether he can produce the same behavior. |
For what it's worth, I retested by cleaning and recompiling my workspace in my
(with the additional line in HonuRobotics/dockwater#11 for catkin tools. and the new world still worked for me:
Old one works too:
|
Thanks @mabelzhang I must have messed up the environment somehow. After starting again from scratch it is now working, though my environment is much brighter than what your screenshots show. I will try out the plugs tomorrow. |
OK, I managed to get the plug into the socket at the top, but just as I seemed about to succeed the simulation crashed: Just a guess, but it looks like the error may be related to the fact that I got close and backed out a few times, causing the joint to be created, then removed, then created again. Here is the text output in case that is helpful:
|
Since all my PRs have been opened, I decided to be a responsible adult and play with the joystick to help get this PR in. I was able to produce both a success and a failure case similar to Michael's, during a single run, through no deliberate attempt. Procedure I took: The first attempt seem to have locked it:
But then I didn't know to open the gripper so I rammed into it and it crashed:
I have a screen recording showing both attempts - so at least they kind of work. It's still processing. |
Michael and Mabel stumbled into the same bug (object destruction issue). I think it's fixed, but I can't check it until I get a joystick from work tomorrow. If it's fixed, I'll go ahead and complete the merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll approve to unblock the merge, with the knowledge it's pending that bug fix. Worst case, that bug could be tracked in an issue if we want to go ahead with the release.
Another idea regarding automating the test in the future, is to break up the task into several unit tests that run headless in the automated test suite so that GitHub checks cover them in reasonable time. One test could check the locking mechanism in a trivial known arrangement, where a plug is, say, 5 cm from the socket in the correct orientation, and the robot just advances 5 cm, releases gripper, and checks that it locks. That could be tested for the 5 orientations.
Here's that PR video, sped up 8x. Though I'm only a few light years from a naval aviator, you may want to pick someone else to be on your team in Super Mario.
2021-11-05-22-10_dave_plug_and_socket_demo_8x_450h.mp4
@mabelzhang - nice driving. If you ever give up software, consider life in the oil patch. @dtdavi1 - I'll go ahead with the release and we can do a patch PR when this is ready. No reason for a special trip over the weekend. |
@bsb808 No reason other than the fact that it will annoy me until I close the loop. |
The error was because the joint was being deleted but not "removed" from the model when the plug was detached from the socket. This caused an error on subsequent attempts to plug into the socket. Also did some code cleanup and updated the models so that they are not so bright on most hosts.
This pull request updates the flying lead plugin functionality to the electrical lead plugin as follows (I listed Brian, Mabel, and Michael as reviewers because I know you've all run the original flying lead demo--let me know if you're all cycle-limited and I'll pick someone else):
Changed it from a world plugin to a model plugin associated with the socket (makes it easier to have multiple instances)
Added link names to the plugin definition (defaults are the same as they were)
Fixed some bugs that didn't manifest in the previous version (some link pose errors when the socket was aligned differently or mounted to another model and some prismatic joint errors).
Developed URDF (Xacro) templates and examples.
The changes should be transparent when the original demo is run:
roslaunch dave_demo_launch dave_electrical_mate.launch
The demo using the Xacro templates can be launched with:
roslaunch dave_demo_launch dave_plug_and_socket_demo.launch
Once it launches, you'll see the same Rexrov UUV and plug model near a simple rectangular stand with 5 sockets mounted on it (one one each side and one on top). You should be able to plug into any socket except the one on top (which may or may not work--I haven't fully tested with it yet).
There are a couple of things I know of that need to be fixed long term (there are a couple of TODO comments in the C++ file) and probably a couple of things that others will come up with as well. I may or may not have enough cycles to do some of them in time for the release, but I wanted to get the 90% solution out there now.