Skip to content

Conversation

@ShahazadAbdulla
Copy link

@ShahazadAbdulla ShahazadAbdulla commented Sep 2, 2025

Fixes #2166

This PR adds the update_rate and is_async parameters to the RQT controller manager details popup window.

Implementation Details:

  • Modified popup_info.ui to add new QLabel widgets for displaying the information.
  • Updated the _on_ctrl_info function in controller_manager.py to fetch the parameters using ros2param.api.call_get_parameters.
  • The logic first attempts to retrieve per-controller parameters (<controller_name>.update_rate, etc.).
  • If a per-controller value is not found, it gracefully falls back to the global update_rate or a sensible default (False for is_async).

Testing:

  • The feature was developed and tested in a ROS 2 Rolling Docker environment.
  • When running the rrbot demo from ros2_control_demos, the popup correctly displays the global and default values as shown below.

Result:

screenshot-20250902_135959 screenshot-20250902_140045

Adds update_rate and is_async parameters to the RQT controller manager details popup. Fetches per-controller parameters and falls back to global/default values if not available. Fixes ros-controls#2166.
@bmagyar
Copy link
Member

bmagyar commented Sep 3, 2025

@ShahazadAbdulla could you please reattempt uploading the screenshot?

@codecov
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.60%. Comparing base (f53552b) to head (62dbf33).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2510   +/-   ##
=======================================
  Coverage   89.60%   89.60%           
=======================================
  Files         152      152           
  Lines       17637    17637           
  Branches     1448     1448           
=======================================
  Hits        15804    15804           
  Misses       1250     1250           
  Partials      583      583           
Flag Coverage Δ
unittests 89.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ShahazadAbdulla
Copy link
Author

ShahazadAbdulla commented Sep 3, 2025

@ShahazadAbdulla could you please reattempt uploading the screenshot?

ofc. @bmagyar

screenshot-20250902_135959 screenshot-20250902_140045

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

I tested it, and it works for controllers, but: The same layout is used for controllers and hardware components, but you only query the parameters from the controllers. If you later open the details of a hardware component, the data is wrong because not updated/deleted any more.

Please consider a consistent code style (comments), and remove LLM output

@github-actions
Copy link
Contributor

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale label Oct 24, 2025
@ShahazadAbdulla
Copy link
Author

Hi @christophfroehlich 

My sincere apologies for the long delay. After a lot of troubleshooting with the build environment, I have now finalized the changes.

The logic to prevent stale data from being displayed has been implemented as requested. I have also applied the suggested comment changes and have run pre-commit to align the code with the project's style guide.

Thank you again for your patience and for the helpful review.

@github-actions github-actions bot removed the stale label Oct 27, 2025
@ShahazadAbdulla
Copy link
Author

ShahazadAbdulla commented Oct 28, 2025

Hi @christophfroehlich ,

I was looking into the failing "Kilted" check to see if there was anything I needed to fix. It seems the failure is happening in the test_spawner_unspawner test.

Based on the job logs, the root cause appears to be an issue within the test setup itself, related to service timeouts and spawner scripts being called with missing arguments. This seems to be an existing issue in the master branch that was revealed when you merged it into my branch.

This appears to be completely unrelated to my RQT changes, but I wanted to share my findings in case it's helpful for tracking down the problem. Please let me know if there's anything else you need from my side for this PR.

Also what should I do next on this.

Thank you!

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Don't worry about this failing test, this is not related. But you haven't addressed my concern regarding update of the hardware components, please do so.

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.

rqt_controller_manager: Add more information to details-window

3 participants