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

Fixed missing callbacks for pending events #420

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

e-hndrks
Copy link
Contributor

@e-hndrks e-hndrks commented Jun 5, 2023

When creating an Entity with a Listener, any instantaneous events that occurred during creation would miss their callback.
This was caused by the C++ API not being able to wrap the underlying C API with a C++ object prior to C already trying to invoking the callback.
That is now resolved by not setting the Listener at creation time, but by setting it after successfully putting a C++ wrapper around the underlying C Entity.
This should fix issue #410
A prerequisite for applying this fix is to make sure pull request #eclipse-cyclonedds/cyclonedds#1717 is applied to the cyclonedds repository.

@e-hndrks e-hndrks force-pushed the master_erik branch 2 times, most recently from fded6fb to 3416c75 Compare June 6, 2023 08:20
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks @e-hndrks !

(I'll wait with merging it until the C part has been merged.)

@eboasson
Copy link
Contributor

eboasson commented Jul 4, 2023

I started a CI run for this PR after merging eclipse-cyclonedds/cyclonedds#1717 and it looks like you missed something in Iceoryx-specific code: https://dev.azure.com/eclipse-cyclonedds/cyclonedds-cxx/_build/results?buildId=4762&view=logs&j=e4f8c82b-de6b-5575-3950-14744af47318&t=aacbc600-1f24-51b1-8fd9-0679ad3e2601

Perhaps you could have another look?

When creating an Entity with a Listener, any instantaneous events that occurred during creation would miss their callback.
This was caused by the C++ API not being able to wrap the underlying C API with a C++ object prior to C already trying to invoking the callback.
That is now resolved by not setting the Listener at creation time, but by setting it after successfully putting a C++ wrapper around the underlying C Entity.
This should fix issue eclipse-cyclonedds#410
A prerequisite for applying this fix is to make sure pull request #eclipse-cyclonedds/cyclonedds#1717 is applied to the cyclonedds repository.

Signed-off-by: Erik Hendriks <erik.hendriks@zettascale.tech>
@e-hndrks
Copy link
Contributor Author

e-hndrks commented Jul 4, 2023

Indeed, I didn't realize that the C++ repository had specialized Shared Memory tests, and therefore didn't run an IceOryx build. However, those remaining issues should now also have been fixed.

Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @e-hndrks !

@eboasson eboasson merged commit 0bdf3a0 into eclipse-cyclonedds:master Jul 4, 2023
20 checks passed
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.

None yet

2 participants