-
Notifications
You must be signed in to change notification settings - Fork 434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add events-executor and timers-manager in rclcpp #2140
Conversation
Signed-off-by: Alberto Soragna <[email protected]>
EXPECT_TRUE(std::chrono::steady_clock::now() - start < 200ms); | ||
} | ||
|
||
// FIX THIS TEST! The entities collector is being called too many times! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: fix this test
Actually, given that the static executor is going to be deprecated, I was about to close #1891 and here I created a completely separate entities-collector. However, I'm open to different directions. |
Let me take a look at what you have here. I refactored the entity collection features out of the base |
Oh ok. the code is a direct follow up of the #1891 PR (the base class should be exactly the same) |
Signed-off-by: Alberto Soragna <[email protected]>
Signed-off-by: Alberto Soragna <[email protected]>
Disabled events-executor tests when using connext dds (events-executor support is WIP there). New CI (rerun after 3799e31) |
Signed-off-by: Alberto Soragna <[email protected]>
Signed-off-by: Alberto Soragna <[email protected]>
…xecutor Signed-off-by: Alberto Soragna <[email protected]>
Signed-off-by: Alberto Soragna <[email protected]>
EXPECT_TRUE(std::chrono::steady_clock::now() - start < 1s); | ||
} | ||
|
||
TEST_F(TestEventsExecutor, destroy_entities) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is flaky.
It fails approximately 1/5 due to a crash, we need to investigate.
Ok, I can review that PR and then take care of it; is it your plan to first merge that one and then we can proceed with this one? |
Maybe we can meet in the middle on the common types in another PR and then the events executor and waitset can be independent. |
Closing this PR in favor of the new one #2155 |
This PR introduces the events-executor based on the design here ros2/design#305 and the already-existing open-source implementation in https://github.com/irobot-ros/events-executor.
FYI @clalancette @wjwwood @mjcarroll @mauropasse @bpwilcox