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
Contributor

This should fix several issues related with services.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@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
Contributor 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
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit e9abdc4)
mergify bot pushed a commit that referenced this pull request Jun 27, 2022
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit e9abdc4)
mergify bot pushed a commit that referenced this pull request Jun 27, 2022
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
(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
…#616)

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
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 <MiguelCompany@eprosima.com>

* Initial commit

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Working subscriptions and services

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Add event support

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Initial new event callback

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Use guard_condition with event listeners

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Remove unused functions

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Fixing tests

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Fixing uncrustify

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Simplify SubListener's get_unread_messages()

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Simplified get_unread_requests() and get_unread_responses()

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Moved Waitset creation/destruction outside loop as suggested

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

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

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Modified oneliners.

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Cleaned more unneeded checks

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Added RCPPUTILS_TSA_GUARDED_BY macros to previously atomic booleans

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

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

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Fix waiting events

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

* Fixing crash

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

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

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Set nullptr after delete

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

* Detach listener

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Ricardo González Moreno <ricardo@richiware.dev>
aditya2592 pushed a commit to SBPL-Cruz/rmw_fastrtps that referenced this pull request Nov 1, 2022
…#616)

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
mauropasse pushed a commit to mauropasse/rmw_fastrtps that referenced this pull request Jan 19, 2023
…#616)

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
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 <MiguelCompany@eprosima.com>
(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 <Tomoya.Fujita@sony.com>

---------

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@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.

None yet

4 participants