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

Fix issue #945 #961

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from
Open

Fix issue #945 #961

wants to merge 3 commits into from

Conversation

Sukhvansh2004
Copy link

@Sukhvansh2004 Sukhvansh2004 commented Jan 14, 2025

Have made services and actions consistent with topics as mentioned in the issue

closes #945

Signed-off-by: Sukhvansh2004 <[email protected]>
@fujitatomoya
Copy link
Collaborator

@Sukhvansh2004 thanks for the PR.

i think you are still working on this, right? please let me know once it is ready to review, and DCO is missing.

Signed-off-by: Sukhvansh2004 <[email protected]>
Signed-off-by: Sukhvansh2004 <[email protected]>
@Sukhvansh2004
Copy link
Author

Hi @fujitatomoya , my PR is complete and is ready for review. Let me know if any more changes are required.

Thanks!

@@ -108,7 +108,7 @@ def send_goal(action_name, action_type, goal_values, feedback_callback):
goal = action_module.Goal()

try:
set_message_fields(goal, goal_dict)
set_message_fields(goal, goal_dict, expand_header_auto=True, expand_time_now=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i do not think this works, actually this does not change anything.

https://github.com/ros2/rosidl_runtime_py/blob/e6c860fec22ffc44556d6d87f730e2d07a8cc62b/rosidl_runtime_py/set_message.py#L42-L48

this method only returns setter functions, those setter functions need to be called to set the values to the field before sending goal request or service request.

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.

expansion of 'auto' and 'now' in actions and services
2 participants