Skip to content

introduce time struct, add timeout duration to rmw_wait#25

Merged
jacquelinekay merged 1 commit intomasterfrom
wait_timeout
Aug 5, 2015
Merged

introduce time struct, add timeout duration to rmw_wait#25
jacquelinekay merged 1 commit intomasterfrom
wait_timeout

Conversation

@jacquelinekay
Copy link
Copy Markdown
Contributor

Connects to ros2/ros2#73

@jacquelinekay jacquelinekay added the in progress Actively being worked on (Kanban column) label Jul 24, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think wait_timeout doesn't need to be a pointer here, we can just pass copies of rmw_time_t

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Jul 25, 2015

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since negative values will be used to represent blocking indefinitely (or nonblocking), I could change this struct to signed integers. Alternatively, I could check for a negative duration in rclcpp and pass a bool that stores "block indefinitely" or "nonblocking" to rmw_wait, along with the timeout.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, I looked at how select() works and it takes the struct as a pointer and a NULL pointer is block forever. So maybe we should do it that way. Changing this to a signed number is not a great option in my opinion. The bool is probably my next favorite option.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also we could instead call this the rmw_timeout_t and include a bool as part of the struct which indicates forever.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea to be similar to select().

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

@dirk-thomas
Copy link
Copy Markdown
Member

+1

1 similar comment
@esteve
Copy link
Copy Markdown
Member

esteve commented Aug 4, 2015

+1

@tfoote
Copy link
Copy Markdown

tfoote commented Aug 4, 2015

+1 squashed

jacquelinekay added a commit that referenced this pull request Aug 5, 2015
introduce time struct, add timeout duration to rmw_wait
@jacquelinekay jacquelinekay merged commit d5bd0c2 into master Aug 5, 2015
@jacquelinekay jacquelinekay removed the in progress Actively being worked on (Kanban column) label Aug 5, 2015
@jacquelinekay jacquelinekay deleted the wait_timeout branch August 5, 2015 02:25
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