-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
L515 FW version compatibility fix #9185
Conversation
…void cross-product versions recognition. This might a temporal solution as eventually FW versions may overlap. Update L515 minimal downgradeable version to 1.5.1.3 instead of 1.4.xx Change-Id: I774c934edd6df2339b610f4d4c59f04fa4fdf69b
|
||
// Limit L515 to FW versions within the 1.5.1.3-1.99.99.99 range to differenciate from the other products | ||
return (firmware_version(fw_version) >= firmware_version(min_max_fw_it->second.first)) && | ||
(firmware_version(fw_version) <= firmware_version(min_max_fw_it->second.second)); |
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.
I think we can use [) for ranges here so it's be <
rather than <=
?
Then you wouldn't have to write numbers like 1.99.99.99
...
I also don't like putting the build number in there. It's got nothing to do with the version number.
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.
The max is required to prevent recognizing D400's 5.12.14.50 as a valid candidate for L515.
Since no formal definition is given 1.99..... is the version that will be currently supported.
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.
I'm saying rather than <= 1.99.99.99
we should say < 2.0
{ L515_PID_PRE_PRQ, "1.4.1.0"}, | ||
{ L515_PID, "1.4.1.0"}, | ||
{ L535_PID, "1.4.1.0"} | ||
static std::map<uint16_t, std::pair<std::string, std::string>> device_to_fw_min_max_version = { |
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.
Why not a pair of firmware_version
... or even char const *
?
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.
We can consider firmware_version
as enhancement separately, I'm not sure whether it improves readability.
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.
Looks OK but see my comments -- up to you.
Change-Id: I8a7a73da4d9714147044977da20af55493db4ac2
Change-Id: I7e74009e57046edcfa59795aa0364c1632d08378
Change-Id: Id0e1f30b945024e0f63a2ac5c2dbd7e831dbf64c
L515 FW version prerequisites must be defined with min/max range to avoid cross-product versions reference.
Can be seen as a temporal solution since FW versions may overlap eventually.
Update L515 minimal downgradeable version to 1.5.1.3 instead of 1.4.1.0
Tracked on: RS5-11513, DSO-16641