Skip to content
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

Merged
merged 4 commits into from
Jun 10, 2021

Conversation

ev-mp
Copy link
Collaborator

@ev-mp ev-mp commented Jun 8, 2021

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

…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
@ev-mp ev-mp requested a review from maloel June 8, 2021 23:20
@ev-mp ev-mp changed the title L515 FW version comatibility fix L515 FW version compatibility fix Jun 8, 2021

// 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));
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

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 = {
Copy link
Collaborator

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 *?

Copy link
Collaborator Author

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.

maloel
maloel previously approved these changes Jun 9, 2021
Copy link
Collaborator

@maloel maloel left a 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.

@maloel maloel dismissed their stale review June 9, 2021 05:18

LibCI fails

ev-mp added 3 commits June 9, 2021 12:38
Change-Id: I8a7a73da4d9714147044977da20af55493db4ac2
Change-Id: I7e74009e57046edcfa59795aa0364c1632d08378
Change-Id: Id0e1f30b945024e0f63a2ac5c2dbd7e831dbf64c
@ev-mp ev-mp merged commit 553649b into IntelRealSense:development Jun 10, 2021
@ev-mp ev-mp deleted the l515_fw_protection branch June 10, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants