Regenerated Ikfast Plugin

[Jmeyer1292]

As the title suggests, I reran the IKFast generation module for the 2400 and have tested it with the existing demo.launch file and with Descartes in the godel project. It appears to work as expected.

[gavanderhoorn]

I suspect the plugin was regenerated with the latest version of moveit_ikfast (seeing as the SEARCH_MODE enum is introduced in this PR). Is that correct?

If that is the case, this PR should be considered as not only a bug fix, but as a potentially behaviour changing (and bw-compat breaking) upgrade of the plugin. Nothing wrong with that, but I thought I'd at least point it out.

[Jmeyer1292]

That is correct.

[Jmeyer1292]

I amended the commit message to include more details so the change logs will indicate the upgrade. Is there anything else I can do to make this more acceptable?

[gavanderhoorn]

Tell us how you've tested it? :)

In all seriousness: my conservative-me would rather you'd regenerate the plugin using the version of moveit_ikfast that was used to create the current version. If we intend for this to be a bug-fix only, that is really the only thing we should do. Professionally I'd say that closing a bug type ticket with an enhancement PR is really a no-no.

However, if we feel that is too much of a hassle (considering the estimated usage of this particular pkg, our commitment to bw compatibility and semantic versioning, etc), then this PR is as acceptable as any. As long as you've sufficiently tested it, we could probably just go ahead and merge it.

[gavanderhoorn]

Also, btw, the fix should probably also be backported to the Hydro package, as this is really a critical bugfix.

[Levi-Armstrong]

@Jmeyer1292, are you able to regenerate the plugin using the version of moveit_ikfast that was used to create the current version?

[Jmeyer1292]

@gavanderhoorn As I said in my first post, I tested by using it as the IK plugin for the moveit demo (by dragging the end effector around), and by using it in the Godel project to generate reasonable looking paths for blending. :)

I will of course be happy to regenerate the plugin with whatever version you guys want: moveit/moveit_ikfast@7ca7125 is the commit corresponding to the first Moveit_ikfast version prior to the date that the current irb2400 module was last modified. Is this one okay? I will wait for your approval on the version.

[gavanderhoorn]

@Jmeyer1292: I think moveit/moveit@a2360256 is also ok. That would include some fixes to the generated pkg manifests (although the abb pkgs should already include those).

Basically any point in the repos history up-to the 10th of June (3a61010b) is fine. That should result the diff showing only changes to the actual FK/IK code, not the rest of the logic.

[Jmeyer1292]

@gavanderhoorn @Levi-Armstrong Have a look. I have tested this by dragging it around using the MoveIt demo and it appears to work (in contrast to the original which turned the end effector by 90 degrees).

[Jmeyer1292]

Also there's an abb_moveit_plugins package in the abb directory that I haven't touched. Does that need to be updated too?

[gavanderhoorn]

This looks a lot better, no changes other than to the FK/IK code.

re: abb_moveit_plugins: hm, that pkg should actually have been removed as part of the Indigo migration. @Levi-Armstrong: did you miss that? It's also deprecated.

abb_moveit_plugins should receive the fix in hydro-devel though.

[Levi-Armstrong]

Yea, I missed it. Once this gets merged I will remove the deprecated package for indigo and update it for hydro.

@gavanderhoorn Is this ready to merge?

[gavanderhoorn]

Afaict, +1 from me.