Skip to content

construct TimerBase/GenericTimer with Clock#523

Merged
dirk-thomas merged 3 commits intomasterfrom
timer-with-clock
Jul 28, 2018
Merged

construct TimerBase/GenericTimer with Clock#523
dirk-thomas merged 3 commits intomasterfrom
timer-with-clock

Conversation

@dirk-thomas
Copy link
Copy Markdown
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
@dirk-thomas dirk-thomas requested review from dhood and sloretz July 23, 2018 23:58
This was referenced Jul 23, 2018
Merged
Closed
@dirk-thomas
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

@dhood dhood Jul 24, 2018

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
Copy Markdown
Member Author

@dirk-thomas dirk-thomas Jul 24, 2018

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
Copy Markdown
Member

@dhood dhood Jul 24, 2018

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
Copy Markdown
Member Author

@dirk-thomas dirk-thomas Jul 24, 2018

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
Copy Markdown
Member Author

dirk-thomas commented Jul 24, 2018

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
Copy Markdown
Member Author

dirk-thomas commented Jul 26, 2018

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
Copy Markdown
Member Author

dirk-thomas commented Jul 27, 2018

New CI builds after rebasing rclpy:

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

@dirk-thomas
Copy link
Copy Markdown
Member Author

dirk-thomas commented Jul 27, 2018

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
Copy Markdown

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
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

@sloretz sloretz Awaiting requested review from sloretz

1 more reviewer

@dhood dhood dhood approved these changes

Reviewers whose approvals may not affect merge requirements

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