ros-industrial / abb_experimental

Experimental packages for ABB manipulators within ROS-Industrial (http://wiki.ros.org/abb_experimental)
Apache License 2.0
131 stars 115 forks source link

ABB IRB 4600_60_205 MoveIt Configuration #99

Closed Andychou007 closed 2 years ago

Andychou007 commented 6 years ago

Add ABB IRB 4600_60_205 MoveIt Configuration package

vlimit commented 4 years ago

Would love to this merged. Can I do anything to help?

gavanderhoorn commented 4 years ago

In general, MoveIt configurations such as the one in this PR are only useful to quickly test out whether things are working with the driver.

Creating a MoveIt configuration is a matter of clicking through a set of screens in the MoveIt Setup Assistant. But the versions here in this repository do not include any information about your work cell, environment or specific robot (ie: limits, dress packs, etc). There is not even a "floor", so MoveIt could just decide to plan "through the floor", and it would be perfectly OK to do that.

As I'm not the main maintainer here, I'm only guessing, but this may be the reason why this PR has not been merged yet.

Having written this, I'll see if it can be merged.

gavanderhoorn commented 4 years ago

Oh, and @vlimit: could I ask you to please not send maintainers/developers private emails?

gonzalocasas commented 3 years ago

sorry to keep barking at the same old tree, but this is a good example of a pattern that -I believe- dissuades people from contributing to these repos.

Besides the very long time between PR open and first active reaction (2 years), claiming that this is not useful because it's generic moveit conf is not a very strong argument. If that were the case, then let's remove all the other generic moveit configs already present in all of the description repositories. On the other hand, a generic configuration IS useful for multiple reasons, first and foremost, it helps beginners to get started and considering the rather steep learning curve from zero to planning, that is a very good thing; but not only that, this assumes that generic configurations are not useful on their own, which is not true. We use MoveIt in containers, without rviz as UI and the creation of the scene elements (floor and what not) is scripted via moveit services, so, at least for our entire institute, a generic configuration has value.

I don't want to be annoying with this, but I really really wish for a lower friction approach to contributions on these repos.

gavanderhoorn commented 3 years ago

claiming that this is not useful because it's generic moveit conf is not a very strong argument. If that were the case, then let's remove all the other generic moveit configs already present in all of the description repositories.

If only we could. But it's not very nice to remove something which has been released, so we don't do that.

On the other hand, a generic configuration IS useful for multiple reasons, first and foremost, it helps beginners to get started and considering the rather steep learning curve from zero to planning, that is a very good thing; but not only that, this assumes that generic configurations are not useful on their own, which is not true. We use MoveIt in containers, without rviz as UI and the creation of the scene elements (floor and what not) is scripted via moveit services, so, at least for our entire institute, a generic configuration has value.

What you describe does not sound like a common usage of MoveIt configurations.

What is more common -- at least in my experience -- is for new users to install/download/git clone a repository with MoveIt configurations, start them, expect everything to work and then wonder why:

  1. trajectories "make no sense" and seem very convoluted for a simple motion
  2. the robot attempts to move "through obstacles" which are "clearly in the workspace"
  3. doesn't take end effectors into account while motion planning/execution trajectories
  4. tries to rip off cables and hosing connected to those EEFs
  5. tries to move outside the joint limits configured on the OEM controller, even though they've updated the _support package with those limits
  6. makes MoveIt print errors and warnings on the console because they've installed a version of MoveIt newer than when the package was generated
  7. doesn't allow the use of planners / planner configurations and/or other plugins MoveIt supports according to the tutorials/documentation
  8. tries to go through complete configuration changes while "only moving 5 cms to the side"

and some more.

I completely agree with you that if you know what you're doing, you can use base MoveIt packages. But if you're just starting out -- and you seem to suggest that should could be the target audience of these packages ("helps beginners to get started") -- there are many problems with trying to reuse such packages. Especially in industrial settings.

That's the main motivation behind not merging in any more MoveIt configurations: for proper usage of MoveIt, you really should create a new configuration.

We use MoveIt in containers, without rviz as UI and the creation of the scene elements (floor and what not) is scripted via moveit services

Right, so that's basically updating the base package with the details on your specific setup, but then programmatically.

Which is what people should be doing, and what is essentially the same as running through the MSA.

gavanderhoorn commented 3 years ago

Don't get me wrong: it'd be great if we could provide a better user experience with basic MoveIt configurations, but no one has spent time coming up with one AFAIK.

And as a maintainer there is another aspect here: MoveIt configuration packages cannot easily be upgraded (to take advantage of new functionality in new MoveIt releases fi), unless you never touch them. That's quite uncommon, as at the very least we'd have to add and update files to add plan execution support. Updating such packages basically comes down to manually diffing things and re-applying changes. Not a very scalable thing to do.

As an example, there are 18 _moveit_config packages in ros-industrial/fanuc alone. The effort to update all of those is significant, as, as I wrote, it has to be done manually.

If we combine that with the fact that for almost all actual use of MoveIt you'd be better off with a freshly generated configuration, it becomes even harder to justify the maintenance overhead such generic configurations incur.

sorry to keep barking at the same old tree, but this is a good example of a pattern that -I believe- dissuades people from contributing to these repos.

there is no need to apologise. I understand your frustration.

The simple truth is there are just very few maintainers, and it's not very rewarding work. That's not an excuse, but an observation.

gavanderhoorn commented 2 years ago

As explained in the previous comments, we're not going to merge this in.

Even though it's really late, I'd still like to thank @Andychou007 for taking the time to submit the PR. That is always appreciated.

(maintenance situation for this repository has been unclear for some time, which also didn't help)