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

For mobile robot with Isaac Sim (bug fixed version) #25

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
6 changes: 6 additions & 0 deletions include/topic_based_ros2_control/topic_based_system.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class TopicBasedSystem : public hardware_interface::SystemInterface
rclcpp::Node::SharedPtr node_;
sensor_msgs::msg::JointState latest_joint_state_;
bool sum_wrapped_joint_states_{ false };
bool initial_states_as_initial_cmd_{ false };
bool ready_to_send_cmds_{ false };
bool initial_cmd_reached_{ false };

/// Use standard interfaces for joints because they are relevant for dynamic behavior
std::array<std::string, 4> standard_interfaces_ = { hardware_interface::HW_IF_POSITION,
Expand All @@ -80,6 +83,8 @@ class TopicBasedSystem : public hardware_interface::SystemInterface

struct MimicJoint
{
std::string joint_name;
std::string mimicked_joint_name;
std::size_t joint_index;
std::size_t mimicked_joint_index;
double multiplier = 1.0;
Expand All @@ -89,6 +94,7 @@ class TopicBasedSystem : public hardware_interface::SystemInterface
/// The size of this vector is (standard_interfaces_.size() x nr_joints)
std::vector<std::vector<double>> joint_commands_;
std::vector<std::vector<double>> joint_states_;
std::vector<std::vector<double>> initial_joint_commands_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Author

Choose a reason for hiding this comment

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

This is used to store the initial value of each joint; assigning to joint_commands_ will overwrite the value.

Copy link
Author

Choose a reason for hiding this comment

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

When using Isaac Sim and Move It 2, I have a different variable for joint_command because of the overwriting behavior when I set the initial value to joint_command. It seems that joint_command is overwritten with all zeros even when joint_command messages are not exchanged.


// If the difference between the current joint state and joint command is less than this value,
// the joint command will not be published.
Expand Down
200 changes: 165 additions & 35 deletions src/topic_based_system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo&
}

// Initial command values
initial_joint_commands_ = joint_commands_;
for (auto i = 0u; i < info_.joints.size(); i++)
{
const auto& component = info_.joints[i];
Expand All @@ -91,11 +92,12 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo&
// Check the initial_value param is used
if (!interface.initial_value.empty())
{
joint_commands_[index][i] = std::stod(interface.initial_value);
initial_joint_commands_[index][i] = std::stod(interface.initial_value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to have a different variable for the initial ones? We should use command_interfaces rather than state_interfaces when getting the initial value

Copy link
Author

Choose a reason for hiding this comment

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

The following file appears to have initial values for state_interfaces. So I'm using state_interfaces.
Was this just because this file is out of date?
https://github.com/moveit/moveit_resources/blob/ros2/panda_moveit_config/config/panda.ros2_control.xacro

}
}
}
}
ready_to_send_cmds_ = true;

// Search for mimic joints
for (auto i = 0u; i < info_.joints.size(); ++i)
Expand All @@ -113,6 +115,8 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo&
throw std::runtime_error(std::string("Mimicked joint '") + joint.parameters.at("mimic") + "' not found");
}
MimicJoint mimic_joint;
mimic_joint.joint_name = joint.name;
mimic_joint.mimicked_joint_name = mimicked_joint_it->name;
mimic_joint.joint_index = i;
mimic_joint.mimicked_joint_index =
static_cast<std::size_t>(std::distance(info_.joints.begin(), mimicked_joint_it));
Expand Down Expand Up @@ -156,6 +160,15 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo&
{
sum_wrapped_joint_states_ = true;
}
if (get_hardware_parameter("use_initial_states_as_initial_commands", "false") == "true")
Copy link
Collaborator

@JafarAbdi JafarAbdi Oct 8, 2024

Choose a reason for hiding this comment

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

Did you copy these changes from https://github.com/PickNikRobotics/topic_based_ros2_control/pull/24/files? Please use git cherry-pick to retain commit authorship attribution to the original author

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review.
I didn't think I would get a response, so I had to copy other pull requests on my own.
I have never used git cherry-pick, but I will give it a try.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the late review, I'm hoping to get it merged today :)

Copy link
Author

Choose a reason for hiding this comment

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

I would like to merge the #24 pull request, is it merged into this repository? Or is there a procedure to merge forked repository?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll review it now, but I still don't understand the use case, can you give me an example where it's needed?

Copy link
Author

Choose a reason for hiding this comment

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

I misunderstood the cherry-pick in git. Does that mean I should delete the relevant part and re-commit?

