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 semantic of Duration::sleep return value #47

Merged
merged 2 commits into from Mar 17, 2016

Conversation

Projects
None yet
3 participants
@dirk-thomas
Copy link
Member

commented Mar 17, 2016

This implements the semantic change of the return value of Duration::sleep as mentioned in #39. This is to align the behavior with Rate::sleep and allows to distinguish if the desired sleep duration was met or not.

dirk-thomas added some commits Mar 17, 2016

@dirk-thomas dirk-thomas changed the title add Duration::sleep test change semantic of Duration::sleep return value Mar 17, 2016

@dirk-thomas

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2016

@ros/ros_team Please review this carefully since it is a pretty critical part of the ROS core.

@wjwwood

This comment has been minimized.

Copy link
Member

commented Mar 17, 2016

+1, I think the change makes sense. I think it should be mentioned on the migration guide: http://wiki.ros.org/kinetic/Migration

@@ -120,6 +120,7 @@ class ROSTIME_DECL Duration : public DurationBase<Duration>
explicit Duration(const Rate&);
/**
* \brief sleep for the amount of time specified by this Duration. If a signal interrupts the sleep, resleeps for the time remaining.
* @return True if the desired sleep duration was met, false otherwise.

This comment has been minimized.

Copy link
@jacquelinekay

jacquelinekay Mar 17, 2016

Should the comments also mention that the function can return false if the sleep was interrupted (g_stopped became true)?

This comment has been minimized.

Copy link
@dirk-thomas

dirk-thomas Mar 17, 2016

Author Member

I would argue for no, because depending on what time it uses it might not be interrupted by g_stopped becoming true. And if it does I think it is covered by True if the desired sleep duration was met.

dirk-thomas added a commit that referenced this pull request Mar 17, 2016

Merge pull request #47 from ros/duration_sleep
change semantic of Duration::sleep return value

@dirk-thomas dirk-thomas merged commit 9ac479f into kinetic-devel Mar 17, 2016

1 check passed

Kpr__roscpp_core__ubuntu_xenial_amd64 Build finished. 23 tests run, 0 skipped, 0 failed.
Details

@dirk-thomas dirk-thomas deleted the duration_sleep branch Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.