Developer parameters api #31
Conversation
|
Here's an easier to read diff: |
esteve
force-pushed
the
developer-parameters-api
branch
from
7c416d0
to
5a74f32
Compare
|
+1 |
| const std::vector<rcl_interfaces::Parameter> & parameters); | ||
|
|
||
| const std::vector<rclcpp::parameter::ParameterVariant> get_parameters( | ||
| const std::vector<std::string> & names); |
There was a problem hiding this comment.
Same comments as on #30:
- Why return a const non-reference here? If it can't be a reference what is the purpose of const then?
- Can we make the getter methods const somehow?
There was a problem hiding this comment.
Why return a const non-reference here? If it can't be a reference what is the purpose of const then?
Fixed.
Can we make the getter methods const somehow?
Done.
|
I've added a few more changes that I noticed when I implemented #34 |
|
I don't understand how the remote name comes into play here. Why is it being appended to the name? |
|
@dirk-thomas because that's the name of the service on the other end. Clients use the service name to route their requests. For example, given a parameter |
|
I think appending the node name makes it very difficult to see which services belong to which nodes. I would find a hierarchy more intuitive. Also we should avoid potential collisions again. So may be something like:
|
|
@dirk-thomas I've changed it so the service names are prefixed with the remote node name. I've made the same change in #34 |
esteve
force-pushed
the
developer-parameters-api
branch
from
58ec332
to
aa69694
Compare
esteve
force-pushed
the
developer-parameters-api
branch
from
aa69694
to
a50dc29
Compare
|
This is blocking ros2/examples#23, which was caused by merging #30 too eagerly. I'm going to merge this PR and we can file tickets if there's any further issues. |
rename local_event_queue -> execution_event_queue
tests generated for each RMW impl must have unique names to be distinguishable
* ros2GH-64 Rearrange default plugins build to use public headers Also already links write integration test against the default plugins. * ros2GH-64 Remove after_write_action Query the underlying db directly in tests to determine the amount of recorded messages. * ros2GH-64 Add convenience getter for single line SQL result * ros2GH-64 Add visibility macros to enable linking on Windows * ros2GH-64 Remove second sqlite exception class (it is not needed) * ros2GH-64 Fix hanging rosbag2_read_integration_test * ros2GH-64 Always log sqlite return code * ros2GH-64 Improve opening of sqlite database - Always open db with threading mode multi-thread. This forbids sharing database connections across threads. Db access from multiple threads is possible when each thread uses its own connection. - Open sqlite db accordingly to given io flags. Readonly open works only with already existing database. - Set journal mode pragma to WAL (write ahead log) and synchronous pragma to NORMAL. This should yield good write performance. - Small fix: use *.db3 as db name in tests. * ros2GH-64 Fix package test dependencies * ros2GH-64 Fix cppcheck error * ros2GH-64 Fix asserting typesupport in test (varies on architectures) * Cleanup - consistently use const ref of string instead of string for function arguments - simplify package dependencies - minor formatting * Make play integration test compile on Mac * Fix sqlite_wrapper_integration_test
Depends on #30
Connects to ros2/ros2#11
Connects to ros2/ros2#28
@dirk-thomas @tfoote @wjwwood