Matrix library submoduled twice: in Firmware and in ecl

[MaEtUgR]

During setup of this pr #7908 I ran into unexpected problems.

After some analysis I found out the problem is the interpretation of the effect resulting from the matrix library being a submodule twice in the code base:

Firmware
├─ ecl
│  └─ matrix
└─ matrix

Probably expected: I and according to our discussion also @bkueng expected the calls to the matrix library inside the ecl library to use their instance and hence version of the submodule. And all the calls to the matrix library inside the Firmware but outside the ecl would use the submodule included in the Firmware directly.

Reality: I checked and this is not the case. Istead when compiling the Firmware all the calls go to the submodule included in the Firmware. In fact you can completely delete all the source content of ecl/matrix and it the compilation of the Firmware does not see any effect.

Conclusion: I think this is a problem for the code base structure. I know @priseborough wants to have the ecl self contained but this structure of submodules can easily lead to a lot of confusion.

FYI @LorenzMeier @julianoes @bkueng @dagar @ChristophTobler @Stifael

[dagar]

Even when not using EKF2 the Firmware requires ecl (data validator used by sensors, L1 controller, FW attitude controller). ECL requires the Matrix library. It seems to me Firmware should be getting Matrix through ECL.
@jgoppert would this still be an issue for you, or has Matrix stabilized sufficiently?

Additionally I think the standalone EKF2 build shouldn't be different than the way it's built for Firmware inclusion, but that can be a separate issue.

[PX4BuildBot]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 30 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

[MaEtUgR]

This is still a problem which I cannot fix myself, are there no plans?

[dagar]

We should make firm plans.

[priseborough]

I'm happy to remove ecl/matrix provided we provide a documented way for developers wanting to incorporate ecl into their own projects to create a shared library. See current instructions in README.md

[MaEtUgR]

That sounds like a plan.
About making ecl self contained, I never looked into that myself but @Stifael told me it's in the current state not that easy. He did some tests here I think: PX4/PX4-ECL#355

[MaEtUgR]

Hey @priseborough, is there any progress on this? We just had a new case of confusion resulting from the "double include".

Do you know who are the main clients of the ecl library? We could make very clear documentation how to use ecl by itself or what I would suggest a repo for self contained one-click usage of the latest ecl stable version that includes all necessary dependencies including ecl itself and matrix.

[priseborough]

TU Delft were integrating it into the paparazzi project, but I do not know the current status of that work. Also https://inertialsense.com was using it, but I have not heard from them in over 12 months.

[dagar]

We should be able to drop matrix from the ecl project now and still have a standalone ecl build the pulls the matrix dependency from github if needed.

[MaEtUgR]

@dagar I see you worked on this 🎉
The double trouble module is gone with PX4/PX4-ECL@e5952fa#diff-8903239df476d7401cf9e76af0252622L1 in PX4/PX4-ECL#432 and #9406. Thanks for your efforts!
I hope @priseborough is content and was able to resolve his build system problems reported in the ecl pr. Otherwise we need to follow up on that.
But the original issue for the Firmware is resolved and hence I'm closing.