Add api version namespaces

[pum1k]

This PR adds inline namespaces to allow choosing API version. By default, the v1 API is set to inline, thus staying API compatible (however, it breaks ABI).

Additionally, source files inside src that were not part of the mrs_lib shared library were moved to examples directory.

[matemat13]

Is there a specific problem this PR is aiming to solve?

Also, please don't mix different changes in a single PR. I don't see a reason not to accept the file rearranging, but the API namespacing is unrelated and will probably need some discussion. Also, changing 99 files in one PR is a lot - especially if it's more than one potential change that needs to be checked in each file.

[pum1k]

Is there a specific problem this PR is aiming to solve?

Yes, this allows additions of backwards incompatible changes without breaking current users. By using a new api version namespace, the two versions will not collide and can be selected by setting the MRS_LIB_API_VERSION macro. This solution chosen as better alternative to introducing another user facing namespace, such as mrs_lib2. I alredy briefly discussed this with @klaxalk, but I'm open to suggestions.

Also, please don't mix different changes in a single PR. I don't see a reason not to accept the file rearranging, but the API namespacing is unrelated and will probably need some discussion.

If you think it would be better to split these, I can open PR with the first commit only and this can be merged after with only the second commit.

Also, changing 99 files in one PR is a lot - especially if it's more than one potential change that needs to be checked in each file.

I don't think the rest can be split in some reasonable way as it is just adding namespace around everything.

[matemat13]

Is there a specific problem this PR is aiming to solve?

Yes, this allows additions of backwards incompatible changes without breaking current users. By using a new api version namespace, the two versions will not collide and can be selected by setting the MRS_LIB_API_VERSION macro. This solution chosen as better alternative to introducing another user facing namespace, such as mrs_lib2. I alredy briefly discussed this with @klaxalk, but I'm open to suggestions.

Honestly I'm not sure if it's not overcomplicating things a bit unnecessarily :D Usually, we're quite open to changing the API even with breaking changes if it's well-justified.

Currently, when you change the API, you are also expected to update all the relevant places that it's being used in the MRS UAV System, the examples, etc., which - although a hassle - is actually a good thing IMO. It forces us to actually adopt and test the changes ASAP and thus also prevents hiding of bugs.

Also, the MRS lib is still primarily used within the MRS UAV System, so I don't think we need to worry too much about breaking stuff. If someone doesn't have the time to update their code with the most recent changes or we find some problem with them, they can always just check-out an older commit until the problem is resolved.

At least this is how we've operated so far. Maybe it is actually time for a change? I'm not 100% convinced TBH. That's why I asked if this is supposed to solve a specific problem or if it's just "a change for the sake of change".

Also, please don't mix different changes in a single PR. I don't see a reason not to accept the file rearranging, but the API namespacing is unrelated and will probably need some discussion.

If you think it would be better to split these, I can open PR with the first commit only and this can be merged after with only the second commit.

Also, changing 99 files in one PR is a lot - especially if it's more than one potential change that needs to be checked in each file.

I don't think the rest can be split in some reasonable way as it is just adding namespace around everything.

I think it'd definitely be worth it to split these (and any similar future) changes into separate PRs. Firstly, it's just good practice not to mix unrelated changes in a single PR. Secondly, splitting it will make checking the different changes much easier. And thirdly, adding unrelated changes (which may be controversial and need further discussion) to unambiguously "good" changes is a bit sketchy - that's how bad laws are passed IRL, after all :D For example, in this case, I have no problem with merging the movement of the examples to a dedicated folder - I think that it is an unambiguously good change. But I have some reservations to the other change in this PR :D

Changing 99 files is OK if it's the same "type" of a change (i.e. moving them all between folders or adding a namespace to all of them). Not if it mixes different changes as in this case.

[pum1k]

Currently, when you change the API, you are also expected to update all the relevant places that it's being used in the MRS UAV System, the examples, etc., which - although a hassle - is actually a good thing IMO. It forces us to actually adopt and test the changes ASAP and thus also prevents hiding of bugs.

This seems on the edge even when doing small changes, as you are probably not able to update all projects. With larger changes, I feel like it's almost impossible.

Also with larger changes, it might be better to roll them out gradually and migrate projects one by one, when the new interfaces are ready. This makes it easier to see if the direction is working and test the individual parts, without committing huge amounts of time trying to migrate everything at once (Which would basically require placing the whole new system into as single PR, because you would need it all at once.).

If someone doesn't have the time to update their code with the most recent changes or we find some problem with them, they can always just check-out an older commit until the problem is resolved.

This sounds like a nightmare... Firstly, you would likely need to build the whole mrs uav system from source, because you wouldn't be able to use the prebuilt binaries, that depend on different version of mrs_lib. Secondly, finding the compatible commits would likely take some effort, because there is no indication of which versions of each system can or cannot go together (or is there?).

