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

Size & CRC in dfu-start; topic-reader stop-from-callback crashes #12789

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Mar 24, 2024

Size and CRC are currently not an input.
FW size is not known in advance.

5e needs to know how much memory to allocate before reading from the dfu topic, so the size needs to be communicated in advance for the allocation.

The CRC was added as well, to aid in validating that the blob was received intact.

  • While doing this, a problem surfaced during testing: quitting DFU from a side thread (as the adapter does, e.g. in case of error) crashed
    • This was because we were destroying the reader from within a callback from the reader
    • Revised dds_topic_reader to use shared_from_this() to keep the object alive; made _stopped a shared_ptr to share ownership
    • Code that relied on the reader dtor stopping the reader needed changing: we can no longer count on the object to be destroyed so must (correctly) specify the intent (stopping) on destruction

I also simplified the DDS tests a bit.

Tracked on [RSDEV-1869]

@maloel maloel requested a review from OhadMeir March 24, 2024 06:53
@maloel maloel changed the title Dfu start Size & CRC in dfu-start; topic-reader stop-from-callback crashes Mar 24, 2024
@maloel maloel requested a review from OhadMeir April 10, 2024 05:41
@maloel maloel merged commit f344f09 into IntelRealSense:development Apr 11, 2024
17 checks passed
@maloel maloel deleted the dfu-start branch April 11, 2024 05:44
maloel added a commit to maloel/librealsense that referenced this pull request Jul 4, 2024
maloel added a commit to maloel/librealsense that referenced this pull request Jul 7, 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.

2 participants