Copy link
Author

Choose a reason for hiding this comment

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

I deleted the relevant part and re-committed.

{
initial_states_as_initial_cmd_ = true;
ready_to_send_cmds_ = false;
}
if (get_hardware_parameter("wait_for_reaching_initial_values", "false") == "false")
{
initial_cmd_reached_ = true;
}

return CallbackReturn::SUCCESS;
}
Expand Down Expand Up @@ -244,6 +257,18 @@ hardware_interface::return_type TopicBasedSystem::read(const rclcpp::Time& /*tim
}
}

if (!ready_to_send_cmds_ && initial_states_as_initial_cmd_)
{
for (std::size_t i = 0; i < joint_states_.size(); ++i)
{
for (std::size_t j = 0; j < joint_states_[i].size(); ++j)
{
joint_commands_[i][j] = joint_states_[i][j];
}
}
ready_to_send_cmds_ = true;
}

return hardware_interface::return_type::OK;
}

Expand All @@ -264,70 +289,175 @@ bool TopicBasedSystem::getInterface(const std::string& name, const std::string&

hardware_interface::return_type TopicBasedSystem::write(const rclcpp::Time& /*time*/, const rclcpp::Duration& /*period*/)
{
if (!ready_to_send_cmds_)
{
return hardware_interface::return_type::ERROR;
}

std::vector<std::vector<double>> target_joint_commands;
if (initial_cmd_reached_ == false)
{
double abs_sum = std::accumulate(joint_commands_[POSITION_INTERFACE_INDEX].begin(), joint_commands_[POSITION_INTERFACE_INDEX].end(), 0, [](double sum, double val) {
return sum + std::abs(val);
});
if (abs_sum >= trigger_joint_command_threshold_)
{
initial_cmd_reached_ = true;
}
}
if (initial_cmd_reached_ == false)
{
target_joint_commands = initial_joint_commands_;
}
else
{
target_joint_commands = joint_commands_;
}
// To avoid spamming TopicBased's joint command topic we check the difference between the joint states and
// the current joint commands, if it's smaller than a threshold we don't publish it.
const auto diff = std::transform_reduce(
joint_states_[POSITION_INTERFACE_INDEX].cbegin(), joint_states_[POSITION_INTERFACE_INDEX].cend(),
joint_commands_[POSITION_INTERFACE_INDEX].cbegin(), 0.0,
target_joint_commands[POSITION_INTERFACE_INDEX].cbegin(), 0.0,
[](const auto d1, const auto d2) { return std::abs(d1) + std::abs(d2); }, std::minus<double>{});
if (diff <= trigger_joint_command_threshold_)

bool exist_velocity_command = false;
static bool exist_velocity_command_old = false; // Use old state to publish zero velocities
for (std::size_t i = 0; i < info_.joints.size(); ++i)
{
if (fabs(target_joint_commands[VELOCITY_INTERFACE_INDEX][i]) > trigger_joint_command_threshold_)
{
exist_velocity_command = true;
}
}

if (diff <= trigger_joint_command_threshold_ && (exist_velocity_command == false && exist_velocity_command_old == false))
{
return hardware_interface::return_type::OK;
}

exist_velocity_command_old = exist_velocity_command;

sensor_msgs::msg::JointState joint_state;
for (std::size_t i = 0; i < info_.joints.size(); ++i)
// For Position Joint
{
joint_state.name.push_back(info_.joints[i].name);
sensor_msgs::msg::JointState joint_state;
joint_state.header.stamp = node_->now();
// only send commands to the interfaces that are defined for this joint
for (const auto& interface : info_.joints[i].command_interfaces)
for (std::size_t i = 0; i < info_.joints.size(); ++i)
{
if (interface.name == hardware_interface::HW_IF_POSITION)
// only send commands to the interfaces that are defined for this joint
for (const auto& interface : info_.joints[i].command_interfaces)
{
joint_state.position.push_back(joint_commands_[POSITION_INTERFACE_INDEX][i]);
if (interface.name == hardware_interface::HW_IF_POSITION)
{
joint_state.name.push_back(info_.joints[i].name);
joint_state.position.push_back(target_joint_commands[POSITION_INTERFACE_INDEX][i]);
}
}
else if (interface.name == hardware_interface::HW_IF_VELOCITY)
}

for (const auto& mimic_joint : mimic_joints_)
{
for (const auto& interface : info_.joints[mimic_joint.mimicked_joint_index].command_interfaces)
{
joint_state.velocity.push_back(joint_commands_[VELOCITY_INTERFACE_INDEX][i]);
if (interface.name == hardware_interface::HW_IF_POSITION)
{
for (size_t index = 0; index < joint_state.name.size(); index++)
{
if (joint_state.name[index] == mimic_joint.mimicked_joint_name)
{
joint_state.name.push_back(mimic_joint.joint_name);
joint_state.position.push_back(mimic_joint.multiplier * joint_state.position[index]);
}
}
}
}
else if (interface.name == hardware_interface::HW_IF_EFFORT)
}

if (rclcpp::ok() && joint_state.name.size() != 0)
{
topic_based_joint_commands_publisher_->publish(joint_state);
}
}

// For Velocity Joint
{
sensor_msgs::msg::JointState joint_state;
joint_state.header.stamp = node_->now();
for (std::size_t i = 0; i < info_.joints.size(); ++i)
{
// only send commands to the interfaces that are defined for this joint
for (const auto& interface : info_.joints[i].command_interfaces)
{
joint_state.effort.push_back(joint_commands_[EFFORT_INTERFACE_INDEX][i]);
if (interface.name == hardware_interface::HW_IF_VELOCITY)
{
joint_state.name.push_back(info_.joints[i].name);
joint_state.velocity.push_back(target_joint_commands[VELOCITY_INTERFACE_INDEX][i]);
}
}
else
}

for (const auto& mimic_joint : mimic_joints_)
{
for (const auto& interface : info_.joints[mimic_joint.mimicked_joint_index].command_interfaces)
{
RCLCPP_WARN_ONCE(node_->get_logger(), "Joint '%s' has unsupported command interfaces found: %s.",
info_.joints[i].name.c_str(), interface.name.c_str());
if (interface.name == hardware_interface::HW_IF_VELOCITY)
{
for (size_t index = 0; index < joint_state.name.size(); index++)
{
if (joint_state.name[index] == mimic_joint.mimicked_joint_name)
{
joint_state.name.push_back(mimic_joint.joint_name);
joint_state.velocity.push_back(mimic_joint.multiplier * joint_state.velocity[index]);
}
}
}
}
}
}

for (const auto& mimic_joint : mimic_joints_)
if (rclcpp::ok() && joint_state.name.size() != 0)
{
topic_based_joint_commands_publisher_->publish(joint_state);
}
}

// For Effort Joint
{
for (const auto& interface : info_.joints[mimic_joint.mimicked_joint_index].command_interfaces)
sensor_msgs::msg::JointState joint_state;
joint_state.header.stamp = node_->now();
for (std::size_t i = 0; i < info_.joints.size(); ++i)
{
if (interface.name == hardware_interface::HW_IF_POSITION)
// only send commands to the interfaces that are defined for this joint
for (const auto& interface : info_.joints[i].command_interfaces)
{
joint_state.position[mimic_joint.joint_index] =
mimic_joint.multiplier * joint_state.position[mimic_joint.mimicked_joint_index];
}
else if (interface.name == hardware_interface::HW_IF_VELOCITY)
{
joint_state.velocity[mimic_joint.joint_index] =
mimic_joint.multiplier * joint_state.velocity[mimic_joint.mimicked_joint_index];
if (interface.name == hardware_interface::HW_IF_EFFORT)
{
joint_state.name.push_back(info_.joints[i].name);
joint_state.effort.push_back(target_joint_commands[EFFORT_INTERFACE_INDEX][i]);
}
}
else if (interface.name == hardware_interface::HW_IF_EFFORT)
}

for (const auto& mimic_joint : mimic_joints_)
{
for (const auto& interface : info_.joints[mimic_joint.mimicked_joint_index].command_interfaces)
{
joint_state.effort[mimic_joint.joint_index] =
mimic_joint.multiplier * joint_state.effort[mimic_joint.mimicked_joint_index];
if (interface.name == hardware_interface::HW_IF_EFFORT)
{
for (size_t index = 0; index < joint_state.name.size(); index++)
{
if (joint_state.name[index] == mimic_joint.mimicked_joint_name)
{
joint_state.name.push_back(mimic_joint.joint_name);
joint_state.effort.push_back(mimic_joint.multiplier * joint_state.effort[index]);
}
}
}
}
}
}

if (rclcpp::ok())
{
topic_based_joint_commands_publisher_->publish(joint_state);
if (rclcpp::ok() && joint_state.name.size() != 0)
{
topic_based_joint_commands_publisher_->publish(joint_state);
}
}

return hardware_interface::return_type::OK;
Expand Down
Loading