robotology / wearables

Code moved to https://github.com/robotology/human-dynamics-estimation
https://github.com/robotology/human-dynamics-estimation
BSD 3-Clause "New" or "Revised" License
19 stars 11 forks source link

Fix #164 #168

Closed lrapetti closed 1 year ago

lrapetti commented 2 years ago

As suggested by @traversaro in https://github.com/robotology/wearables/issues/164#issuecomment-1247799135 find_package(IWear) should not be called in the IWearFrameVisualizer

traversaro commented 2 years ago

Are you sure this is working? For some reason, the IWear target is referenced in the build directory as Wearable::IWear (see https://github.com/robotology/wearables/blob/0ba0bfc5c3e84afd1ebaa14811aad5d55e8a70a9/interfaces/IWear/CMakeLists.txt#L29), while when it is installed is IWear::IWear (see https://github.com/robotology/wearables/blob/0ba0bfc5c3e84afd1ebaa14811aad5d55e8a70a9/interfaces/IWear/CMakeLists.txt#L63 and https://github.com/robotology/wearables/blob/0ba0bfc5c3e84afd1ebaa14811aad5d55e8a70a9/modules/IWearFrameVisualizer/CMakeLists.txt#L29).

Furthermore, I think you are still not compiling this in CI until we move https://github.com/robotology/wearables/blob/0ba0bfc5c3e84afd1ebaa14811aad5d55e8a70a9/CMakeLists.txt#L97 before add_subdirectory(modules) .

My suggestions:

lrapetti commented 2 years ago

See comments, I would not merge this as it also breakes the only (super-strange) way in which the visualizer is working. @lrapetti if you want I can quickly explain this face2face, as I imagine it is not straightforward to understand how this works even with my comments.

Also @RiccardoGrieco had some concerns about the current cmake structure, we can have a f2f alignment next week.

RiccardoGrieco commented 1 year ago

@traversaro I guess we can close this since we merged #182, right?

traversaro commented 1 year ago

@traversaro I guess we can close this since we merged #182, right?

Ok!

lrapetti commented 1 year ago

The original problem was solved by @traversaro with https://github.com/robotology/wearables/pull/182. Thanks a lot!