If we deprecate the feature instead, we give users time to migrate before these interfaces are removed. Furthermore, the deprecation messages can provide guidance on migrating that specific part of the code.

[matemat13]

Currently, when you change the API, you are also expected to update all the relevant places that it's being used in the MRS UAV System, the examples, etc., which - although a hassle - is actually a good thing IMO. It forces us to actually adopt and test the changes ASAP and thus also prevents hiding of bugs.

This seems on the edge even when doing small changes, as you are probably not able to update all projects. With larger changes, I feel like it's almost impossible.

Also with larger changes, it might be better to roll them out gradually and migrate projects one by one, when the new interfaces are ready. This makes it easier to see if the direction is working and test the individual parts, without committing huge amounts of time trying to migrate everything at once (Which would basically require placing the whole new system into as single PR, because you would need it all at once.).

I mean that's how we've done it so far and it worked :D It's usually not so much work, you just have to refactor a bunch of packages. Also, API-breaking changes are (or at least were) relatively rare. And they often don't touch the whole system. See my recent PRs on the full-quadratic thrust model, which required updating several other packages.

If someone doesn't have the time to update their code with the most recent changes or we find some problem with them, they can always just check-out an older commit until the problem is resolved.

This sounds like a nightmare... Firstly, you would likely need to build the whole mrs uav system from source, because you wouldn't be able to use the prebuilt binaries, that depend on different version of mrs_lib. Secondly, finding the compatible commits would likely take some effort, because there is no indication of which versions of each system can or cannot go together (or is there?).

Fair. It's true that I'm still used to building most of the system locally since I do change stuff in mrs_lib from time to time.

If we deprecate the feature instead, we give users time to migrate before these interfaces are removed. Furthermore, the deprecation messages can provide guidance on migrating that specific part of the code.

Which will result in people never migrating :D We're not a software development company, but a research group... people tend to just ignore deprecation warnings (I'm still mostly working in ROS1, I've gotten used to ignoring deprecation warnings too :D). Actually breaking compilation is often the only way to actually force people to update. And also breaking compilation for others is a good way to force people to update other packages of the system that are affected by any changes they make...

I also have some other issues with this.

Firstly, it's a bit confusing code bloat, further increasing the minimal threshold of SW development knowledge required to use the mrs_lib, which is already not super-low, and some fresh students are already struggling with this a bit.

Secondly, this would basically force us to always define a new API version with each minor breaking change instead of just fixing the few affected packages (again, see the full-quadratic thrust model example), further complicating development, slowing down adoption, and just bloating the lib.

Thirdly, it feels a bit like a change for the sake of a change. It's not really fixing any imminent issue, AFAIK. I'd prefer to obey the KISS rule for mrs_lib whenever possible (related to the previous two issues).

Overall, I see where you're coming from with this and it's not that I think it's a bad idea, but let me reiterate that we're not a SW development company, but a research group (and also educational), so our requirements are a bit different. I'm not 100% opposed to this change, but I'm quite hesitant to accept it. Maybe it'd be best to meet in person and discuss the pros/cons.

[filipopo]

Well we do have "SW development companies" and customers using the mrs_lib and UAV system e.g F4F and Eagle and I don't really think think change is bloat, add one line to everything and you're done but Matouš has a point:

Is this just adding complexity? Personally I'm wondering why is there already a v1 and a v2 added in this commit, why not call everything at this point v1? because I would not like to need to remember what's in which version

There is a solution kinda, to make the versions a "living standard" like HTML5, then we would not need to make a new API version will for every small change but only if there is a "nightmarish" breaking change

I don't know if we ever made such a huge change but if we did, this would definitely be a fix to a problem, in the case of the full-quadratic thrust model PRs, I was replicating it to ROS2 and kinda left it broken for a day then nothing happened 😅

[pum1k]

There is a solution kinda, to make the versions a "living standard" like HTML5, then we would not need to make a new API version will for every small change but only if there is a "nightmarish" breaking change

I agree that it does not make sense for small changes, this is meant to be used for large changes across the whole library.

I don't know if we ever made such a huge change but if we did, this would definitely be a fix to a problem, in the case of the full-quadratic thrust model PRs, I was replicating it to ROS2 and kinda left it broken for a day then nothing happened 😅

This would definitely be overkill for that one.

[filipopo]

This sounds like a nightmare... Firstly, you would likely need to build the whole mrs uav system from source, because you wouldn't be able to use the prebuilt binaries, that depend on different version of mrs_lib

Btw, we are aiming to solve this through Docker by tagging images with dates, so you can always grab a fully working version of the full mrs uav system as it was then