Upstream changes from iit-danieli-joint-lab/abb_libegm fork

[gavanderhoorn]

@traversaro et al.: your fork appears to have some interesting changes that may be good to upstream.

Things that caught my eye:

• windows build fixes (I don't believe we currently target Windows too much, but it'd be good to try and be nice to other users)
• s/Boost/std/g
• Protobuf related changes/fixes (although we need to maintain bw-compatibility with CMake 3.6 for now)
• changes to better support cross-compilation

and a few others.

Would you be interested in submitting some of that as PRs here?

[traversaro]

Hi @gavanderhoorn , we are definitely interested in providing the fix upstream, especially because that would reduce the maintenance effort for us in the long term.

windows build fixes (I don't believe we currently target Windows too much, but it'd be good to try and be nice to other users)

Most of those are just earlier versions of #63 , so I think there is not a lot to port.

s/Boost/std/g

This is probably the most important and under some aspects non-trivial change. As soon as it is clear that C++98 compatibility can be dropped, I would be happy to provide the PRs to migrate away from boost. Note that if you want to avoid Boost completely we need at least do depend on standalone Asio and this may be desirable or not depending on the use case.
In theory to avoid depending on boost::math we used Eigen, but that can probably be avoided as the only operation used is actually the equation conjugate, that is trivial to implement.

Relevant commits (pay attention as we have also subsequent bug fixes not squashed):

• boost --> std iit-danieli-joint-lab@efa97d7, iit-danieli-joint-lab@61a1542 and iit-danieli-joint-lab@4f65f22
• boost --> Asio standalone iit-danieli-joint-lab@6fb23cb, iit-danieli-joint-lab@ca0bc0f and iit-danieli-joint-lab@4280e66
• boost --> Eigen iit-danieli-joint-lab@7d350da

Protobuf related changes/fixes (although we need to maintain bw-compatibility with CMake 3.6 for now)

Yes, there are some strange issues on the location of protobuf generated files with recent protobuf and CMAke versions, probably setting up a CI job that consumes dependencies from vcpkg is the best way to highlight those issues (see iit-danieli-joint-lab@f80b8b1).

changes to better support cross-compilation

Those are a bit tricky, because you need to define how to handle the code generation with an external protoc, and unfortunately probably this requires changes in the FindProtobuf.cmake shipped with CMake, or with the ProtobufConfig.cmake shipped with Protobuf itself, and especially protobuf upstream is simply ignoring issues regarding non-standard platforms (see for example protocolbuffers/protobuf#4198 ). The alternative that we used is to incude the generated files directly in the source, but unfortunatly this only works for a given version of protobuf. Related ign-msgs issue: https://bitbucket.org/ignitionrobotics/ign-msgs/issues/34/add-support-for-cross-compilation .

[gavanderhoorn]

s/Boost/std/g

This is probably the most important and under some aspects non-trivial change. As soon as it is clear that C++98 compatibility can be dropped [..]

We're currently targetting Kinetic and Melodic, mostly, here.

Kinetic allows to set the minimum level of C++ to c++11.

We have no (and don't state any) requirement to be c++98 compatible afaik.

@jontje ?

[jontje]

We have no (and don't state any) requirement to be c++98 compatible afaik.

@jontje ?

There is no such requirement.

s/Boost/std/g

This is probably the most important and under some aspects non-trivial change. As soon as it is clear that C++98 compatibility can be dropped, I would be happy to provide the PRs to migrate away from boost.

I would be glad to see those changes; I have had that on my agenda for years, but no time for it 😄 @traversaro can you provide link(s) to the changes? I just want to skim through them.

[traversaro]

@traversaro can you provide link(s) to the changes? I just want to skim through them.

There are links to the relevant commit in the issue #70 (comment) , if you tell us what are the changes you are first interested in we can prepare clean PRs/patches .

[gavanderhoorn]

@traversaro: the Windows build related changes may be easiest / most stand-alone?

[traversaro]

@traversaro: the Windows build related changes may be easiest / most stand-alone?

As far as I remember the Windows-related changes are due to the use of a newer protobuf (3.11) installed by vcpkg, see iit-danieli-joint-lab#2 . Interestingly, currently Debian sid and Ubuntu 20.04 still ship protobuf 3.6, so even on 20.04 you will not experience the issue. I would be happy to start tackling those one, but without having a CI that covers the Windows compilation while installing dependencies with vcpkg it would be quite easy to have regression. I would be happy to add a new GitHub Actions based CI for covering the vcpkg case, but that is yes another thing to maintain so let me know if you are interested in it or not.

[jontje]

There are links to the relevant commit in the issue #70 (comment) , if you tell us what are the changes you are first interested in we can prepare clean PRs/patches .

Regarding the code changes, then I think it would be reasonable to do it in this order:

1. Remove the boost::math dependency by implementing the quaternion conjugate operation, which should be straight-forward.
2. Replace as much as possible of the boost components with std components (i.e. the non-asio components).
3. Finally consider replacing boost::Asio with standalone Asio.

[gavanderhoorn]

@traversaro: would what @jontje suggests be acceptable?

[traversaro]

@traversaro: would what @jontje suggests be acceptable?

Totally! Recent events here in Italy kind of changed our workplan, but I think we can start with a PR removing the boost::math depending relatively soon.

[gavanderhoorn]

Of course, there is no hurry.

Some things are more important.

[traversaro]

First related PR: #91 .

[traversaro]

Second related PR: #102 .

[traversaro]

Third PR: #103 .