Skip to content

rqt_controller_manager: Add more information to details-window #2166

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
8 tasks
christophfroehlich opened this issue Apr 4, 2025 · 12 comments
Open
8 tasks
Assignees

Comments

@christophfroehlich
Copy link
Contributor

Background

Since #2126 we have information about the update rate and async setting in the CLI. Would be great to have that also in the rqt_controller_manager.

Instructions

Hi, this is a good-first-issue issue. This means we've worked to make it more legible to people who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

We're interested in helping you take the first step, and can answer questions and help you out along the way. Note that we're especially interested in contributions from underrepresented groups!

We know that creating a pull request is the biggest barrier for new contributors. This issue is for you 💝

If you have contributed before, consider leaving this PR for someone new, and looking through our general bug issues. Thanks!

🤔 What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

📋 Step by Step

  • 🙋 Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!

  • 🗄️ Create a local workspace for making your changes and testing following these instructions, for Step 3 use "Download Source Code" section with these instructions.

  • 🍴 Fork the repository using the handy button at the top of the repository page and clone it into ~/ws_ros2_control/src/ros-controls/ros2_control, here is a guide that you can follow (You will have to remove or empty the existing ros2_control folder before cloning your own fork)

  • Checkout a new branch using git checkout -b <branch_name>

  • 🤖 Apply pre-commit auto formatting, by running pip3 install pre-commit and running pre-commit install in the ros2_control repo.

  • 💾 Commit and Push your changes

  • 🔀 Start a Pull Request to request to merge your code into master. There are two ways that you can start a pull request:

  1. If you are not familiar with GitHub or how to create a pull request, here is a guide you can follow on how GitHub works.
  • 🏁 Done Ask in comments for a review :)

Is someone else already working on this?

🔗- We encourage contributors to link to the original issue in their pull request so all users can easily see if someone's already started on it.

👥- If someone seems stuck, offer them some help!

🤔❓ Questions?

Don’t hesitate to ask questions or to get help if you feel like you are getting stuck. For example leave a comment below!
Furthermore, you find helpful resources here:

Good luck with your first issue!

@ShravanDeva5327
Copy link

Hey, I'm looking to get started with open source contributions. I saw this "good-first-issue" and it seems like a great opportunity. I'd like to claim it and work on it.

@christophfroehlich
Copy link
Contributor Author

Great!

@ShahazadAbdulla
Copy link

ShahazadAbdulla commented Apr 15, 2025

Hi @ShravanDeva5327 327, I saw you claimed this good first issue #2166 a couple of weeks ago and were assigned. Are you still actively working on it or planning to submit a PR soon?

I'm also new to open source contributions (currently learning ROS 2 Humble through projects) and looking for my first issue to tackle. This one seems like a perfect fit with the excellent instructions provided by @christophfroehlich .

(Big thanks for putting that detailed guide together, @christophfroehlich ! I've seen your helpful comments and issues around while searching, and it's really encouraging for newcomers.)

If you've become busy or decided not to pursue this specific issue anymore, @ShravanDeva5327 327, I would be very interested in taking it on.

@christophfroehlich , if #2166 is still being worked on by ShravanDeva5327, or perhaps in addition, if you know of any other similar beginner-friendly tasks or areas within ros2 (or related projects you maintain) where help might be needed, I'd be very grateful for any pointers! I'm eager to contribute.

Please let me know the status of #2166 when you have a moment. Thanks!

@ShravanDeva5327
Copy link

ShravanDeva5327 commented Apr 15, 2025

Hey @ShahazadAbdulla. I did start looking into this issue but ran into some setup problems on my laptop and couldn’t make much progress. On top of that, I got busy with my coursework and unfortunately didn’t get a chance to update here — really sorry about that!

Please feel free to take over this issue. @christophfroehlich, apologies for the delay and any confusion caused.

@ShahazadAbdulla
Copy link

Thanks for the update @ShravanDeva5327 327, and no worries at all – setup issues and coursework happen! I really appreciate you letting me know.

I'll go ahead and take on this issue now.

@christophfroehlich , could you please assign issue #2166 to me? I'm looking forward to working through the steps you outlined!

@ShahazadAbdulla
Copy link

Hi @christophfroehlich

Following up on the assignment for #2166: I've successfully set up the environment on Humble, built the code, and modified the RQT plugin (popup_info.ui and controller_manager.py) to try and fetch the update_rate and is_async parameters for the selected controller using call_get_parameters.

