-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Versioning firmware versions w.r.t. ROS versions #208
Comments
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. |
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 |
Exactly. If I have a 1.x firmware with 0.3.x client and all the sudden an
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.
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 ( |
Here's where I'm still not sure we're quite on the same page:
and
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? |
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 What I'm saying is that if we release a set of binaries to 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 |
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.The text was updated successfully, but these errors were encountered: