[IR] Implementation of QoS Features#1
Conversation
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
* EDIT: alphabetical order cmakelist and imports Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Fix errors, remove redundant information, and try to make sentences more concise. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
| RCL_PUBLIC | ||
| RCL_WARN_UNUSED | ||
| rcl_ret_t | ||
| rcl_publisher_event_init( |
There was a problem hiding this comment.
All these functions should have documentation added.
|
|
||
| // rcl event specific ret codes in 20XX | ||
| /// Invalid rcl_event_t given return code. | ||
| #define RCL_RET_EVENT_INVALID 2000 |
There was a problem hiding this comment.
Is this just the same as a bad parameter error code?
rcl/src/rcl/event.c
Outdated
| rmw_ret_t ret = rmw_take_event(event->impl->rmw_handle, event_status, &taken); | ||
| if (RMW_RET_OK != ret) { | ||
| RCL_SET_ERROR_MSG(rmw_get_error_string().str); | ||
| if (RMW_RET_BAD_ALLOC == ret) { |
There was a problem hiding this comment.
I believe we have a function already for converting rmw errors to rcl errors
There was a problem hiding this comment.
I think overall our error types still need to be better defined.
There was a problem hiding this comment.
I think overall our error types still need to be better defined.
What are we missing?
There was a problem hiding this comment.
There are far fewer RMW_RET_* types than RCL_RET_* types. Getting a rcl_ret_t from a rmw_ret_t feels like rcl_ret_t isn't living up to its potential.
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
(rmw_events_t::events to point to rmw_event_t instead of rmw_event_t::data) Signed-off-by: Miaofei <miaofei@amazon.com>
| RCL_PUBLIC | ||
| RCL_WARN_UNUSED | ||
| rmw_event_t * | ||
| rcl_event_get_rmw_handle(const rcl_event_t * event); |
There was a problem hiding this comment.
Where is this used? Is there a reason we need to expose the rmw layer above the rcl layer?
There was a problem hiding this comment.
Looks like it is not used anywhere. But neither is rcl_subscription_get_rmw_handle(). However, rcl_publisher_get_rmw_handle() is used in rclcpp...
Let's just leave it here for now.
There was a problem hiding this comment.
Apparently, this function is needed by the SET_ADD_RMW macro in wait.c.
rcl/src/rcl/event.c
Outdated
| bool taken; | ||
| RCL_CHECK_ARGUMENT_FOR_NULL(event, RCL_RET_INVALID_ARGUMENT); | ||
| RCL_CHECK_ARGUMENT_FOR_NULL(event_status, RCL_RET_INVALID_ARGUMENT); | ||
| rmw_ret_t ret = rmw_take_event(event->impl->rmw_handle, event_status, &taken); |
There was a problem hiding this comment.
Seems a little odd that we're diverging rcl_take_event and rmw_take_event in terms of having the extra parameter to determine if something was actually taken. Why can't they both work the same.
There was a problem hiding this comment.
I am for removing the taken parameter.
There was a problem hiding this comment.
On second thought, it looks like the other rmw_take() functions also have this extra taken boolean. We should probably leave it in there to be consistent.
address PR comments Signed-off-by: Miaofei <miaofei@amazon.com>
Call rcutils_logging_shutdown() in TearDown
to avoid leaking logging internal objects.
As any change to this file makes test_logging_named
fail, this releaxes the EXACT_EQ call on
g_last_log_event.location->line_number[1]. Instead
of looking for a particular value, we test an invariant
of the field (line numbers are positive).
Finally, in test_logging_function, initialize g_counter
and g_function_called values to make sure the test will
not fail even if `--gtest_repeat=2` is passed.
Other tests are still failing when this flag is passed:
[ FAILED ] TestLoggingMacros.test_logging_once
[ FAILED ] TestLoggingMacros.test_logging_skipfirst
[ FAILED ] TestLoggingMacros.test_logging_skipfirst_throttle
However, fixing those tests would require actually changing
the logging implementation which is out of the scope of this
change.
Before:
$ ./test_logging_macros
Running main() from gmock_main.cc
[==========] Running 8 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 8 tests from TestLoggingMacros
[ RUN ] TestLoggingMacros.test_logging_named
[ OK ] TestLoggingMacros.test_logging_named (0 ms)
[ RUN ] TestLoggingMacros.test_logging_once
[ OK ] TestLoggingMacros.test_logging_once (0 ms)
[ RUN ] TestLoggingMacros.test_logging_expression
[ OK ] TestLoggingMacros.test_logging_expression (0 ms)
[ RUN ] TestLoggingMacros.test_logging_function
[ OK ] TestLoggingMacros.test_logging_function (0 ms)
[ RUN ] TestLoggingMacros.test_logging_skipfirst
[ OK ] TestLoggingMacros.test_logging_skipfirst (0 ms)
[ RUN ] TestLoggingMacros.test_logging_throttle
[ OK ] TestLoggingMacros.test_logging_throttle (302 ms)
[ RUN ] TestLoggingMacros.test_logging_skipfirst_throttle
[ OK ] TestLoggingMacros.test_logging_skipfirst_throttle (302 ms)
[ RUN ] TestLoggingMacros.test_logger_hierarchy
[ OK ] TestLoggingMacros.test_logger_hierarchy (0 ms)
[----------] 8 tests from TestLoggingMacros (605 ms total)
[----------] Global test environment tear-down
[==========] 8 tests from 1 test case ran. (605 ms total)
[ PASSED ] 8 tests.
=================================================================
==19903==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 504 byte(s) in 7 object(s) allocated from:
#0 0x7fd640c1fb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
#1 0x7fd64091cbb5 in __default_allocate (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5)
#2 0x7fd64092f1b0 in rcutils_string_map_init (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0)
#3 0x7fd640926641 in rcutils_logging_initialize_with_allocator (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641)
#4 0x7fd6409261ab in rcutils_logging_initialize (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab)
#5 0x564405fc6627 in TestLoggingMacros::SetUp() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x29627)
#6 0x564406045999 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0xa8999)
#7 0x564406037b7d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x9ab7d)
#8 0x564405fe4503 in testing::Test::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x47503)
#9 0x564405fe59e4 in testing::TestInfo::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x489e4)
ros2#10 0x564405fe6588 in testing::TestCase::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x49588)
ros2#11 0x564406001699 in testing::internal::UnitTestImpl::RunAllTests() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x64699)
ros2#12 0x56440604844c in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0xab44c)
ros2#13 0x564406039e46 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x9ce46)
ros2#14 0x564405ffe42d in testing::UnitTest::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x6142d)
ros2#15 0x564405fd197d in RUN_ALL_TESTS() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x3497d)
ros2#16 0x564405fd18c3 in main (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x348c3)
ros2#17 0x7fd63fd84b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
After:
$ ./test_logging_macros
Running main() from gmock_main.cc
[==========] Running 9 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 9 tests from TestLoggingMacros
[ RUN ] TestLoggingMacros.test_empty
[ OK ] TestLoggingMacros.test_empty (0 ms)
[ RUN ] TestLoggingMacros.test_logging_named
[ OK ] TestLoggingMacros.test_logging_named (0 ms)
[ RUN ] TestLoggingMacros.test_logging_once
[ OK ] TestLoggingMacros.test_logging_once (0 ms)
[ RUN ] TestLoggingMacros.test_logging_expression
[ OK ] TestLoggingMacros.test_logging_expression (0 ms)
[ RUN ] TestLoggingMacros.test_logging_function
[ OK ] TestLoggingMacros.test_logging_function (0 ms)
[ RUN ] TestLoggingMacros.test_logging_skipfirst
[ OK ] TestLoggingMacros.test_logging_skipfirst (0 ms)
[ RUN ] TestLoggingMacros.test_logging_throttle
[ OK ] TestLoggingMacros.test_logging_throttle (303 ms)
[ RUN ] TestLoggingMacros.test_logging_skipfirst_throttle
[ OK ] TestLoggingMacros.test_logging_skipfirst_throttle (302 ms)
[ RUN ] TestLoggingMacros.test_logger_hierarchy
[ OK ] TestLoggingMacros.test_logger_hierarchy (0 ms)
[----------] 9 tests from TestLoggingMacros (605 ms total)
[----------] Global test environment tear-down
[==========] 9 tests from 1 test case ran. (606 ms total)
[ PASSED ] 9 tests.
[1] https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html
Signed-off-by: Thomas Moulard <tmoulard@amazon.com>
Add rcutils_logging_shutdown() every time needed
in test_logging.cpp. Remove spurious `g_rcutils_logging_initialized = false;`
preventing rcutils_logging_shutdown() from freeing memory.
Before:
$ ./test_logging
Running main() from ../../../install-asan/gtest_vendor/src/gtest_vendor/src/gtest_main.cc
[==========] Running 5 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 5 tests from TestLogging
[ RUN ] TestLogging.test_logging_initialization
[ OK ] TestLogging.test_logging_initialization (0 ms)
[ RUN ] TestLogging.test_logging
[ OK ] TestLogging.test_logging (0 ms)
[ RUN ] TestLogging.test_log_severity
[ OK ] TestLogging.test_log_severity (0 ms)
[ RUN ] TestLogging.test_logger_severities
[ OK ] TestLogging.test_logger_severities (0 ms)
[ RUN ] TestLogging.test_logger_severity_hierarchy
[ OK ] TestLogging.test_logger_severity_hierarchy (0 ms)
[----------] 5 tests from TestLogging (0 ms total)
[----------] Global test environment tear-down
[==========] 5 tests from 1 test case ran. (0 ms total)
[ PASSED ] 5 tests.
=================================================================
==1676==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x7f7a7229db50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
#1 0x7f7a71f9abb5 in __default_allocate (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5)
#2 0x7f7a71fad1b0 in rcutils_string_map_init (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0)
#3 0x7f7a71fa4641 in rcutils_logging_initialize_with_allocator (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641)
#4 0x7f7a71fa41ab in rcutils_logging_initialize (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab)
#5 0x561e9d22c972 in TestLogging_test_logging_Test::TestBody() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x1a972)
#6 0x561e9d2b545d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa345d)
#7 0x561e9d2a71c7 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x951c7)
#8 0x561e9d2534c9 in testing::Test::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x414c9)
#9 0x561e9d2548f4 in testing::TestInfo::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x428f4)
ros2#10 0x561e9d255498 in testing::TestCase::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x43498)
ros2#11 0x561e9d2705a9 in testing::internal::UnitTestImpl::RunAllTests() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5e5a9)
ros2#12 0x561e9d2b7f10 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa5f10)
ros2#13 0x561e9d2a9490 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x97490)
ros2#14 0x561e9d26d33d in testing::UnitTest::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5b33d)
ros2#15 0x561e9d24088c in RUN_ALL_TESTS() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e88c)
ros2#16 0x561e9d2407d2 in main (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e7d2)
ros2#17 0x7f7a71402b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
Direct leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x7f7a7229db50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)
#1 0x7f7a71f9abb5 in __default_allocate (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5)
#2 0x7f7a71fad1b0 in rcutils_string_map_init (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0)
#3 0x7f7a71fa4641 in rcutils_logging_initialize_with_allocator (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641)
#4 0x7f7a71fa41ab in rcutils_logging_initialize (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab)
#5 0x561e9d22a9b6 in TestLogging_test_logging_initialization_Test::TestBody() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x189b6)
#6 0x561e9d2b545d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa345d)
#7 0x561e9d2a71c7 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x951c7)
#8 0x561e9d2534c9 in testing::Test::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x414c9)
#9 0x561e9d2548f4 in testing::TestInfo::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x428f4)
ros2#10 0x561e9d255498 in testing::TestCase::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x43498)
ros2#11 0x561e9d2705a9 in testing::internal::UnitTestImpl::RunAllTests() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5e5a9)
ros2#12 0x561e9d2b7f10 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa5f10)
ros2#13 0x561e9d2a9490 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x97490)
ros2#14 0x561e9d26d33d in testing::UnitTest::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5b33d)
ros2#15 0x561e9d24088c in RUN_ALL_TESTS() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e88c)
ros2#16 0x561e9d2407d2 in main (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e7d2)
ros2#17 0x7f7a71402b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
SUMMARY: AddressSanitizer: 144 byte(s) leaked in 2 allocation(s).
After:
$ ./test_logging
Running main() from ../../../install-asan/gtest_vendor/src/gtest_vendor/src/gtest_main.cc
[==========] Running 5 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 5 tests from TestLogging
[ RUN ] TestLogging.test_logging_initialization
[ OK ] TestLogging.test_logging_initialization (0 ms)
[ RUN ] TestLogging.test_logging
[ OK ] TestLogging.test_logging (0 ms)
[ RUN ] TestLogging.test_log_severity
[ OK ] TestLogging.test_log_severity (0 ms)
[ RUN ] TestLogging.test_logger_severities
[ OK ] TestLogging.test_logger_severities (0 ms)
[ RUN ] TestLogging.test_logger_severity_hierarchy
[ OK ] TestLogging.test_logger_severity_hierarchy (0 ms)
[----------] 5 tests from TestLogging (1 ms total)
[----------] Global test environment tear-down
[==========] 5 tests from 1 test case ran. (1 ms total)
[ PASSED ] 5 tests.
Signed-off-by: Thomas Moulard <tmoulard@amazon.com>
Implementation of QoS features in the rcl package.