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

Create debugging.rst #1625

Merged
merged 11 commits into from
Jul 21, 2024
Merged

Conversation

firesurfer
Copy link
Contributor

Copy link
Member

@saikishor saikishor 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 documenting this part. I've left some comments to review.

doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Show resolved Hide resolved
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.

Thanks a lot!!

I have only minor cosmetic suggestions (e.g., we use the term hardware component for the plugin, hardware interface is a single "port" and also the class name).

Please consider also the pre-commit and doc-job failures.

And it need to be added to the index.rst file, for example in a new section

=====================================
Guidelines and Best Practices
=====================================

.. toctree::
   :titlesonly:

   debugging.rst

doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
@christophfroehlich christophfroehlich added backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron. labels Jul 19, 2024
@firesurfer
Copy link
Contributor Author

@christophfroehlich I applied your suggestions, added a reference into the index.rst and fixed the pre-commit checks.

@christophfroehlich
Copy link
Contributor

thx. Sphinx is still complaining about something, but i can fix that later

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.

thanks for the changes!

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Some more comments. Thank you guys for refining it very well

doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
doc/debugging.rst Outdated Show resolved Hide resolved
Co-authored-by: Sai Kishor Kothakota <[email protected]>
Copy link
Member

@saikishor saikishor 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! LGTM

@christophfroehlich christophfroehlich merged commit 89292e3 into ros-controls:master Jul 21, 2024
5 checks passed
mergify bot pushed a commit that referenced this pull request Jul 21, 2024
---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
(cherry picked from commit 89292e3)
mergify bot pushed a commit that referenced this pull request Jul 21, 2024
---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
(cherry picked from commit 89292e3)
christophfroehlich pushed a commit that referenced this pull request Jul 21, 2024
---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
(cherry picked from commit 89292e3)

Co-authored-by: Lennart Nachtigall <[email protected]>
christophfroehlich pushed a commit that referenced this pull request Jul 21, 2024
---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
(cherry picked from commit 89292e3)

Co-authored-by: Lennart Nachtigall <[email protected]>
@firesurfer
Copy link
Contributor Author

Thanks for polishing this PR @christophfroehlich :)

christophfroehlich added a commit that referenced this pull request Aug 12, 2024
* [ResourceManager] Make destructor virtual for use in derived classes (#1607)

* Fix typos in test_resource_manager.cpp (#1609)

* [CM] Remove support for the description parameter and use only `robot_description` topic (#1358)

---------

Co-authored-by: Felix Exner <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>

* move critical variables to the private context (#1623)

* Fix controller starting with later load of robot description test (#1624)

* Fix the duplicated restart of the controller_manager services initialization

* Scope the ControllerManagerRunner to avoid malloc and other test issues

* reorder the tests for consistent log at the end

* Remove noqa (#1626)

* Unused header cleanup (#1627)

* Create debugging.rst (#1625)


---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>

* Update changelogs

* 4.14.0

* add missing rclcpp logging include for Humble compatibility build (#1635)

* [CM] Remove deprecated spawner args (#1639)

* Add a pytest launch file to test ros2_control_node (#1636)

* Fix rst markup (#1642)

* Fix rqt_cm paragraph

* Fix indent

* CM: Add missing includes (#1641)

* [RM] Add `get_hardware_info` method to the Hardware Components (#1643)

* Fix the namespaced controller_manager spawner + added tests (#1640)

* Bump version of pre-commit hooks (#1649)

Co-authored-by: christophfroehlich <[email protected]>

* Add missing include for executors (#1653)

* Update changelogs

* 4.15.0

* refactor SwitchParams to fix the incosistencies in the spawner tests (#1638)

* Modify test with missing CM to have a timeout

* Catch exception when CM services are not found

And print the error and exit in the application

* Exit with code 1 on unreached CM

---------

Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Parker Drouillard <[email protected]>
Co-authored-by: Dr. Denis <[email protected]>
Co-authored-by: Christoph Froehlich <[email protected]>
Co-authored-by: Christoph Fröhlich <[email protected]>
Co-authored-by: Henry Moore <[email protected]>
Co-authored-by: Lennart Nachtigall <[email protected]>
Co-authored-by: Sai Kishor Kothakota <[email protected]>
Co-authored-by: Bence Magyar <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: christophfroehlich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label should be used by maintaines only! Label triggers PR backport to ROS2 humble. backport-iron This label should be used by maintaines only! Label triggers PR backport to ROS2 Iron.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants