-
Notifications
You must be signed in to change notification settings - Fork 5k
rsutils::version, replacing librealsense::firmware_version & sw_update::version #11191
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
Conversation
| test.check( v2.is_between( v1, v3 )) | ||
| test.check( v3.is_between( v1, v3 )) | ||
| test.check( v2.is_between( v2, v3 )) | ||
| test.check( v2.is_between( v2, v2 )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to add a state when is_between() failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I just took your code from the .cpp :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh :)
| test.check( not version( 0x100, 0xFFFF, 0xFF, 0xFFFFFFFF )) | ||
| test.check( not version( 0xFF, 0x10000, 0xFF, 0xFFFFFFFF )) | ||
| test.check( not version( 0xFF, 0xFFFF, 0x100, 0xFFFFFFFF )) | ||
| #test.check( not version( 0xFF, 0xFFFF, 0xFF, 0x100000000 )) Python throws - type error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checks the limits
| { | ||
| unsigned major = 0; | ||
| char const * ptr = base; | ||
| while( *ptr != '.' ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add protection from nullptr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Killjoy...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
Nir-Az
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
versionto pyrsutilsrspy.repo.compare_fw_versions()and.pretty_fw_version()rsutils/string/test-sw-update-ver.cpptest in favor ofrsutils/test-version.pyNote:
A small side-effect of this is that FW versions that were previously expressed as
05.14.00.00will now be5.14.0(the last number is omitted when 0). I've tested a bit (including FW downgrade and upgrade, with rs-fw-update and viewer) and found no issue...