<Foxy> Discriminate when the Client has gone from when the Client has not completely matched#469
<Foxy> Discriminate when the Client has gone from when the Client has not completely matched#469JLBuenoLopez wants to merge 3 commits intoros2:foxyfrom
Conversation
…mpletely matched (ros2#467) * Workaround when the client is gone before server sends response Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Change add to the map to listener callback Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Apply suggestions from code review Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com> Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> * Uncrustify Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com> Co-authored-by: Michel Hidalgo <michel@ekumenlabs.com>
|
@ros-pull-request-builder retest this please. |
hidmic
left a comment
hidmic
left a comment
There was a problem hiding this comment.
This looks OK to me, but I'll defer to @jacobperron the decision as to whether we can accept an ABI breakage into Foxy or not.
|
@hidmic @jacobperron Please bear in mind that the ABI break is on package |
jacobperron
left a comment
jacobperron
left a comment
There was a problem hiding this comment.
An ABI change is one thing, but this is changing API in a public header. Although it is unlikely that external packages are using the affected type, I'm rather not risk releasing a source-incompatible change into Foxy, especially in a core package that many depend on, albeit transitively.
Perhaps we could modify the change to retain ABI (or at least API) compatibility?
rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_service_info.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
|
#475 reverts the API break in master. |
rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_service_info.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
|
I've discussed this change offline when the ROS 2 team and we would rather not break ABI. I've proposed an alternative PR that attempts to backport this change in an ABI compatible way (see #479). |
|
Backported in #479, maintaining ABI compatibility. |
4 participants
This backports #467 to Foxy.
Note, this breaks ABI compatibility. The first workaround (#412) also had to deal with this problem. The breakage may be acceptable considering the improvement to service discovery (ros2/ros2#1024).