Skip to content

Conversation

@drindr
Copy link

@drindr drindr commented Dec 5, 2025

Thanks for your nice crate.

Finding that to access the service or topic in the ActionClient, a mutable reference of ActionClient is required, I think it is totally unnecessary. It is almost impossible to modify them in other crate because most of the related methods are pub(crate) even private. Removing the &mut can make external design easier as well.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the API ergonomics of ActionClient by removing unnecessary mutable reference requirements from accessor methods. The change is safe because the underlying Client and Subscription types use interior mutability patterns (e.g., AtomicI64), allowing their operations through immutable references.

Key changes:

  • Modified five accessor methods (goal_client, cancel_client, result_client, feedback_subscription, status_subscription) to take &self instead of &mut self and return immutable references
  • Updated example code to use immutable binding for rotate_action_client

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/action/client.rs Changed five accessor methods from &mut self to &self, returning immutable references instead of mutable ones
examples/turtle_teleop/main.rs Changed rotate_action_client binding from let mut to let to match the updated API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jhelovuo jhelovuo self-assigned this Dec 8, 2025
@jhelovuo jhelovuo added the enhancement New feature or request label Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants