ensure type of third argument of service method #32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dirk-thomas
added
in review
|
+1 |
| >::value>::type * = nullptr> | ||
| >::value | ||
| >::type * = nullptr | ||
| > |
There was a problem hiding this comment.
The indention of the trailing > was very confusing to me since they do not close on the column in which they originate. I had to trace back over all the pairs to figure out what the grouping was. Is it really our style to do this:
template<
...
>Rather than:
template<
...
>There was a problem hiding this comment.
This is what uncrustify is happy with. I agree that the second version would be much better. The code before the patch already had the same weird indentation though.
|
If it works for you and @esteve ok's it, then +1. It's really hard to understand what this is doing. It maybe should have some comments around it to prevent people inadvertently breaking lambda support in the future and for general clarity. |
|
+1 |
dirk-thomas
added a commit
that referenced
this pull request
May 18, 2015
ensure type of third argument of service method
mauropasse
pushed a commit
to mauropasse/rclcpp
that referenced
this pull request
Nov 20, 2020
Move rcutils/executor_event_types.h to rmw/executor_event_types.h
nnmm
pushed a commit
to ApexAI/rclcpp
that referenced
this pull request
Jul 9, 2022
* Enable fastrtps * Fix invalid timer access
DensoADAS
pushed a commit
to DensoADAS/rclcpp
that referenced
this pull request
Aug 5, 2022
* ros2GH-69 Read storage content in a separate thread For now the publishing starts only after the reading is completly done. This should change aufter ros2GH-68 is done and a thread-safe queue can be used instead of std::queue. * ros2GH-71 Add integration test for timing behavior * ros2GH-68 Introduce vendor package for shared queue - Download and install headers from moodycamel readerwriterqueue - Download and install headers from moodycamel concurrentqueue - Use readerwriterqueue in code to load and publish concurrently * ros2GH-71 Retain time difference of messages when playing a bag file - The main (play) thread sleeps until the time for publishing the message is reached. - Using std::chrono time_point and duration for type-safe time arithmetic instead of rcutils time types. * ros2GH-71 Improve stability of read test - Subscribers need to maintain a longer history if the messages are not consumed fast enough. * ros2GH-71 Fix Classloader instance lifetime The Classloader instance needs to outlive all objects created by it. * ros2GH-71 Extract playing code into a class of its own Reason: record and play have almost no common code but do the exact opposite with the storage and rclcpp. * ros2GH-70 Do not link explicitly against std_msgs - only required in tests - this decreases the amount of packages needed for a clean build without tests * ros2GH-70 Fix error message of storage * ros2GH-70 Fix pluginlib/storage issue for recording * ros2GH-71 Cleanup: variable naming * ros2GH-70 Load storage continuously instead of as fast as possible - Only load if queue contains less than 1000 messages - Wait a millisecond before loading again once the queue is long enough * ros2GH-70 Add options struct to allow specification of queue size * ros2GH-72 Wait for messages to fill up * ros2GH-74 Rename integration tests to play/record tests * ros2GH-74 Use test_msgs in integration tests - gets rid of string_msgs dependency * ros2GH-70 Rename is_not_ready to is_pending, use bulk reading to queue * ros2GH-70 Harmonize storage_loading_future variable * ros2GH-88 Read messages in order of their timestamps - Currently, we write sequentially in order of arrival time so reading in id order is fine - This may change at a later time and should not change the reading behaviour, i.e. we need to read in order of timestamps * Fix compiler error on Mac * ros2GH-8 Fix: use correct ros message type in test * ros2GH-8 Cleanup: minor code style fixes * ros2GH-8 Refactor future usage in player Make the future a class member of player to avoid having to hand it into several functions which is difficult with a move-only type. * ros2GH-8 Cleanup: remove verbose logging for every stored message * ros2GH-8 Refactor rosbag2 interface Add an explicit overload for record without a topic_names argument to record all topics. * fix: call vector.reserve instead of default initalization * fix record demo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will add the check of the type for the third argument of the
create_service_internalmethod.Required for #29.
Passes on the farm (at least for the platforms which currently work):
@esteve @tfoote @wjwwood Please review.