Skip to content
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

Porting test_tf2 #203

Merged
merged 46 commits into from
Jan 22, 2020
Merged

Porting test_tf2 #203

merged 46 commits into from
Jan 22, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Dec 4, 2019

Initial port for the package test_tf2

  • buffer_core_test
  • test_message_filter
  • test_convert
  • test_utils
  • test_buffer_server
  • test_buffer_client
  • test_static_publisher
  • test_tf2_bullet - Waiting this PR Porting tf2_bullet to ros2 #205

@ahcorde ahcorde added the enhancement New feature or request label Dec 17, 2019
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a number of things here, though many of them are small things to fix.

test_tf2/CMakeLists.txt Outdated Show resolved Hide resolved
test_tf2/package.xml Show resolved Hide resolved
test_tf2/test/buffer_core_test.cpp Outdated Show resolved Hide resolved
test_tf2/test/buffer_core_test.cpp Outdated Show resolved Hide resolved
test_tf2/test/buffer_core_test.cpp Outdated Show resolved Hide resolved
test_tf2/test/permuter.hpp Outdated Show resolved Hide resolved
test_tf2/test/permuter.hpp Outdated Show resolved Hide resolved
test_tf2/test/test_message_filter.cpp Outdated Show resolved Hide resolved
test_tf2/test/test_message_filter.cpp Outdated Show resolved Hide resolved
test_tf2/test/test_message_filter.cpp Show resolved Hide resolved
@ahcorde ahcorde added the in progress Actively being worked on (Kanban column) label Dec 18, 2019
@ahcorde ahcorde changed the title [WIP] - Porting test_tf2 Porting test_tf2 Dec 19, 2019
@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 16, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 16, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 16, 2020

@clalancette finally CI is green.

We can merge #217 and #216 too

All this 3 PR are included and tested in this branch https://github.com/ros2/geometry2/tree/ahcorde/test/test_tf2_msg_filter_fix2

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, quite a few more comments. However, they are all small problems to be fixed either in comments, include files, or with removing dead code. We won't need a full CI run for these; just on one platform should ensure that nothing broke.


*/

// /*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is still open.

}

// rclcpp::shutdown();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is still open.

{
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just remove this commented-out code.

{
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this commented out code.

{
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
//printf("source_frame %s target_frame %s time %f\n", source_frame.c_str(), target_frame.c_str(), eval_time.toSec());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this commented out code.

@@ -2809,7 +2868,7 @@ TEST(tf2_stamped, OperatorEqual)
*/
int main(int argc, char **argv){
testing::InitGoogleTest(&argc, argv);
builtin_interfaces::msg::Time::init(); //needed for builtin_interfaces::msg::Time::now()
ros::init(argc, argv, "tf_unittest");
// builtin_interfaces::msg::Time::init(); //needed for builtin_interfaces::msg::Time::now()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented-out code.

Comment on lines 36 to 37
#include <vector>
#include <mutex>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: alphabetize.

Comment on lines 43 to 46
#include <chrono>
#include <thread>
#include <functional>
#include <memory>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: alphabetize.

Comment on lines 41 to 42
#include <memory>
#include <functional>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: alphabetize.

};

TEST(StaticTransformPublisher, tf_from_param_server_valid)
// TODO (ahcorde) Load transform from yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand a bit on what needs to be done to port this test?

@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 20, 2020

  • Linux Build Status

@ahcorde ahcorde mentioned this pull request Jan 22, 2020
@clalancette
Copy link
Contributor

The code changes now look good to me. Do you know what is going on with the test failure in CI?

@ahcorde
Copy link
Contributor Author

ahcorde commented Jan 22, 2020

  • Linux Build Status

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good assuming green CI.

@ahcorde ahcorde merged commit 8b82505 into ros2 Jan 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/test/test_tf2 branch January 22, 2020 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress Actively being worked on (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants