Skip to content

[kilted] Fix incorrect action client feedback callback type hint#1654

Closed
laser wants to merge 1 commit into
ros2:kiltedfrom
laser:backport-1616-kilted
Closed

[kilted] Fix incorrect action client feedback callback type hint#1654
laser wants to merge 1 commit into
ros2:kiltedfrom
laser:backport-1616-kilted

Conversation

@laser
Copy link
Copy Markdown

@laser laser commented Apr 20, 2026

What Problem Are We Trying to Solve?

  • ActionClient.send_goal_async() declares feedback_callback as Callable[[FeedbackT], None], but the runtime dispatcher in the same file invokes the registered callback with a FeedbackMessage[FeedbackT] (the wrapper with goal_id and feedback fields), not a bare FeedbackT.
  • Consumers typing their callback correctly according to the declared signature cannot reach the feedback payload. The type hint is simply wrong.
  • This was fixed on rolling by Fix incorrect action client/server callback type hints #1616 (commit f735d59) but was never backported to kilted. kilted users still see the incorrect hint.

How Did We Solve the Problem?

  • Backport the client-side portion of Fix incorrect action client/server callback type hints #1616 to kilted.
  • Introduce a FeedbackCallbackUnion TypeAlias in the TYPE_CHECKING block, move SendGoalKWargs into the same block as Generic[FeedbackT], and use the new alias in _feedback_callbacks, send_goal, and send_goal_async.
  • Update test_action_client.py to type feedback_callback with FeedbackMessage[Fibonacci.Feedback], documenting the corrected signature.
  • Server-side changes from Fix incorrect action client/server callback type hints #1616 are not included: they depend on the ImplT generic parameter introduced after kilted.
  • Pure type-hint correction; no runtime behavior change.

…hint

The ActionClient.send_goal_async() feedback_callback parameter is
declared as Callable[[FeedbackT], None] but the runtime dispatcher
invokes the registered callback with a FeedbackMessage[FeedbackT]
(the wrapper carrying goal_id and feedback fields), not a bare
FeedbackT.  Consumers who type their callback correctly according
to the declared signature cannot reach the feedback payload.

This is a client-side-only backport of upstream commit f735d59
(PR ros2#1616) to kilted.  The server-side changes in that PR depend
on the ImplT generic parameter introduced after kilted, so they
are excluded here.

Introduces a FeedbackCallbackUnion type alias in the TYPE_CHECKING
block and threads it through SendGoalKWargs, the internal
_feedback_callbacks dict, and send_goal_async's signature.  No
runtime behavior changes; this is a pure type-hint correction.

Signed-off-by: Laser <l@s3r.me>
@laser laser force-pushed the backport-1616-kilted branch from 4a28811 to 1c8ac8f Compare April 20, 2026 22:18
@bjsowa
Copy link
Copy Markdown
Contributor

bjsowa commented Apr 22, 2026

@laser Since I was planning to do this either way, I pushed the complete backport of #1616 to #1655 so I think this can be closed.

@laser
Copy link
Copy Markdown
Author

laser commented Apr 22, 2026

@laser Since I was planning to do this either way, I pushed the complete backport of #1616 to #1655 so I think this can be closed.

Sounds good. My dreams of landing a commit in rclpy have been dashed, alas, but I'm pleased with the outcome nonetheless :)

@laser laser closed this Apr 22, 2026
@bjsowa
Copy link
Copy Markdown
Contributor

bjsowa commented Apr 22, 2026

@laser I added you as a co-author since you technically helped me by reminding to do this.

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.

2 participants