ouster-lidar / ouster-ros

Official ROS drivers for Ouster sensors (OS0, OS1, OS2, OSDome)
https://ouster.com
Other
116 stars 139 forks source link

Versioning firmware versions w.r.t. ROS versions #208

Open SteveMacenski opened 3 years ago

SteveMacenski commented 3 years ago

If releasing ROS drivers, we need to select specific ouster_client headers to use. When multiple firmware versions are compatible with the same ouster_client, then we can support a range of firmwares within a same ROS distribution.

However when changes are needing to be made to ouster_client that make existing supported firmware versions within that distribution no longer compatible, then the new changes / firmwares need to be released in an upcoming ROS distribution.

Ex.

ouster_client before november supported all 1.x versions and we released the drivers for Kinetic / Melodic / Foxy. All 1.x's are supported. However since the ouster_client changed and no longer supports 1.x versions, that change can only be made in future ROS distribution releases (e.g. Noetic, Galactic) and those distributions will then only support 2.x+ firmware versions.

So we need to come up with a strategy / a table of what distributions are going to support what version ranges and plan out ouster_client better so that in the future firmware releases don't break older versions, if possible.

dmitrig commented 3 years ago

Right, so if I understand correctly the situation we'd like to avoid is: ROS distributions effectively losing support for Ouster sensors when we start shipping new firmware because the updated client libraries include backwards-incompatible changes (including but not limited to dropping support for old firmware).

This would mean that, if a library we provide gets pulled into a ROS distribution, we would have to provide support for updated firmware in an API/ABI compatible way for at least the lifetime of the distribution (currently 5 years). Failing to do that would place the burden on ROS maintainers, who would have to fork the library and backport support for newer firmware.

My understanding is that, starting with fw 2.0 we will have explicit backwards-compatibility guarantees for the sensor interface, so that shouldn't be an issue. You should be able to pick a release of the client library for inclusion in ROS and the associated maintenance should be limited to bug-fixes. If an old client release stops working with new firmware, IMO that's a regression and we should provide either a firmware fix or (backwards-compatible) client library update.

To be clear, that's just my opinion and not official in any way -- I'll see if I can provide some more information on that front.

dmitrig commented 3 years ago

A related question is if there's something we can do to make updating the current current ros2 driver (included in foxy?) with support for gen2 products easier.

The main issue I see is that it's based on (and exposes all the interfaces of) an old version of this sample code that predates gen2 devices. Back then, we could assume any Ouster device was essentially a 64-beam OS-1, so the APIs (e.g. for parsing packets) don't include a way to specify a product line.

It's not clear to me right now how it would be possible to add gen2 support without something that amounts to creating a v2 namespace, deprecating the gen1-specific APIs, and removing them in the next release. Thoughts?

SteveMacenski commented 3 years ago

ROS distributions effectively losing support for Ouster sensors when we start shipping new firmware because the updated client libraries include backwards-incompatible changes

Exactly. If I have a 1.x firmware with 0.3.x client and all the sudden an apt update comes and now everything's broken because the new 0.5.x client only supports 2.x firmware, you've just nuked someone's entire product for a day or more :laughing:. Once you release a distribution, whatever client code needs to support the matching ranges of firmwares until the distribution goes end of life. You can totally support newer firmwares in the same distribution and add more to the table, but not less. Then between distributions is where you made the breaking changes support or stop supporting certain older firmwares.

Failing to do that would place the burden on ROS maintainers, who would have to fork the library and backport support for newer firmware.

Failing to do would make the drivers unusable and add a ton of frustration to your customers - probably to the point that it wouldn't even make sense for us to try to release these drivers at all. It wouldn't add any burden on the ROS maintainers because there's literally nothing we can do about it. API/ABI is sacred in ROS 2.

A related question is if there's something we can do to make updating the current current ros2 driver (included in foxy?) with support for gen2 products easier.

If there's a client code that supports both 1.x and 2.x, then we can update Foxy and release new binaries of both (though that doesn't seem to be the case from the tickets from customers filed). But since Foxy is already released, that's a done deal, its 1.x supported if they're indeed mutually exclusive.

We can add branches to support 2.x versions so that if they build the code themselves they can use it (foxy_v2.x branch or something) so there's non-released support. We may also be able to release another ROS 2 binary under a different name with V2 support, but that would make things very confusing for everyone other than us if they rely on apt names. But the v2 branch support is totally doable.

dmitrig commented 3 years ago

Here's where I'm still not sure we're quite on the same page:

Once you release a distribution, whatever client code needs to support the matching ranges of firmwares until the distribution goes end of life

and

... it wouldn't even make sense for us to try to release these drivers at all. It wouldn't add any burden on the ROS maintainers because there's literally nothing we can do about it

You're assuming that a ROS package that uses some client code we provide must also expose all of the client APIs directly to ROS users. I'd like to emphasize that it's completely possible to write the ROS code to wrap a client library and only expose, for example, a ROS service to configure the sensor and ROS topics for receiving data (or even just a subset of the C++ API).

The benefit of that would be that, if Ouster decides that its customers only need a major version of the client to be supported for, say, 2 years, it's still possible to have ROS provide a stable API/API for 5 years while using the "official" client library under the hood. Yes, there's more work involved in updating to a new major version every 2 years while preserving the API/ABI of the ROS library, but surely that's still better than writing and maintaining a completely separate codebase (assuming the library and documentation provided reasonably good ;)).

Agree/disagree?

SteveMacenski commented 3 years ago

At the end of the day, that client code is going to be packaged into a binary that is going to be installed on someone's machine with the wrapper. It's not that the API is exposed for users to play with directly (most won't if the ROS drivers work), but that its shipped with the drivers. So if you update the client code which drops support for 1.x and that gets pulled into the driver and released - now everyone's sensors on 1.x on that ROS distribution will fail to work after an apt upgrade.

What I'm saying is that if we release a set of binaries to apt with a client version (0.3.x lets say) that supports all 1.x and no 2.x for Foxy, then if you want to update the client for Foxy, that updated client (0.4.x) must support all 1.x as well as 2.x or it may not be released. Else you're going to destroy a bunch of your customer's systems. But you're always free to update the client to any x.y.z as long as the original range of firmwares 1.x are supported in addition to whatever other version you'd like to support.

Concisely: After binaries for a distribution are shipped, you may add new firmware version support to new client library updates, but you may not remove support for any. To remove support for any, it must be done between ROS distributions so that when that new distribution has binaries released, it supports 2.x (and the cycle turns around if you make a 3.x)