loco-3d / crocoddyl

Crocoddyl is an optimal control library for robot control under contact sequence. Its solver is based on various efficient Differential Dynamic Programming (DDP)-like algorithms
BSD 3-Clause "New" or "Revised" License
858 stars 174 forks source link

Fix unittest factory for future compatibility #1112

Closed ManifoldFR closed 1 year ago

ManifoldFR commented 1 year ago

The main change is as follows:

unittest : use GeometryObject's CollisionGeometryPtr typedef (4f6f3e4aa42fd1c93a19ccd06100bac60cf62d12)

Which makes the factory code independent on whether installed release of Pinocchio (or hpp-fcl) uses std or boost shared_ptr.

wxmerkt commented 1 year ago

CI is failing. Does this need a bump in the Pinocchio version?

/root/target_ws/src/crocoddyl/unittest/factory/cost.cpp:215:63: error: no matching constructor for initialization of 'pinocchio::GeometryObject'
    pinocchio::GeomIndex ig_frame = geometry->addGeometryObject(pinocchio::GeometryObject(
                                                                ^
  /opt/ros/noetic/include/pinocchio/multibody/fcl.hpp:181:3: note: candidate constructor not viable: no known conversion from 'pinocchio::SE3' (aka 'SE3Tpl<double, 0>') to 'const pinocchio::GeometryObject::CollisionGeometryPtr' (aka 'const shared_ptr<fcl::CollisionGeometry>') for 4th argument
    GeometryObject(const std::string & name,
    ^
  /opt/ros/noetic/include/pinocchio/multibody/fcl.hpp:222:3: note: candidate constructor not viable: no known conversion from 'pinocchio::FrameTpl::JointIndex' (aka 'unsigned long') to 'const pinocchio::GeometryObject::CollisionGeometryPtr' (aka 'const shared_ptr<fcl::CollisionGeometry>') for 3rd argument
    GeometryObject(const std::string & name,
ManifoldFR commented 1 year ago

CI is failing. Does this need a bump in the Pinocchio version?

/root/target_ws/src/crocoddyl/unittest/factory/cost.cpp:215:63: error: no matching constructor for initialization of 'pinocchio::GeometryObject'
    pinocchio::GeomIndex ig_frame = geometry->addGeometryObject(pinocchio::GeometryObject(
                                                                ^
  /opt/ros/noetic/include/pinocchio/multibody/fcl.hpp:181:3: note: candidate constructor not viable: no known conversion from 'pinocchio::SE3' (aka 'SE3Tpl<double, 0>') to 'const pinocchio::GeometryObject::CollisionGeometryPtr' (aka 'const shared_ptr<fcl::CollisionGeometry>') for 4th argument
    GeometryObject(const std::string & name,
    ^
  /opt/ros/noetic/include/pinocchio/multibody/fcl.hpp:222:3: note: candidate constructor not viable: no known conversion from 'pinocchio::FrameTpl::JointIndex' (aka 'unsigned long') to 'const pinocchio::GeometryObject::CollisionGeometryPtr' (aka 'const shared_ptr<fcl::CollisionGeometry>') for 3rd argument
    GeometryObject(const std::string & name,

Hm, it seems like the version of Pinocchio in the CI was older than I thought. This is due to a switch in the order of arguments of the ctor (the old-order ctor will be deprecated). I'll change it back to old order.

Edit: fixed as of a22add8

ManifoldFR commented 1 year ago

Last thing is fixing the formatting. I remember that @cmastalli linked me to a clang-format command on the wiki which mentioned clang-format 6.0 or something. Applying that command (I'm on clang-15) changed the formatting of all the files on my end which is strange.

Which style format are we supposed to use here ? Google, llvm ? I'll update the wiki if needed

cmastalli commented 1 year ago

Last thing is fixing the formatting. I remember that @cmastalli linked me to a clang-format command on the wiki which mentioned clang-format 6.0 or something. Applying that command (I'm on clang-15) changed the formatting of all the files on my end which is strange.

Which style format are we supposed to use here ? Google, llvm ? I'll update the wiki if needed

It shouldn't change all the documents. There is clearly something wrong.

You might need to install clang-6, but not entirely sure. Have you put the configuration file as described in the wiki? If so, have you done this before running the clang command?

Note that if you put the config file after running for the first time, then clang doesn't consider anymore this configuration for some weird reason. When this happens, I clone again the repo.

cmastalli commented 1 year ago

Thanks, @ManifoldFR!!