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.

Sign up for GitHub

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

Jump to bottom

Cartesian twist controller #300

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Cartesian twist controller #300

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d5d4dc
Introduce cartesian twist controller.
livanov93 Feb 23, 2022
9c6d2c1
Remove unused-parameter and return-type warnings.
livanov93 Feb 23, 2022
b7187c9
Add pluginlib export based on ControllerInterface.
livanov93 Feb 23, 2022
4bf2cc5
Add cartesian controllers test.
livanov93 Feb 23, 2022
8f31cf7
Correct package.xml/=.
livanov93 Feb 23, 2022
5bda0a8
Twist Controller sync for open sourcing
moriarty May 1, 2023
c312b69
fix typo in Twist Controller Tests
moriarty May 2, 2023
5c39225
Twist Controller fixup old tests
moriarty May 3, 2023
87717e7
remove unused ament_cmake_ros
moriarty May 3, 2023
d99b3af
run pre-commit to fixup formatting
moriarty May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 7 additions & 10 deletions cartesian_controllers/test/test_twist_controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ TEST_F(TwistControllerTest, joint_names_parameter_not_set)
ASSERT_TRUE(controller_->joint_name_.empty());
ASSERT_TRUE(controller_->interface_names_.empty());

controller_->get_node()->set_parameter({"joint", joint_name_});
controller_->get_node()->set_parameter({"interface_names", interface_names_});
Copy link
Contributor

Choose a reason for hiding this comment

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

This test was called joint_names_parameter_not_set but was setting the joint param.
The following test was called interface_parameter_not_set but was setting interface_name parameter.

These two tests just check to it isn't possible to configure the controller with missing mandatory parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct, could be that additional work was required.

Copy link
Member

Choose a reason for hiding this comment

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

We have to swith to generate_parameter_library anyway and then those tests get partially obsolete.


ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), NODE_ERROR);

Expand All @@ -55,7 +55,7 @@ TEST_F(TwistControllerTest, interface_parameter_not_set)
{
SetUpController(false);

controller_->get_node()->set_parameter({"interface_names", interface_names_});
controller_->get_node()->set_parameter({"joint", joint_name_});

ASSERT_EQ(controller_->on_configure(rclcpp_lifecycle::State()), NODE_ERROR);

Expand Down Expand Up @@ -138,6 +138,8 @@ TEST_F(TwistControllerTest, reactivate_success)
TEST_F(TwistControllerTest, command_callback_test)
{
SetUpController();
rclcpp::executors::MultiThreadedExecutor executor;
executor.add_node(controller_->get_node()->get_node_base_interface());

// default values
ASSERT_EQ(command_itfs_.size(), 6u);
Expand All @@ -149,20 +151,15 @@ TEST_F(TwistControllerTest, command_callback_test)
ASSERT_EQ(command_itfs_[4].get_value(), 0.0);
ASSERT_EQ(command_itfs_[5].get_value(), 0.0);

auto node_state = controller_->configure();
auto node_state = controller_->get_node()->configure();
ASSERT_EQ(node_state.id(), lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE);

node_state = controller_->get_node()->activate();
ASSERT_EQ(node_state.id(), lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE);

// send a new command
// send a new command and wait
publish_commands();

// wait for command message to be passed
ASSERT_EQ(wait_for(controller_->twist_command_subscriber_), rclcpp::WaitResultKind::Ready);

// process callbacks
rclcpp::spin_some(controller_->get_node()->get_node_base_interface());
ASSERT_TRUE(controller_->wait_for_commands(executor));
Copy link
Contributor

Choose a reason for hiding this comment

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

This wait_for resulted in rclcpp throwing an exception, there was a wait_for_commands which looks like it was intended for this. A similar pattern exists in the other controller tests.

ros2_controllers/admittance_controller/test/test_admittance_controller.cpp

Lines 268 to 269 in 6793e02

publish_commands();
ASSERT_TRUE(controller_->wait_for_commands(executor));


// update successful
ASSERT_EQ(
Expand Down