-
Notifications
You must be signed in to change notification settings - Fork 4.9k
fix rs-fw-update wait for device to reconnect #13385
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
tools/fw-update/rs-fw-update.cpp
Outdated
|
|
||
| std::cout << std::endl << "Waiting for device to reconnect..." << std::endl; | ||
| std::unique_lock<std::mutex> lk(mutex); | ||
| new_device = rs2::device(); // otherwise the wait will exit right away |
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.
On which device did you see it?
I am not sure that's the case on all devices/platforms
How is
rs2::device new device;
different from
rs2::device new device = rs2::device() ;
?
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.
new_device is updated every time the callback sees a new device. If the fw-update saw a new device (which is likely, and does happen with D555), then it will be set and the wait will not wait.
I tested with D455 and D555.
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.
It waited for a recovery device:
if (d.is<rs2::update_device>...
IMO your new code now can override the device the callback already set and we will not get a notification of a new device.
Just wait until timeout right?
Maybe DDS should be handled differently?
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 just restarted the device... why does it not make sense to reset the device we're waiting on? Has nothing to do with DDS.
I tested on D455... it works
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 tested on D455... it works
On your system, with current load, but it's not robust.
Add this line and you will see you will wait 15 seconds for timeout.

The original solution does work.
On D500 when it takes longer to enumerate we do wait.
If you wish to reset the 'new_device' you wait on (to solve DDS related issue) try doing it 1 line before the update call which cause a device reset when the update end
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 equating a 3-second wait with a few lines of code while a device is rebooting is stretching it, but ok:
I'll move the reset to above line 530, before we set done = true, ok?
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 before the update?
Is it in use in this function?
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.
Moved before the update and tested.
The last stage when running rs-fw-update is "Waiting for device to reconnect...".
I noticed this stage does not wait at all and exits right away.
Tracked on [RSDEV-2791]