operator+= and operator-= for Duration#1988
Conversation
Signed-off-by: Tyler Weaver <maybe@tylerjw.dev>
fujitatomoya
left a comment
There was a problem hiding this comment.
adding operators makes sense.
but this PR cannot even be built, can you address build failure and add some test?
|
LGTM with the changes requested by Tomoya |
|
Thank you for the review, I'll add the tests and accept the tests tonight when I'm back at my computer. I'm sorry I didn't test this before submitting this. |
Signed-off-by: Tyler Weaver <maybe@tylerjw.dev>
|
I fixed the compile errors, added other operators that I noticed were missing, and added tests for it all. |
fujitatomoya
left a comment
There was a problem hiding this comment.
@tylerjw thank you for your contribution. i got a few comments, could you share your thoughts? I was thinking that basically operator -/+ extension would be enough.
Signed-off-by: Tyler Weaver <maybe@tylerjw.dev>
|
@fujitatomoya to make this PR easier I reduced the scope to only |
|
As we have operators that work on With the current implementation if someone writes: auto x = rclcpp::Duration::from_seconds(0.1) * 33554435;The cast of of the integer value |
thanks for the information. rclcpp/rclcpp/src/rclcpp/duration.cpp Lines 219 to 222 in b953bdd but as you suggested, probably it is too much... i would not have the case either. |
Signed-off-by: Tyler Weaver maybe@tylerjw.dev