However, when testing with the standard rrbot demo (ros2_control_demo_example_1) on Humble, I found that the /controller_manager node does not expose per-controller parameters like <controller_name>.update_rate or <controller_name>.is_async. The ros2 param list /controller_manager output only shows a global update_rate parameter and no is_async parameter at all.

This means the RQT plugin cannot fetch the per-controller data as requested on the Humble branch. It also seems to confirm that ShravanDeva327's merged PR #1 (commit ed4ccb0), which addressed this issue previously, likely targeted a different branch (e.g., master) where these parameters are available, and the changes haven't been backported to Humble.

Given this, it seems impossible to complete issue #2166 on the Humble branch as the required data isn't available.

I'm still very eager to contribute! @christophfroehlich, could you perhaps point me towards another "good first issue" in ros2_control that is suitable for the Humble distribution? or can you help me complete this.

Thanks for the guidance through this investigation!

@christophfroehlich
Copy link
Contributor Author

In general, we only accept new features targeting the master branch. Then we triage to do backports to e.g., humble. I suggest that you establish a suitable workflow for you to develop on rolling branch, for example with the provided docker containers in this repository.

@ShahazadAbdulla
Copy link

ShahazadAbdulla commented Apr 19, 2025

Hi @christophfroehlich ,

Thanks for assigning this issue to me! and Thanks for the advice about targeting the master/rolling branch for new features and setting up a suitable workflow. BUT While exploring the code and related PRs (like #2126), I discovered that ShravanDeva5327 actually already submitted and merged a PR (referenced as #1 from his fork context, merged commit ed4ccb0 into master about 3 weeks ago) that implements exactly this feature (adding update rate and async properties to the RQT GUI, using table columns and the popup tree view).

It looks like the work for this issue (#2166) is already completed in the codebase on the master branch!

Could you please clarify on that matter.

Could you perhaps point me towards what I should do about this or if its already done then could please point me to another "good first issue" in ros2_control (maybe one targeting master/rolling) where I could help out?

Thanks again!

@christophfroehlich
Copy link
Contributor Author

christophfroehlich commented Apr 19, 2025

There hasn't been any merge w.r.t. to this issue? https://github.com/ros-controls/ros2_control/commits/master/rqt_controller_manager/rqt_controller_manager
and I don't find any commit in the fork of ShravanDeva5327 https://github.com/ShahazadAbdulla/ros2_control/commits/master/rqt_controller_manager/rqt_controller_manager

Can you clarify what you mean by that?

@ShahazadAbdulla
Copy link

Hi @christophfroehlich,

Ah, thank you so much for the clarification and for checking the history! My apologies for the confusion. I completely misinterpreted the status of a Pull Request I saw on @ShravanDeva5327's fork specifically,
ShravanDeva5327#1.
I mistakenly thought the "Merged" status there meant it had been merged into ros-controls/ros2_control, but I understand now it was likely merged within his own fork's branches.

Thanks for confirming that issue #2166 is indeed still open!

I would definitely still like to work on it. Based on your previous advice, I will:

  1. Set up a ROS 2 Rolling development environment (likely using Docker).
  2. Ensure my local fork is up-to-date with the latest ros-controls/ros2_control master branch.
  3. Create a new feature branch based off master.
  4. Implement the changes to add the update rate and async status to the RQT GUI (fetching the parameters as discussed).
  5. Submit a Pull Request targeting the ros-controls/ros2_control master branch.

Does that sound like the correct plan? Please let me know if there's anything else I should consider. Thanks again for your patience and guidance!

@christophfroehlich
Copy link
Contributor Author

Sry, I also had a look in the wrong fork (in yours, instead of that of @ShravanDeva5327).
This is a good plan. You can use the commit that you have already found as a starting point, test and improve it. (cherry-pick it, then the original author will be retained)

@ShahazadAbdulla
Copy link

Hi @christophfroehlich ,

Ah, thanks a million for clearing that up about the fork vs. upstream merge – totally my mistake reading that PR status! Appreciate you checking.

Got it. Plan is now:

  1. Set up a Rolling/Docker dev environment.
  2. Sync with upstream/master.
  3. Create a new branch.
  4. git cherry-pick @ShravanDeva5327's original commit(s) from his PR (Add update rate and async controller properties to rqt_controller_manager ShravanDeva5327/ros2_control#1) as you suggested – great idea!
  5. Test thoroughly on Rolling, fix/improve if needed.
  6. Submit a new PR targeting master, referencing this issue and crediting the original commit.

Starting on the Rolling setup now. Thanks again for steering me right!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants