Skip to content

rviz common ros service property#1548

Merged
ahcorde merged 8 commits intoros2:rollingfrom
jsupratman13:feature/rviz_common_ros_service_property
Sep 2, 2025
Merged

rviz common ros service property#1548
ahcorde merged 8 commits intoros2:rollingfrom
jsupratman13:feature/rviz_common_ros_service_property

Conversation

@jsupratman13
Copy link
Contributor

@jsupratman13 jsupratman13 commented Aug 17, 2025

Description

Fixes #1540
Added a new property similar to RosTopicProperty but list services instead.

Is this user-facing behavior change?

Did you use Generative AI?

none

Additional Information

Usage is similar to RosTopicProperty except you will want to set service type instead of message type.

ahcorde
ahcorde requested changes Aug 19, 2025
View reviewed changes
@jsupratman13
Copy link
Contributor Author

jsupratman13 commented Aug 21, 2025 •
edited
Loading

Thanks for the review @ahcorde
I was wondering, if it is better to change target to rolling and then later on backported jazzy or keep the target as jazzy?

@ahcorde
Copy link
Contributor

ahcorde commented Aug 21, 2025

Thanks for the review @ahcorde I was wondering, if it is better to change target to rolling and then later on backported jazzy or keep the target as jazzy?

yes, please. Retarget both PRs to rolling

@jsupratman13 jsupratman13 mentioned this pull request Aug 22, 2025
Merged
@jsupratman13 jsupratman13 changed the base branch from jazzy to rolling August 22, 2025 02:39
@jsupratman13 jsupratman13 force-pushed the feature/rviz_common_ros_service_property branch from c9ea616 to 197920c Compare August 22, 2025 02:42
jsupratman13 and others added 5 commits August 22, 2025 11:48
@jsupratman13
added get_service_names_and_types
f75b8a8
@jsupratman13
added get ros service property
cd48ce9
@jsupratman13 @ahcorde
Update copyright
8f44646 
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Joshua Supratman <supratmanjoshua@gmail.com>
@jsupratman13 @ahcorde
Update copyright
27af0e6 
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Joshua Supratman <supratmanjoshua@gmail.com>
@jsupratman13
added headers
38af523
@jsupratman13 jsupratman13 force-pushed the feature/rviz_common_ros_service_property branch from 197920c to 38af523 Compare August 22, 2025 02:48
@jsupratman13 jsupratman13 marked this pull request as ready for review August 22, 2025 06:42
@jsupratman13 jsupratman13 marked this pull request as draft August 22, 2025 06:43
@jsupratman13
Copy link
Contributor Author

jsupratman13 commented Aug 22, 2025

@ahcorde

I'm getting a CI error, possible due to the changes in #1472.
Is it ok if I put this ready for review? Or should I wait for a PR that fixes the CI build check?

@jsupratman13 jsupratman13 marked this pull request as ready for review August 24, 2025 13:54
@jsupratman13 jsupratman13 requested a review from ahcorde August 24, 2025 13:54
ahcorde
ahcorde requested changes Aug 25, 2025
View reviewed changes
rviz_common/include/rviz_common/properties/ros_service_property.hpp Outdated
void setServiceType(const QString & service_type);

QString getServiceType() const
{return service_type_;}
Copy link
Contributor

@ahcorde ahcorde Aug 25, 2025

Choose a reason for hiding this comment

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

move implementation to cpp, here and below

@jsupratman13 jsupratman13 requested a review from ahcorde August 27, 2025 16:06
@ahcorde
Copy link
Contributor

ahcorde commented Aug 28, 2025

Pulls: #1548
Gist: https://gist.githubusercontent.com/ahcorde/268ee27fc648add11d359b0909235f8c/raw/822cc23888292cc6a44a1d03d44bc087318e9564/ros2.repos
BUILD args: --packages-above-and-dependencies rviz_common
TEST args: --packages-above rviz_common
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16798

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

ahcorde
ahcorde requested changes Aug 29, 2025
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

See this comment #1549 (review)

@jsupratman13 jsupratman13 requested a review from ahcorde August 29, 2025 10:02
@ahcorde
Copy link
Contributor

ahcorde commented Aug 29, 2025 •
edited
Loading

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

@ahcorde
Merge branch 'rolling' into feature/rviz_common_ros_service_property
1a4a373 
Signed-off-by: Alejandro Hernández Cordero <alejandro@openrobotics.org>
@ahcorde ahcorde merged commit 9e3e88a into ros2:rolling Sep 2, 2025
2 checks passed
@jsupratman13
Copy link
Contributor Author

jsupratman13 commented Sep 5, 2025

@ahcorde
Can I backport this feature to jazzy?

@ahcorde
Copy link
Contributor

ahcorde commented Sep 5, 2025

@ahcorde Can I backport this feature to jazzy?

Adding a new virtual method will break ABI, I don't think we can backport this one

@mini-1235 mini-1235 mentioned this pull request Oct 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahcorde ahcorde ahcorde approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ros service version of rviz_common::properties::RosTopicProperty

2 participants

@jsupratman13 @ahcorde