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

Flying lead updates #161

Merged
merged 11 commits into from Nov 6, 2021
Merged

Flying lead updates #161

merged 11 commits into from Nov 6, 2021

Conversation

dtdavi1
Copy link
Contributor

@dtdavi1 dtdavi1 commented Oct 30, 2021

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.

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.
@mabelzhang
Copy link
Contributor

Haven't looked but FYI the failing check is about style.

@dtdavi1
Copy link
Contributor Author

dtdavi1 commented Oct 30, 2021

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).
Copy link
Contributor

@mabelzhang mabelzhang left a 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"/>
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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?

models/dave_object_models/models/plug/model.sdf Outdated Show resolved Hide resolved
Also fixed a bug that kept the joint from being locked correctly.
@M1chaelM
Copy link
Contributor

M1chaelM commented Nov 4, 2021

This is what I see when I run the original demo:
Screenshot from 2021-11-04 15-50-50

And this is what I see when I run the new xacro template version:
Screenshot from 2021-11-04 15-49-57

I'm surprised the seafloor and water seem to be missing. Is this expected?

@M1chaelM
Copy link
Contributor

M1chaelM commented Nov 4, 2021

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.

@mabelzhang
Copy link
Contributor

For what it's worth, I retested by cleaning and recompiling my workspace in my noetic Docker container from dockwater:

catkin clean
catkin build

(with the additional line in HonuRobotics/dockwater#11 for catkin tools. catkin_make clean should do the same)

and the new world still worked for me:

. devel/setup.bash
roslaunch dave_demo_launch dave_plug_and_socket_demo.launch

2021-11-05-010224_3706x2049_scrot

Old one works too:

roslaunch dave_demo_launch dave_electrical_mate.launch 

2021-11-05-010413_3706x2049_scrot

@M1chaelM
Copy link
Contributor

M1chaelM commented Nov 5, 2021

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.

@M1chaelM
Copy link
Contributor

M1chaelM commented Nov 5, 2021

OK, I managed to get the plug into the socket at the top, but just as I seemed about to succeed the simulation crashed:
top_plug_crash

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:

[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug within proximity
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link-plug joint formed
[Msg] socket5_link-plug joint locked!
[Msg] socket5_link-plug joint unlocked!
[Err] [Joint.cc:856] SetVelocityMaximal failed for joint [plug::socket5_link_plug_joint] since a child link was not found.
[Msg] socket5_link-plug joint removed
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug not within proximity, please move the plug closer
[Msg] socket5_link and plug are aligned in orientation and distance
[Msg] socket5_link and plug within proximity
[Wrn] [Model.cc:1595] Model [plug] already has a joint named [socket5_link_plug_joint], skipping creating joint.
gzserver: /usr/include/boost/smart_ptr/shared_ptr.hpp:734: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = gazebo::physics::Joint; typename boost::detail::sp_member_access<T>::type = gazebo::physics::Joint*]: Assertion `px != 0' failed.
Aborted (core dumped)
[gazebo-2] process has died [pid 104716, exit code 134, cmd /opt/ros/noetic/lib/gazebo_ros/gzserver --verbose -e ode /home/mrmccarr@ern.nps.edu/uuv_ws/src/dave/models/dave_worlds/worlds/dave_plug_and_socket_demo.world __name:=gazebo __log:=/home/mrmccarr@ern.nps.edu/.ros/log/3f5edc6e-3e54-11ec-af53-0242ac110002/gazebo-2.log].
log file: /home/mrmccarr@ern.nps.edu/.ros/log/3f5edc6e-3e54-11ec-af53-0242ac110002/gazebo-2*.log
[Wrn] [Publisher.cc:136] Queue limit reached for topic /gazebo/oceans_waves/user_camera/pose, deleting message. This warning is printed only once.

@mabelzhang
Copy link
Contributor

mabelzhang commented Nov 6, 2021

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:
This is the plug on the side wall kind of facing the robot. I moved the vehicle itself via the two sticks, and then RB + B to grab the yellow bar. RB + X to close. Then moved the vehicle to the wall. RB + Left stick Up to rotate elbow to 90 degrees such that the plug is oriented horizontal. Then aimed at the socket.

The first attempt seem to have locked it:

$ roslaunch dave_demo_launch dave_plug_and_socket_demo.launch
...
[Msg] socket1_link and plug are aligned in orientation and distance
[Msg] socket1_link and plug within proximity
[Msg] socket1_link-plug joint formed
[Msg] socket1_link-plug joint locked!

But then I didn't know to open the gripper so I rammed into it and it crashed:

[Msg] plug and finger_tip in contact.
[Msg] plug and finger_tip in contact.
[Msg] plug and finger_tip in contact.
[Msg] plug and finger_tip in contact.
[Msg] socket1_link-plug joint unlocked!
[Err] [Joint.cc:856] SetVelocityMaximal failed for joint [plug::socket1_link_plug_joint] since a child link was not found.
[Msg] socket1_link-plug joint removed
[Msg] socket1_link and plug are aligned in orientation and distance
[Msg] socket1_link and plug not within proximity, please move the plug closer
[Msg] socket1_link and plug are aligned in orientation and distance
[Msg] socket1_link and plug not within proximity, please move the plug closer
[Msg] socket1_link and plug are aligned in orientation and distance
[Msg] socket1_link and plug within proximity
[Msg] socket1_link and plug are aligned in orientation and distance
[Msg] socket1_link and plug within proximity
[Wrn] [Model.cc:1595] Model [plug] already has a joint named [socket1_link_plug_joint], skipping creating joint.
gzserver: /usr/include/boost/smart_ptr/shared_ptr.hpp:734: typename boost::detail::sp_member_access<T>::type boost::shared_ptr<T>::operator->() const [with T = gazebo::physics::Joint; typename boost::detail::sp_member_access<T>::type = gazebo::physics::Joint*]: Assertion `px != 0' failed.
Aborted (core dumped)
[gazebo-2] process has died [pid 116, exit code 134, cmd /opt/ros/noetic/lib/gazebo_ros/gzserver --verbose -e ode /home/master/dave_ws/src/dave/models/dave_worlds/worlds/dave_plug_and_socket_demo.world __name:=gazebo __log:=/home/master/.ros/log/6f3e09f2-3e93-11ec-8ed3-0242ac110002/gazebo-2.log].
log file: /home/master/.ros/log/6f3e09f2-3e93-11ec-8ed3-0242ac110002/gazebo-2*.log

I have a screen recording showing both attempts - so at least they kind of work. It's still processing.

@dtdavi1
Copy link
Contributor Author

dtdavi1 commented Nov 6, 2021

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.

Copy link
Contributor

@mabelzhang mabelzhang left a 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

@bsb808
Copy link
Contributor

bsb808 commented Nov 6, 2021

@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.

@dtdavi1
Copy link
Contributor Author

dtdavi1 commented Nov 6, 2021

@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.
@dtdavi1 dtdavi1 merged commit d090d32 into master Nov 6, 2021
@dtdavi1 dtdavi1 deleted the elec_lead branch November 6, 2021 22:49
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.

None yet

4 participants