Skip to content

Update FW version display after FW update - #13316

Merged
Nir-Az merged 6 commits into
realsenseai:developmentfrom
noacoohen:update_fw_version_display_after_fw_update
Sep 11, 2024
Merged

Update FW version display after FW update#13316
Nir-Az merged 6 commits into
realsenseai:developmentfrom
noacoohen:update_fw_version_display_after_fw_update

Conversation

@noacoohen

Copy link
Copy Markdown
Contributor

Tracked by LRS-686

@noacoohen
noacoohen requested a review from Nir-Az September 3, 2024 15:06
Comment thread common/fw-update-helper.cpp Outdated
@Nir-Az
Nir-Az requested a review from maloel September 3, 2024 16:24
@Nir-Az

Nir-Az commented Sep 3, 2024

Copy link
Copy Markdown
Collaborator

@maloel can you help reviewing this PR?
Thanks

@maloel

maloel commented Sep 4, 2024

Copy link
Copy Markdown
Contributor

This PR will work as long as the user holds the context from which the device was created.

E.g.:

ctx = context()
ctx.set_devices_changed_callback( on_devices_changed )
device = context().query_devices()[0]
device.hardware_reset()

This will not call on_devices_changed because it's on another context!!!

This may be OK: if we can agree that, for the D457 notifications to work, the user has to keep holding the context, then the PR is acceptable.

To make it so that the notification is sent to ALL contexts that may be listening, more extensive changes must be made: we need to take the invocation up to the level of the device-watcher and likely add an API to device_info.

@Nir-Az please see if the above is acceptable or whether we want to make more extensive changes.

Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-factory.cpp Outdated
@Nir-Az

Nir-Az commented Sep 4, 2024

Copy link
Copy Markdown
Collaborator

This PR will work as long as the user holds the context from which the device was created.

E.g.:

ctx = context()
ctx.set_devices_changed_callback( on_devices_changed )
device = context().query_devices()[0]
device.hardware_reset()

This will not call on_devices_changed because it's on another context!!!

This may be OK: if we can agree that, for the D457 notifications to work, the user has to keep holding the context, then the PR is acceptable.

To make it so that the notification is sent to ALL contexts that may be listening, more extensive changes must be made: we need to take the invocation up to the level of the device-watcher and likely add an API to device_info.

@Nir-Az please see if the above is acceptable or whether we want to make more extensive changes.

I'm OK with that,
It's much better than what we have today, and this is an internal enhancement so for now it's better to avoid massive changes IMO

Comment thread src/ds/d400/d400-factory.cpp Outdated
Comment thread src/ds/d400/d400-factory.cpp
@maloel

maloel commented Sep 10, 2024

Copy link
Copy Markdown
Contributor

Notice there's an error in the CI

@Nir-Az
Nir-Az merged commit 05777c5 into realsenseai:development Sep 11, 2024
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.

3 participants