refactored the smart ptr class macros #87
Merged
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
|
These changes were pull out of #73, so I'll rebase that one once this is merged. For review. |
wjwwood
added
in review
|
CI:
|
| RCLCPP_WEAK_PTR_DEFINITIONS(Class) \ | ||
| __RCLCPP_UNIQUE_PTR_TYPEDEF(Class) | ||
|
|
||
| #define __RCLCPP_SHARED_PTR_TYPEDEF(Class) typedef std::shared_ptr<Class> SharedPtr; |
There was a problem hiding this comment.
Just an aesthetic suggestion. Given that we're redoing the macros, we can start using using instead of typedef:
#define __RCLCPP_SHARED_PTR_TYPEDEF(Class) using SharedPtr = std::shared_ptr<Class>;this would make the code more uniform if any of the aliases require a template, since typedef can't be templated.
|
+1 |
|
+1 for the proposed changes (ignoring the result of the CI job) |
|
Ok, I fixed the bug (it was caused by selecting partial diffs to commit out of the previous pr) and I switched to New Linux job: http://ci.ros2.org/job/ros2_batch_ci_linux/190/ OS X is still broken for some reason. |
|
Ok, here is the OS X one: http://ci.ros2.org/job/ros2_batch_ci_osx/125/ |
wjwwood
force-pushed
the
refactor_smart_ptr_macros
branch
from
849d886
to
ac6f98b
Compare
| RCLCPP_WEAK_PTR_DEFINITIONS(Class) \ | ||
| __RCLCPP_UNIQUE_PTR_TYPEDEF(Class) | ||
|
|
||
| #define __RCLCPP_SHARED_PTR_TYPEDEF(Class) using SharedPtr = std::shared_ptr<Class>; |
There was a problem hiding this comment.
I'd rename name this macro to __RCLCPP_SHARED_PTR_ALIAS (or something similar), given that it's no longer a typedef
There was a problem hiding this comment.
Good point, done as a fixup commit.
wjwwood
force-pushed
the
refactor_smart_ptr_macros
branch
from
ac6f98b
to
fb49ee9
Compare
wjwwood
force-pushed
the
refactor_smart_ptr_macros
branch
from
fb49ee9
to
32160fe
Compare
|
A working OS X job: http://ci.ros2.org/job/ros2_batch_ci_osx/126/ |
|
I'm going to merge, @tfoote @jacquelinekay you can comment after the fact if you like. |
wjwwood
added a commit
that referenced
this pull request
Aug 14, 2015
refactored the smart ptr class macros
nnmm
pushed a commit
to ApexAI/rclcpp
that referenced
this pull request
Jul 9, 2022
DensoADAS
pushed a commit
to DensoADAS/rclcpp
that referenced
this pull request
Aug 5, 2022
* rosbag2_transport package with python interface * use cpp for python extension * use rosbag2_transport cpp API * use rosbag2_transport API in cli * linters * ros2GH-25 Rename target librosbag2 to rosbag2 CMake already prepends libraries with `lib`, so the old name resulted in `liblibrosbag2` * ros2GH-21 Initial call of rosbag2.record() from rosbag2_transport * ros2GH-21 Add missing copyright header * ros2GH-21 Cleanup clang tidy issues * ros2GH-21 Remove rclcpp dependency from rosbag2 * ros2GH-21 Wire rosbag play into CLI * ros2GH-21 Add missing test_depend in rosbag2_transport package.xml * ros2GH-21 Unify name of python import * ros2GH-21 Enable -a in CLI, show help on wrong args * ros2GH-85 Introduce topic and type struct for readability * ros2GH-85 Do not export sqlite3 as dependency from default plugins - not referenced in header, therefore unnecessary * ros2GH-85 Move rosbag2 except typesupport to rosbag2_transport * ros2GH-85 Add rosbag2 wrapper * ros2GH-85 Change signature of create_topic to take TopicWithType * ros2GH-85 Use rosbag2 in rosbag2_transport - Don't link against rosbag2_storage anymore * ros2GH-84 Cleanup package.xmls and CMakeLists everywhere * ros2GH-21 Add missing init() and shutdown() in record * ros2GH-85 Fix Windows build * ros2GH-85 Add visibility control to rosbag2 * ros2GH-85 Cleanup and documentation * ros2GH-87 Add test package rosbag2_tests * ros2GH-87 [WIP] Add first working prototype of an end-to-end test * ros2GH-87 Use test_msgs instead of std_msgs/String in end-to-end test * ros2GH-87 Use SIGTERM instead of SIGKILL and refactor test * ros2GH-87 Make end-to-end test work on Windows * ros2GH-87 Fix uncrustify * ros2GH-87 Refactor end-to-end test fixture * ros2GH-21 Extend transport python module interface The python interface should accept all options that can be passed to rosbag2_transport * ros2GH-87 Fix test fixture for Windows * ros2GH-87 Refactor test fixture * ros2GH-87 Separate record from play end-to-end test * ros2GH-87 Make record end-to-end test work * ros2GH-87 Publish before recording to create topic * ros2GH-87 Fix record all on Windows * ros2GH-87 Check for topics instead of all * ros2GH-87 Wait until rosbag record opened database * ros2GH-87 Delete directory recursively * ros2GH-87 Delete directories recursively on Linux * ros2GH-87 Reset ROS_DOMAIN_ID to protect against concurrent tests * ros2GH-89 Make rosbag2 interfaces virtual and add explicit open() method This allows downstream packages (e.g. rosbag2_transport) to mock these interfaces in tests. * ros2GH-87 Improve test and refactoring * ros2GH-87 Minor refactoring to increase test readability * ros2GH-87 Fix environmental variable behaviour on Mac * ros2GH-87 Fix Windows build * ros2GH-89 Use mock reader and writer in rosbag2_transport tests * ros2GH-87 Add play end_to_end test * ros2GH-87 Improvements of test * ros2GH-87 Fix Windows build * ros2GH-89 Cleanup: small documentation fixes. * ros2GH-89 [WIP] Test if Writer and Reader work with class visibility * ros2GH-87 Stabilize rosbag2_play test * ros2GH-87 Minor refactoring of tests * ros2GH-87 Rename end to end tests * add license agreement * ros2GH-89 Simplification of writing to in-memory storage * ros2GH-89 Stabilize transport tests * ros2GH-87 Refactoring of tests - Extract temporary file handling - Extract subscription management * ros2GH-87 Add pytest cache to gitignore * ros2GH-87 Refactoring of play test - Extract Publisher manager * ros2GH-87 Extract record test fixture for readability * ros2GH-89 Refactor transport tests - Use subscription and publisher manager just as e2e tests - Use options in recording * ros2GH-89 Use temporary directory fixture in sqlite tests * ros2GH-89 Conform to naming standard for tests * ros2GH-89 Prevent burst publishing of all messages - Improves test stability * ros2GH-89 Improve play stability - Sometimes the first message is lost (discovery) * ros2GH-25 Fix package.xmls * Consistently use project name in CMakeLists * Minor cleanup - make rosbag2_transport description more expressive - hide unnecessary methods in typesupport_helpers - fix incorrect logging in tests - minor cleanup * Change name of nodes in rosbag2_transport * Cleanup folder structure in rosbag2_storage and rosbag2_tests - use src/<package_name>/ and test/<package_name>/ folders everywhere - harmonises with all other packages - results in better header guards * Export sqlite3 dependency as package dependency * Create node in Rosbag2Transport always * Only hold one node in rosbag2_transport * Move all duplicate files to common package * Adapt namespacing in test commons package - use "using namespace" declaratives for tests - use package name as namespace * Replace "Waiting for messages..." message * ros2GH-25 rename rosbag2_test_commons -> rosbag2_test_common * ros2GH-25 Overwrite already existing test.bag when recording This is a temporary solution and will be handled properly once a file path can be passed via the cli. * ros2GH-25 Cleanups - Log every subscription - move all dependencies onside BUILD_TESTING for rosbag2_test_common * fix cmake typo for test_common * Remove superfluous loop in rosbag2 transport * Delete superfluous test_msgs dependency * Add rclcpp to test dependencies - Apparently ament_export_dependencies does not work in rosbag2_test_common * Fix rosbag2 node test - Clock topic is no longer present on all nodes - Remove assumptions on foreign ros topics * Fix dependencies by exporting them explicitly
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.
The macros are now more modular and there they include weak pointer and unique pointer definitions.
On classes which are not copy constructible we cannot make a
make_uniquedefinition, so there are two main entry point macros now,RCLCPP_SMART_PTR_DEFINITIONSandRCLCPP_SMART_PTR_DEFINITIONS_NOT_COPYABLE. Where the latter is used on classes which are not copyable. I worked with @esteve to come up with an automatic solution to this usingenable_ifor something similar, but it doesn't seem to be possible with only one macro inside the class definition. So I made two macros instead. It will not compile if you use the wrong macro, so that should be ok.