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.

Sign up for GitHub

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

Jump to bottom

construct TimerBase/GenericTimer with Clock #523

Merged
merged 3 commits into from
Jul 28, 2018
Merged

construct TimerBase/GenericTimer with Clock #523

merged 3 commits into from
Jul 28, 2018

Conversation

dirk-thomas
Copy link
Member

@dirk-thomas dirk-thomas commented Jul 23, 2018 •
edited
Loading

Extend the Timer API to take a Clock.

Requires ros2/rcl#272.

@dirk-thomas dirk-thomas added enhancement New feature or request in review Waiting for review (Kanban column) labels Jul 23, 2018
@dirk-thomas dirk-thomas self-assigned this Jul 23, 2018
This was referenced Jul 23, 2018
Merged
Closed
@dirk-thomas
Copy link
Member Author

dirk-thomas commented Jul 24, 2018 •
edited
Loading

CI builds only up to rclcpp:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

dhood
dhood approved these changes Jul 24, 2018
View reviewed changes
Copy link
Member

@dhood dhood left a comment

Choose a reason for hiding this comment

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

looks alright to me, just some docs I noticed would be out-of-date.

rclcpp/include/rclcpp/timer.hpp
@@ -112,8 +112,10 @@ class GenericTimer : public TimerBase
* \param[in] period The interval at which the timer fires.
Copy link
Member

Choose a reason for hiding this comment

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

missing parameter doc

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 0462196.

rclcpp/include/rclcpp/timer.hpp Outdated
@@ -95,11 +97,9 @@ using TimerCallbackType = std::function<void (TimerBase &)>;
/// Generic timer templated on the clock type. Periodically executes a user-specified callback.
Copy link
Member

Choose a reason for hiding this comment

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

doc needs update

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in 0462196.

@dirk-thomas
Copy link
Member Author

some docs I noticed would be out-of-date.

Absolutely, packages on-top of rclcpp also haven't been updated yet. Will put it back in review when that is the case and the docblocks are updated too.

@dirk-thomas dirk-thomas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Jul 24, 2018
@dirk-thomas dirk-thomas mentioned this pull request Jul 26, 2018
Merged
@dirk-thomas
Copy link
Member Author

CI builds with FastRTPS (both static as well as dynamic since we can't select them individually atm):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Putting this and the connected tickets back in review.

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jul 26, 2018
@dirk-thomas
Copy link
Member Author

New CI builds after rebasing rclpy:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dirk-thomas
Copy link
Member Author

Another round of CI builds:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dirk-thomas dirk-thomas merged commit 4ddb76f into master Jul 28, 2018
@dirk-thomas dirk-thomas deleted the timer-with-clock branch July 28, 2018 01:27
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jul 28, 2018
@KenYN
Copy link

KenYN commented May 29, 2020

It would be nice to have a RosTimer as well as the WallTimer class, and/or a more generic create_timer() that takes an rcl_clock_type_t as an additional parameter.

nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
@Karsten1987 @mjcarroll
use return_loaned_message_from (ros2#523)
68c93c8 
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
@jacobperron
Add record test for ros2bag (ros2#523)
54cee3c 
* Add record test for ros2bag

A nominal smoke test to confirm that the tool is working.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unused import

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix lint

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Expect exit code 2 from rclcpp

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove unused import

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Fix test for Windows

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhood dhood dhood approved these changes

@sloretz sloretz Awaiting requested review from sloretz

Assignees

@dirk-thomas dirk-thomas

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dirk-thomas @KenYN @dhood