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

added time param in update_reference_from_subscribers() #846

Conversation

Robotgir
Copy link
Contributor

to calculate ageOfLastCommand and use reference_timeout variable in chainable and standard controller setup a time param is required inupdate_reference_from_subscribers(
const rclcpp::Time & time, const rclcpp::Duration & /period/) as in update_and_write_commands(
const rclcpp::Time & time, const rclcpp::Duration & /period/)

@anfemosa
Copy link

In my opinion, the .vscode configuration files should not be included in the merge request

@Robotgir
Copy link
Contributor Author

Robotgir commented Nov 9, 2022

@anfemosa Thankyou for the feedback. It makes sense. I will remove them.

bmagyar
bmagyar previously approved these changes Nov 9, 2022
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

destogl
destogl previously approved these changes Nov 17, 2022
@destogl destogl force-pushed the time-arg-for-ageOfLastCommand-var-in-update-fun branch from 4981d6d to 8f96281 Compare November 17, 2022 19:20
@destogl destogl dismissed stale reviews from bmagyar and themself via 8f96281 November 17, 2022 19:25
@destogl destogl enabled auto-merge (squash) November 17, 2022 19:28
@destogl destogl force-pushed the time-arg-for-ageOfLastCommand-var-in-update-fun branch from e49a5d5 to c95579d Compare November 17, 2022 19:29
@destogl destogl enabled auto-merge (squash) November 17, 2022 19:30
@codecov-commenter
Copy link

Codecov Report

Merging #846 (8f96281) into master (925f5f3) will decrease coverage by 1.84%.
The diff coverage is 37.57%.

❗ Current head 8f96281 differs from pull request most recent head c95579d. Consider uploading reports for the commit c95579d to get more accurate results

@@            Coverage Diff             @@
##           master     #846      +/-   ##
==========================================
- Coverage   34.61%   32.77%   -1.85%     
==========================================
  Files          52       91      +39     
  Lines        2981     9467    +6486     
  Branches     1855     6367    +4512     
==========================================
+ Hits         1032     3103    +2071     
- Misses        310      689     +379     
- Partials     1639     5675    +4036     
Flag Coverage Δ
unittests 32.77% <37.57%> (-1.85%) ⬇️

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

Impacted Files Coverage Δ
controller_manager/src/controller_manager.cpp 38.36% <ø> (-1.35%) ⬇️
controller_manager/src/realtime.cpp 0.00% <0.00%> (ø)
controller_manager/src/ros2_control_node.cpp 0.00% <0.00%> (ø)
..._interface/include/hardware_interface/actuator.hpp 100.00% <ø> (ø)
...re_interface/include/hardware_interface/sensor.hpp 100.00% <ø> (ø)
...re_interface/include/hardware_interface/system.hpp 100.00% <ø> (ø)
hardware_interface/src/sensor.cpp 50.52% <ø> (ø)
hardware_interface/src/system.cpp 55.45% <ø> (ø)
...rface/test/mock_components/test_generic_system.cpp 9.14% <ø> (ø)
...dware_interface/test/test_component_interfaces.cpp 32.44% <ø> (+4.25%) ⬆️
... and 108 more

@saikishor
Copy link
Member

Looks good 👏🏽

@bmagyar bmagyar added the rolling label Dec 6, 2022
bmagyar
bmagyar previously approved these changes Dec 6, 2022
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

Thanks!

@destogl destogl merged commit fea2d8a into ros-controls:master Mar 28, 2023
@destogl destogl deleted the time-arg-for-ageOfLastCommand-var-in-update-fun branch March 28, 2023 17:41
gwalck added a commit to StoglRobotics/ros_team_workspace that referenced this pull request Jun 1, 2023
mechwiz pushed a commit to mechwiz/ros2_control that referenced this pull request Jul 3, 2023
…nce_from_subscribers() (ros-controls#846) #API-break

* Add Migration file for API breaking changes.

Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
mechwiz pushed a commit to mechwiz/ros2_control that referenced this pull request Aug 3, 2023
…nce_from_subscribers() (ros-controls#846) #API-break

* Add Migration file for API breaking changes.

Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
mechwiz pushed a commit to mechwiz/ros2_control that referenced this pull request Aug 8, 2023
…nce_from_subscribers() (ros-controls#846) #API-break

* Add Migration file for API breaking changes.

Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
mechwiz pushed a commit to mechwiz/ros2_control that referenced this pull request Sep 10, 2023
…nce_from_subscribers() (ros-controls#846) #API-break

* Add Migration file for API breaking changes.

Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
mechwiz pushed a commit to mechwiz/ros2_control that referenced this pull request Sep 11, 2023
…nce_from_subscribers() (ros-controls#846) #API-break

* Add Migration file for API breaking changes.

Co-authored-by: Denis Štogl <denis@stoglrobotics.de>
pac48 pushed a commit to pac48/ros2_control that referenced this pull request Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants