Skip to content

Add offered qos incompatible event and event executor test #11

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.

Sign up for GitHub

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

Jump to bottom
Merged
ivanpauno merged 6 commits into from
Aug 20, 2020
Merged

Add offered qos incompatible event and event executor test #11

ivanpauno merged 6 commits into from
Aug 20, 2020

Conversation

@ivanpauno
Copy link
Collaborator

@ivanpauno ivanpauno commented Aug 19, 2020 •
edited
Loading

(edit) #10 has already been merged.

This PR is build on top of #10.

It adds a new publisher event status: OfferedQosIncompatible.
That new event status is used to test the executor code in #10, which surprisingly works without changes.

There are some other cosmetic changes here.

I will rebase and redirect the PR to feature/events when #10 gets merged.

@ivanpauno ivanpauno added the enhancement New feature or request label Aug 19, 2020
@ivanpauno ivanpauno requested a review from jacobperron August 19, 2020 16:14
@ivanpauno ivanpauno self-assigned this Aug 19, 2020
jacobperron
jacobperron reviewed Aug 19, 2020
View reviewed changes
@jacobperron
Copy link

jacobperron commented Aug 19, 2020

surprisingly works without changes.

😂

jacobperron
jacobperron approved these changes Aug 19, 2020
View reviewed changes
Copy link

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Besides the minor comments above, this LGTM

@ivanpauno
Add OfferedQosIncompatible publisher event status
14505eb 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Add OfferedQosIncompatible publisher event status
ca698ed 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Remove unnecesary print
d7ad984 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno force-pushed the ivanpauno/add-offered-qos-incompatible-event branch from 9951769 to d7ad984 Compare August 20, 2020 13:06
@ivanpauno ivanpauno changed the base branch from ivanpauno/events-executor-support to feature/events August 20, 2020 13:06
@ivanpauno ivanpauno marked this pull request as ready for review August 20, 2020 13:08
@ivanpauno
Copy link
Collaborator Author

ivanpauno commented Aug 20, 2020

I have rebased feature/events against foxy-devel, and this branch against feature/events.
In that way, we can see CI being exercised now 😃 .

@ivanpauno
Fix string comparisons
ce1ba9f 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Compatibility with foxy
1fbeb17 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Copy link
Collaborator Author

ivanpauno commented Aug 20, 2020 •
edited
Loading

@jacobperron there are some event statuses that we want but are not available in foxy (e.g. message lost), should we create a galactic-devel branch?

(edit) that should be done after completing basic event handling support in the foxy branch

@ivanpauno
Copy link
Collaborator Author

ivanpauno commented Aug 20, 2020

Unrelated, but I want to get rid of all stale branches.
Do we need to keep some of them or can I delete them all?

@ivanpauno
Foxy compatibility, take 2
18c020f 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno merged commit f36782a into feature/events Aug 20, 2020
@ivanpauno ivanpauno deleted the ivanpauno/add-offered-qos-incompatible-event branch August 20, 2020 15:48
This was referenced Aug 20, 2020
Merged
Merged
@jacobperron
Copy link

jacobperron commented Aug 24, 2020

@ivanpauno

there are some event statuses that we want but are not available in foxy (e.g. message lost), should we create a galactic-devel branch?

Yeah, feel free to create a galatic-devel branch when the time is right.

Unrelated, but I want to get rid of all stale branches.

Go ahead and delete stale branches 👍

@ivanpauno ivanpauno mentioned this pull request Aug 28, 2020
Merged
ivanpauno added a commit that referenced this pull request Aug 31, 2020
@ivanpauno
Add offered qos incompatible event and event executor test (#11)
81a6342 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
ivanpauno added a commit that referenced this pull request May 17, 2021
@ivanpauno
Add offered qos incompatible event and event executor test (#11)
cc972c7 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
ivanpauno added a commit to ros2-java/ros2_java that referenced this pull request Jan 14, 2022
@ivanpauno
Add offered qos incompatible event and event executor test (osrf#11)
e80c5df 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno mentioned this pull request Jan 14, 2022
Merged
ivanpauno added a commit to ros2-java/ros2_java that referenced this pull request Jan 25, 2022
@ivanpauno
Add offered qos incompatible event and event executor test (osrf#11)
0565917 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jacobperron jacobperron jacobperron approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ivanpauno ivanpauno

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ivanpauno @jacobperron