Refactor logging #543
Refactor logging #543
Conversation
christophfroehlich
force-pushed
the
use_logger_var
branch
from
e8b0a76 to
9026c32
Compare
|
This pull request is in conflict. Could you fix it @christophfroehlich? |
christophfroehlich
force-pushed
the
use_logger_var
branch
from
9026c32 to
cc7dd51
Compare
This reverts commit 10f2e22.
|
I reverted the changes to the update rates as suggested here |
|
This pull request is in conflict. Could you fix it @christophfroehlich? |
github-actions
bot
requested review from
aprotyas,
bmagyar,
destogl,
livanov93,
mamueluth,
olivier-stasse and
progtologist
There was a problem hiding this comment.
As written in some examples: There currently seems to be an issue with INT vs DOUBLE datatypes with the test_forward_position_controller.launch.py launches in all examples, as a setpoint of [0, 0] should be [0.0, 0.0]. Should we take this to a new issue instead of fixing it here?
There was a problem hiding this comment.
Is example 3 missing a doc update? Also: the example output there is only valid when the user switches to the forward_position_controller either by providing it as a launch argument or by switching the controller during runtime. Maybe, we should migrate the output example to the velocity example?
Additionally: The test_forward_position_controller.launch.py crashed for me:
[publisher_forward_position_controller-1] rclpy.exceptions.InvalidParameterTypeException: Trying to set parameter 'pos2' to '[0, 0]' of type 'INTEGER_ARRAY', expecting type 'DOUBLE_ARRAY': pos2
Updating the rrbot_forward_position_publisher.yaml file to
publisher_forward_position_controller:
ros__parameters:
wait_sec_between_publish: 5
publish_topic: "/forward_position_controller/commands"
goal_names: ["pos1", "pos2", "pos3", "pos4"]
pos1: [0.785, 0.785]
pos2: [0.0, 0.0]
pos3: [-0.785, -0.785]
pos4: [0.0, 0.0]did solve that for me. This could probably be taken to a separate issue, though.
example_8/hardware/rrbot_transmissions_system_position_only.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Felix Exner (fexner) <[email protected]>
Co-authored-by: Felix Exner (fexner) <[email protected]>
Co-authored-by: Felix Exner (fexner) <[email protected]>
* Refactor logging for example 1 - 6 * Update logger for remaining examples * Revert change to copyright * Revert "Reduce update rate to avoid clogging the log" This reverts commit 10f2e22. * Don't use throttling for example_6 * Update example_1/doc/userdoc.rst Co-authored-by: Felix Exner (fexner) <[email protected]> * Update example_2/doc/userdoc.rst Co-authored-by: Felix Exner (fexner) <[email protected]> * Updating docs * Merge into one logging statement * Remove throttled logging from example_14 * Apply suggestions from code review Co-authored-by: Felix Exner (fexner) <[email protected]> --------- Co-authored-by: Felix Exner (fexner) <[email protected]>
I had issues with writing tests for the demos, and one cause might be an overload of logging in the hardware components.
Since ros-controls/ros2_control#1585 we have logger+clock in the interfaces, so I refactored the hardware components and use throttled logging on top of that.
We encountered an issue with throttled logging from identical plugins ros2/rclcpp#2587, this is why logging in example_6 does not work currently -> I'll open a follow-up PR to remember this.
Note: This is not 1:1 backportable because the API changes were not backported. But this can be achieved by adding local member variables for clock and logger to the hardware components.