Feature ROS2 services and actions#9
Merged
Conversation
juanlofer-eprosima
force-pushed
the
feature/publish-data
branch
2 times, most recently
from
June 4, 2025 10:03
43290e5 to
a757991
Compare
EugenioCollado
force-pushed
the
feature/services_actions
branch
2 times, most recently
from
June 4, 2025 11:45
3d52ff1 to
4859367
Compare
juanlofer-eprosima
force-pushed
the
feature/publish-data
branch
3 times, most recently
from
June 4, 2025 13:01
f465d47 to
4588279
Compare
EugenioCollado
force-pushed
the
feature/services_actions
branch
4 times, most recently
from
June 6, 2025 08:29
005e3ff to
5378d3d
Compare
juanlofer-eprosima
force-pushed
the
feature/publish-data
branch
from
June 6, 2025 10:23
915cb76 to
79d9c19
Compare
Contributor
Author
|
Review notes:
|
juanlofer-eprosima
force-pushed
the
feature/publish-data
branch
from
June 12, 2025 10:34
aabc30a to
71da6f2
Compare
EugenioCollado
force-pushed
the
feature/services_actions
branch
from
June 23, 2025 08:29
5378d3d to
252c4b7
Compare
EugenioCollado
force-pushed
the
feature/services_actions
branch
from
July 3, 2025 10:03
aa359c5 to
5444cfc
Compare
EugenioCollado
requested review from
richiprosima
and removed request for
richiprosima
July 4, 2025 07:32
EugenioCollado
force-pushed
the
feature/services_actions
branch
3 times, most recently
from
July 4, 2025 11:46
866da48 to
3327f12
Compare
juanlofer-eprosima
requested changes
Jul 18, 2025
juanlofer-eprosima
left a comment
juanlofer-eprosima
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the great work! New API and logic look sound to me, most of things I'm not happy about require first refactoring the RPC module in DDS-Pipe.
|
|
||
| //! Mutex synchronizing access to object's data structures | ||
| std::mutex mtx_; | ||
| std::recursive_mutex mtx_; |
Contributor
There was a problem hiding this comment.
Try to refactor the code so recursiveness is not required.
Contributor
There was a problem hiding this comment.
Will double check if have the time.
EugenioCollado
force-pushed
the
feature/services_actions
branch
from
July 22, 2025 07:21
f58fa96 to
63ba9fe
Compare
EugenioCollado
requested review from
richiprosima
and removed request for
richiprosima
August 26, 2025 11:37
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
juanlofer-eprosima
requested changes
Oct 28, 2025
| volumes: | ||
| - ./config.yml:/config/config.yml | ||
| - ../persistence:/persistence | ||
| command: ./build/ddsenabler/examples/service/ddsenabler_example_service server --persistence-path /persistence --requests-path /persistence/requests --config /config/config.yml --expected-requests 3 |
Contributor
There was a problem hiding this comment.
Open suggestion to review when refactoring the DDS-Pipe: it should be possible to announce a service even if types are not present in the persistence database, since they are received from the discovered client.
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
juanlofer-eprosima
requested changes
Nov 5, 2025
Comment on lines
+109
to
+112
| try | ||
| { | ||
| rpc_info = std::make_shared<RpcInfo>(dds_topic.m_topic_name); | ||
| } |
Contributor
There was a problem hiding this comment.
Leaving this open for reference in case RPC structs is refactored in the future.
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
Signed-off-by: Eugenio Collado <eugeniocollado@eprosima.com>
juanlofer-eprosima
approved these changes
Nov 6, 2025
juanlofer-eprosima
left a comment
juanlofer-eprosima
left a comment
Contributor
There was a problem hiding this comment.
LGTM, pending suggestions to be tackled in a future PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is the rebase of all commits from here squashed on top of this branch.