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

Take all available samples on service/client on_data_available. #616

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

MiguelCompany
Copy link
Collaborator

This should fix several issues related with services.

@fujitatomoya
Copy link
Collaborator

Just a reference: eProsima/Fast-DDS#2760

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@fujitatomoya
Copy link
Collaborator

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@fujitatomoya
Copy link
Collaborator

fujitatomoya commented Jun 24, 2022

CI windows:

  • Windows Build Status

@aditya2592
Copy link

Will this be backported to galactic?

@clalancette
Copy link
Contributor

Will this be backported to galactic?

It certainly can be; it doesn't change API or ABI. Once we have some confirmation that this passes CI and we merge it into Rollng, we can consider backports to Humble and Galactic.

@aditya2592
Copy link

aditya2592 commented Jun 25, 2022

Thanks. Is it also possible to know which service issues get fixed by this update? Just so that we can plan to report/reproduce the remaining? I went through the linked issue but it's a lot of detail that I am not familiar with. The two issues that we face currently :

  1. Service response loss when there is on server and several clients that are making requests (possibly at the same time)
  2. Service servers not discovered during startup

All services and clients are on the same machine and we are using discovery server.

@MiguelCompany
Copy link
Collaborator Author

@clalancette @fujitatomoya I don't think the warnings on the Windows CI are related to this PR. Do you think it can be merged?

@clalancette
Copy link
Contributor

@clalancette @fujitatomoya I don't think the warnings on the Windows CI are related to this PR. Do you think it can be merged?

I agree that the warnings are not related to this PR. In that case, we can go ahead and merge this.

@clalancette clalancette merged commit e9abdc4 into ros2:master Jun 27, 2022
@clalancette
Copy link
Contributor

@Mergifyio backport humble galactic foxy

mergify bot pushed a commit that referenced this pull request Jun 27, 2022
mergify bot pushed a commit that referenced this pull request Jun 27, 2022
mergify bot pushed a commit that referenced this pull request Jun 27, 2022
Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit e9abdc4)

# Conflicts:
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_client_info.hpp
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_service_info.hpp
@mergify
Copy link

mergify bot commented Jun 27, 2022

backport humble galactic foxy

✅ Backports have been created

jsan-rt pushed a commit to eProsima/rmw_fastrtps that referenced this pull request Jul 12, 2022
jsan-rt added a commit to eProsima/rmw_fastrtps that referenced this pull request Jul 13, 2022
* Take all available samples on service/client on_data_available. (ros2#616)

Signed-off-by: Miguel Company <[email protected]>

* Initial commit

Signed-off-by: Ricardo González Moreno <[email protected]>

* Working subscriptions and services

Signed-off-by: Ricardo González Moreno <[email protected]>

* Add event support

Signed-off-by: Ricardo González Moreno <[email protected]>

* Initial new event callback

Signed-off-by: Ricardo González Moreno <[email protected]>

* Use guard_condition with event listeners

Signed-off-by: Ricardo González Moreno <[email protected]>

* Remove unused functions

Signed-off-by: Ricardo González Moreno <[email protected]>

* Fixing tests

Signed-off-by: Ricardo González Moreno <[email protected]>

* Fixing uncrustify

Signed-off-by: Ricardo González Moreno <[email protected]>

* Simplify SubListener's get_unread_messages()

Signed-off-by: Javier Santiago <[email protected]>

* Simplified get_unread_requests() and get_unread_responses()

Signed-off-by: Javier Santiago <[email protected]>

* Moved Waitset creation/destruction outside loop as suggested

Signed-off-by: Javier Santiago <[email protected]>

* Renamed variable. Removed unneded checks. Replaced get_unread_count with get_first_untaken_info

Signed-off-by: Javier Santiago <[email protected]>

* Modified oneliners.

Signed-off-by: Javier Santiago <[email protected]>

* Cleaned more unneeded checks

Signed-off-by: Javier Santiago <[email protected]>

* Added RCPPUTILS_TSA_GUARDED_BY macros to previously atomic booleans

Signed-off-by: Javier Santiago <[email protected]>

* Fixed wrong mutex guard. Renamed and removed break; from TERMINATE_THREAD

Signed-off-by: Javier Santiago <[email protected]>

* Fix waiting events

Signed-off-by: Ricardo González Moreno <[email protected]>

* Fixing crash

Signed-off-by: Ricardo González Moreno <[email protected]>

* Removed unneeded include. Restored some cleanup code. Added some comments.

Signed-off-by: Javier Santiago <[email protected]>

* Set nullptr after delete

Signed-off-by: Javier Santiago <[email protected]>

* Detach listener

Signed-off-by: Javier Santiago <[email protected]>

Co-authored-by: Miguel Company <[email protected]>
Co-authored-by: Ricardo González Moreno <[email protected]>
aditya2592 pushed a commit to SBPL-Cruz/rmw_fastrtps that referenced this pull request Nov 1, 2022
mauropasse pushed a commit to mauropasse/rmw_fastrtps that referenced this pull request Jan 19, 2023
alsora added a commit to irobot-ros/rmw_fastrtps that referenced this pull request Jan 19, 2023
Take all available samples on service/client on_data_available. (ros2#616)
quarkytale pushed a commit that referenced this pull request May 17, 2023
…port #616) (#623)

* Take all available samples on service/client on_data_available. (#616)

Signed-off-by: Miguel Company <[email protected]>
(cherry picked from commit e9abdc4)

# Conflicts:
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_client_info.hpp
#	rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_service_info.hpp

* resolve merge conflicts.

Signed-off-by: Tomoya Fujita <[email protected]>

---------

Signed-off-by: Tomoya Fujita <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
Co-authored-by: Tomoya Fujita <[email protected]>
@MiguelCompany MiguelCompany deleted the bugfix/services-data-available branch June 12, 2024 17:26
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.

4 participants