Startup check for librealsense version mismatch

[mjsobrep]

Adding a check to the ros wrapper's startup that ensures that the librealsense version being used is the ideal one should be added. That check should not break anything but should make a very loud (but silenceable through parameters) noise to let the user know.

(from #1187)

[MartyG-RealSense]

@mjsobrep Thanks for your feature request! I have applied the Enhancement label (meaning it is a Feature Request) so that the RealSense developers can track it. :)

[doronhi]

This is not a trivial feature. Suppose after a new librealsense2 version is released, with bug corrections, after releasing a realsense2_camera version. The new librealsense2 is better then the latest available at the time of the wrapper's release but how would it know about it?
On the other hand, sometimes there is a modification in librealsense2 that prevents the wrapper to use it without modifications. How will the wrapper know NOT to recommend that update?
Also, Sometimes a bug is introduced in librealsense2 that makes it better not to upgrade.
I can think of a place on the net where the recommended librealsense2 version is stored. It's an option but requires an internet connection and is another item to manually maintain.
Do you have anything else in mind?

[mjsobrep]

but how would it know about it?

If a new version of librealsense2 comes out, and realsense2_camera supports it without changes then just release a new version of realsense2_camera with the only change being the supported version of librealsense2.

How will the wrapper know NOT to recommend that update?

The wrapper would never recommend any update which it has not been explicitly told (at release time) is supported.

Essentially the wrapper should never provide support to future versions of librealsense2

[MartyG-RealSense]

@mjsobrep Do you wish to continue with this discussion or can it be closed please? Thanks!

[mjsobrep]

@MartyG-RealSense looks like I was the last comment. If your team says this is something that they are not doing, then y'all can close it. Not sure why you are asking me...

[MartyG-RealSense]

The case was created by you, so I wanted to make sure that you were okay with closing it instead of cutting you off.

Reading the comments of @doronhi the RealSense ROS wrapper developer, it seems unlikely that the feature discussed will be implemented, so I will close the case. Thanks so much for your contribution!

[mjsobrep]

@MartyG-RealSense ok, that is a bummer. Stability issues like this make it hard to use realsense. All of the robotics folks I know use realsense for one generation of development, get frustrated, and use something else.

[Achllle]

@doronhi @MartyG-RealSense simply closing this without offering an alternative solution is disappointing. I always manually apt-mark hold anything realsense because updates continuously break and aren't synchronized.
Perhaps the above suggestion is not the best way to do it, but instead clearly an indication of a bigger underlying issue.

[MartyG-RealSense]

Since there is clearly interest from RealSense users in this subject, I think a fair compromise is to re-open the issue and then users can post their ideas here for how a startup check might be implemented, given the considerations that @doronhi outlined in #1189 (comment)

[varunagrawal]

For my $0.02 on the matter, since librealsense follows semantic versioning, it would make sense to have releases deal with checks at only the major and minor levels. For implementing this feature, the idea would be the same as Python's requirements file: you allow certain major release ranges (or just a single major release version) and you get more restrictive in the support depending on the compatibility.

E.g. if 2.35.0 and 2.35.2 are fully supported, then the check would just be a simple VERSION == 2.35. If however 2.36 breaks something, then a new release with the appropriate support and check would be in order. Any release at a particular time should not be making guarantees about future versions and this is standard in software release cycles and underlies the entire concept of semantic versioning.

Now that I think about it, I disagree with @doronhi's comment "sometimes there is a modification in librealsense2 that prevents the wrapper to use it without modifications". The idea of semantic versioning is that API changes require a major version bump and breaking changes require at least a minor version bump. Patch version bumps should be opaque to end users and not affect anything (other than fixing the relevant bug), at least externally.

If the wrapper has a hard time with this, that just means the wrapper has some major design flaws and needs to be re-thought out. This issue is definitely a housekeeping chore - not hard to implement, but not glamorous either.

[doronhi]

realsense2_camera has a version check for librealsense version in it's CMakeLists.txt file. The recommended version is being updated, manually, every realsense2_camera's release.
The last realsense2_camera's release (2.2.14) requires, in the CMakeLists.txt file, librealsense2 of version 2.35.2 and above. According to semantic versioning it could have been 2.35.0 only but it is recommended to upgrade LRS to it's latest version. (It is also acceptable this time, maybe, because there was actually an unintentional change in API in 2.35.2, but that's besides the point).
Do you feel that it should have stayed 2.35.0 and above?
Do you mean that the same version check, being done on runtime, would improve your experience?
@mjsobrep , is that what you meant in your original post?
Or did I misunderstand you completely? If so I would really appreciate some examples of problematic cases that can and should be solved.

[mjsobrep]

@doronhi If I understand, then yes, that check should be done at runtime in addition to build time.

Because the two packages are managed and installed separately, checking at build time does not make any guarantees about the state of the target system at runtime. It would be even better if the underlying synchronization problems did not exist.

Case1:

I install librealsense and realsense-ros, everything is working. I run an apt update & upgrade, expecting that to be safe. librealsense updates, but a realsense-ros update is not available and the entire system breaks.

Ideally that would not be possible. It does happen, regularly. At the least, when I go to debug the system, it would be helpful to be alerted to the source of the error

[doronhi]

Please check out #1278 merged into version 2.2.15 to see how much it meets your request.
It is now merged in version 2.2.15.

[mjsobrep]

Hey @doronhi , that looks great. I haven't tested it, but it seems to do the minimum of what I want.

It would be great if there were a way to prevent that error from ever being thrown. I don't know if there is a way to achieve that. With #1278 we will at least know why errors are cropping up. Thank you

[doronhi]

I'm glad it helps a little.
My thought were that we can throw an exception when there is a mismatch but sometimes users like to experiment with previous versions and I don't want to block that option.
We could add a parameter to control that behavior but all in all, as far as it manageable, I'd like to keep the code as simple as possible.
I'll close the ticket now. Please feel free to update if further corrections will be beneficial to you.

[mjsobrep]

Makes sense. Appreciate your help with this.