isri-aist / BaselineWalkingController

Humanoid walking controller with various baseline methods
https://isri-aist.github.io/BaselineWalkingController
BSD 2-Clause "Simplified" License
110 stars 19 forks source link

Doxygen output directory in catkin build #8

Closed mmurooka closed 1 year ago

mmurooka commented 1 year ago

The two packages have DOXYGEN_HTML_OUTPUT set to different paths. I don't understand it very well, but is this correct?

https://github.com/isri-aist/BaselineWalkingController/blob/1f24bf72ef466045e7cd18ed82a5d073c9d46f33/CMakeLists.txt#L24

https://github.com/isri-aist/CentroidalControlCollection/blob/1fffebebb336493911e2fc8a1a1620a18ad62aaf/CMakeLists.txt#L18

gergondet commented 1 year ago

This is made on purpose. jrl-cmakemodules has hard-coded the html output folder and it breaks otherwise :(

mmurooka commented 1 year ago

Sorry, it's not clear yet. In the case of a catkin build, DOXYGEN_HTML_OUTPUT is set to doc/html for BaselineWalkingController and to html for CentroidalControlCollection. My understanding is that both use jrl-cmakemodules in the same way. Can you please explain in detail where this difference comes from?

gergondet commented 1 year ago

Ah sorry, I misread the original issue since the difference is in the catkin build.

I think the difference boils down to the OUTPUT_DIRECTORY in Doxyfile.extra.in. The doc target created by jrl-cmakemodules is created in the PROJECT_SOURCE_DIR but the one you introduce in the doc folder as ${PROJECT_SOURCE_DIR}/doc.

The triggers are also different, the target introduced in the doc folder is always built whereas the target produced by jrl-cmakemodules is triggered at install (which never happens in the catkin builds we run)

Overall I think we fiddled a bit with these settings to get the doc to be uploaded in CI (under the catkin build) and maybe didn't pay much attention to having a consistent solution :(

mmurooka commented 1 year ago

Thanks for the explanation.

I made the change to make it consistent in the commit above. This is a minor detail that could have gone either way, but it was the information necessary for me to set up the new controller repository correctly.