remove robot_model:: and robot_state::

[v4hn]

Fixes #1878 .

Please merge this as two commits, as we should only backport the internal usage, but not the API change.

[rhaschke]

Removing robot_model and robot_state requires changes in downstream packages like moveit_visual_tools as well.

[simonschmeisser]

Shouldn't there be a deprecation warning for some time before "removing" API? There are many places using robot_state::RobotState in ROS-I for example

[v4hn]

It's funny how @davetcoleman was the one complaining about the aliased namespaces, when his own projects use them. 😄

The problem with API deprecations in master is that it's not possible to bind the deprecation to a ROS release anymore, because OR wants us to abandon ROS.
In that context we agreed to allow more API-breaking changes in master directly.

Given the amount of usage of these namespaces, I switched to deprecation.
This will probably still fail with some ROS-I projects though, because, I seem to recall, they compile with -Werror(?).

[davetcoleman]

It's funny how @davetcoleman was the one complaining about the aliased namespaces, when his own projects use them. smile

What projects specifically? I'm sure at one point I took some snippets from other places that used that type of namespace, yes. So what?

[rhaschke]

Travis needs to be fixed before merging - although we have a lot of approvals already!

[v4hn]

What projects specifically

I noticed moveit_visual_tools with quite a few offenders.

But among other things the tutorials also use them a lot.

This syntax breaks Travis.

True. It turns out namespace aliases (namespace a = b) do not support attributes at all, not even in the standard. Looks like an oversight to me, but might be related to simplified processing. Aliases are usually treated as straight text replacement.
A simple rewrite is namespace [[deprecated]] a { using namespace b; } which is allowed in the C++ standard.
However, because the gcc developers are as slow to fix issues as we are, gcc did not support this syntax until August 2019 (talking about c++11...). The patch did not even land in gcc 9.3, so it's still broken in Ubuntu 20.04.
Even the old-fashioned way of specifying gnu-specific deprecations (__attribute__((deprecated))) was not supported before.
Either way triggers

warning: 'deprecated' attribute directive ignored [-Wattributes]

and does not recognize the deprecation.

Clang just does the right thing.

That simply leaves us with no way of deprecating the namespaces without removing them.
Because the syntax is so common, let's just leave the namespace definitions around.
@henningkayser make sure you remove them for moveit2 though!

I added a decent hint to MIGRATION.md asking people to change the namespaces for future versions.

[henningkayser]

@v4hn can't we just deprecate the corresponding types using aliases (like b735145)

[v4hn]

Is the commit you cite actually accepted syntax by our CI? You didn't get CI to succeed yet. After all it's also an alias declaration, even if for a class.

@v4hn can't we just deprecate the corresponding types using aliases

We did similar things in the past for `MoveGroup` -> `MoveGroupInterface`, but in this case it would mean deprecating **every single class** in `moveit::core` for two namespaces. That's fragile *and* a bulk of explicit definitions nobody will ever look at. To me, that approach plus the inconvenience for users is not worth it to me to remove this hidden alias declaration...

[codecov]

Codecov Report

Merging #2135 into master will decrease coverage by 0.40%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2135      +/-   ##
==========================================
- Coverage   57.82%   57.41%   -0.41%     
==========================================
  Files         328      328              
  Lines       25700    25700              
==========================================
- Hits        14860    14755     -105     
- Misses      10840    10945     +105     

Impacted Files	Coverage Δ
...ls/include/moveit/py_bindings_tools/gil_releaser.h	0.00% <0.00%> (-100.00%)	⬇️
.../include/moveit/py_bindings_tools/py_conversions.h	0.00% <0.00%> (-86.67%)	⬇️
...s/include/moveit/py_bindings_tools/serialize_msg.h	0.00% <0.00%> (-80.00%)	⬇️
.../ompl_interface/src/detail/constrained_sampler.cpp	46.15% <0.00%> (-17.95%)	⬇️
...ove_group_interface/src/wrap_python_move_group.cpp	31.96% <0.00%> (-11.08%)	⬇️
...ma_kinematics_plugin/src/lma_kinematics_plugin.cpp	75.18% <0.00%> (-3.76%)	⬇️
.../move_group_interface/src/move_group_interface.cpp	44.44% <0.00%> (-3.64%)	⬇️
...ipulation/pick_place/src/manipulation_pipeline.cpp	76.41% <0.00%> (-0.95%)	⬇️
...raint_samplers/src/default_constraint_samplers.cpp	82.90% <0.00%> (+0.36%)	⬆️
...meterization/work_space/pose_model_state_space.cpp	82.99% <0.00%> (+0.68%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bdce08...ae29ea6. Read the comment docs.

[davetcoleman]

I see you already did the tutorials. Here's MVT: moveit/moveit_visual_tools#65