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

Change behavior of timer on construction #146

Merged
merged 1 commit into from
Nov 6, 2015
Merged

Conversation

jacquelinekay
Copy link
Contributor

Fixes ros2/system_tests#36

Based on the discussion in ros2/system_tests#66

Previously, timers would fire on the very first call to spin, no matter how much time passed between their construction and the call to spin.

This PR will change the behavior of timers so that the timer will fire one period after being created, or on the first call to spin IF that call happens after one period.

@wjwwood
Copy link
Member

wjwwood commented Nov 5, 2015

I'm working on getting #140 merged today, can we defer this fix until then?

@tfoote
Copy link
Contributor

tfoote commented Nov 5, 2015

+1

@jacquelinekay
Copy link
Contributor Author

Sure. I'll run tests with this branch to check its correctness but I won't
merge until #140 is in.

On Thu, Nov 5, 2015 at 2:09 PM, William Woodall notifications@github.com
wrote:

I'm working on getting #140 #140
merged today, can we defer this fix until then?


Reply to this email directly or view it on GitHub
#146 (comment).

@wjwwood
Copy link
Member

wjwwood commented Nov 5, 2015

It looks good to me too.

@dirk-thomas
Copy link
Member

+1

1 similar comment
@esteve
Copy link
Member

esteve commented Nov 5, 2015

+1

jacquelinekay added a commit that referenced this pull request Nov 6, 2015
Change behavior of timer on construction
@jacquelinekay jacquelinekay merged commit 361faf6 into master Nov 6, 2015
@jacquelinekay jacquelinekay removed the in progress Actively being worked on (Kanban column) label Nov 6, 2015
@jacquelinekay jacquelinekay deleted the fix_timer branch November 6, 2015 17:29
@jacquelinekay
Copy link
Contributor Author

This appears to have caused a regression in test_rclcpp:

http://ci.ros2.org/job/ros2_batch_ci_linux/567/testReport/

I'll follow up with a fix.

DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Aug 6, 2024
* add logs and minor fixes

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* use >0 rather than ==1 in comparison

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

---------

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Aug 8, 2024
* add logs and minor fixes

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

* use >0 rather than ==1 in comparison

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>

---------

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants