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

[GenericSystem] Add velocity-only mode in fake_components #540

Conversation

livanov93
Copy link
Contributor

PR info:

  • Add disable_command flag to be able to simulate disconnected driver
  • Add prepare_command_mode_switch and perform_command_mode_switch for switching between velocity-only and position-only controllers
  • Add first-order integration of velocity for position calculation in velocity mode

Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

As per call.

Comment on lines +36 to +42
enum StoppingInterface
{
NONE,
STOP_POSITION,
STOP_VELOCITY
};

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
enum StoppingInterface
{
NONE,
STOP_POSITION,
STOP_VELOCITY
};

Comment on lines +318 to +337
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;

start_modes_.clear();
stop_modes_.clear();

// Starting interfaces
// add start interface per joint in tmp var for later check
for (const auto & key : start_interfaces)
{
for (auto i = 0u; i < info_.joints.size(); i++)
{
if (key == info_.joints[i].name + "/" + hardware_interface::HW_IF_POSITION)
{
start_modes_.push_back(hardware_interface::HW_IF_POSITION);
}
if (key == info_.joints[i].name + "/" + hardware_interface::HW_IF_VELOCITY)
{
start_modes_.push_back(hardware_interface::HW_IF_VELOCITY);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;
start_modes_.clear();
stop_modes_.clear();
// Starting interfaces
// add start interface per joint in tmp var for later check
for (const auto & key : start_interfaces)
{
for (auto i = 0u; i < info_.joints.size(); i++)
{
if (key == info_.joints[i].name + "/" + hardware_interface::HW_IF_POSITION)
{
start_modes_.push_back(hardware_interface::HW_IF_POSITION);
}
if (key == info_.joints[i].name + "/" + hardware_interface::HW_IF_VELOCITY)
{
start_modes_.push_back(hardware_interface::HW_IF_VELOCITY);
}
}
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;
// Currently we don't accept ACCELERATION-only and EFFORT-only interfaces
for (const auto & key : start_interfaces)
{
for (auto i = 0ul; i < info_.joints.size(); i++)
{
if (key == info_.joints[i].name + "/" + hardware_interface::HW_IF_ACCELERATION ||
key == info_.joints[i].name + "/" + hardware_interface::HW_IF_EFFORT)
{
ret_val = false;
}
}
return ret_val;

Copy link
Member

Choose a reason for hiding this comment

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

Also not allowed if velocity is missing and there are only ACC and POS interfaces.

Comment on lines +364 to +392
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;

position_controller_running_ = false;
velocity_controller_running_ = false;

if (
start_modes_.size() != 0 &&
std::find(start_modes_.begin(), start_modes_.end(), hardware_interface::HW_IF_POSITION) !=
start_modes_.end())
{
for (size_t i = 0; i < joint_commands_[POSITION_INTERFACE_INDEX].size(); ++i)
{
joint_commands_[POSITION_INTERFACE_INDEX][i] = joint_states_[POSITION_INTERFACE_INDEX][i];
}
position_controller_running_ = true;
}

if (
start_modes_.size() != 0 &&
std::find(start_modes_.begin(), start_modes_.end(), hardware_interface::HW_IF_VELOCITY) !=
start_modes_.end())
{
for (size_t i = 0; i < joint_commands_[VELOCITY_INTERFACE_INDEX].size(); ++i)
{
joint_commands_[VELOCITY_INTERFACE_INDEX][i] = 0.0;
}
velocity_controller_running_ = true;
}
return ret_val;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;
position_controller_running_ = false;
velocity_controller_running_ = false;
if (
start_modes_.size() != 0 &&
std::find(start_modes_.begin(), start_modes_.end(), hardware_interface::HW_IF_POSITION) !=
start_modes_.end())
{
for (size_t i = 0; i < joint_commands_[POSITION_INTERFACE_INDEX].size(); ++i)
{
joint_commands_[POSITION_INTERFACE_INDEX][i] = joint_states_[POSITION_INTERFACE_INDEX][i];
}
position_controller_running_ = true;
}
if (
start_modes_.size() != 0 &&
std::find(start_modes_.begin(), start_modes_.end(), hardware_interface::HW_IF_VELOCITY) !=
start_modes_.end())
{
for (size_t i = 0; i < joint_commands_[VELOCITY_INTERFACE_INDEX].size(); ++i)
{
joint_commands_[VELOCITY_INTERFACE_INDEX][i] = 0.0;
}
velocity_controller_running_ = true;
}
return ret_val;
hardware_interface::return_type ret_val = hardware_interface::return_type::OK;
// class variable
std::vector<std::vector<bool>> used_interface_per_joint_[interface_type][joint];
// set everything to false first
for (const auto & interface : start_interfaces)
{
joint_name = split (interface.split("/")[0])
interface_name = split (interface.split("/")[1])
auto found_it = std::find(standard_interfaces_.begin(), standard_interfaces_.end(), interface_name)
if (found_it != standard_interfaces_.end())
{
auto pos = std::dist(standard_interfaces_.begin(), found_it);
auto found_joint = std::find_if(info_.joints.being(), info_.joints.end(), [&](const auto & joint) { return joint.name == joint_name; });
auto joint_pos = std::dist(info_.joints.begin(), found_joint);
used_interface_per_joint_[dist][joint_pos] = true;
}
return true;

Comment on lines +132 to +133
std::vector<uint> stop_modes_;
std::vector<std::string> start_modes_;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the nomenclature here exactly. Why do we need this?

Comment on lines +134 to +135
bool position_controller_running_;
bool velocity_controller_running_;
Copy link
Member

Choose a reason for hiding this comment

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

Why not to use one variable and use indexes defined in the above constants (line 73 and 74)?

Suggested change
bool position_controller_running_;
bool velocity_controller_running_;
size_t controller_running_;

std::vector<std::string> start_modes_;
bool position_controller_running_;
bool velocity_controller_running_;
std::chrono::system_clock::time_point begin;
Copy link
Member

Choose a reason for hiding this comment

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

Class member:

Suggested change
std::chrono::system_clock::time_point begin;
std::chrono::system_clock::time_point begin_;

@@ -66,6 +79,7 @@ return_type GenericSystem::configure(const hardware_interface::HardwareInfo & in

// Initialize storage for standard interfaces
initialize_storage_vectors(joint_commands_, joint_states_, standard_interfaces_);

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +361 to +362
const std::vector<std::string> & /*start_interfaces*/,
const std::vector<std::string> & /*stop_interfaces*/)
Copy link
Member

Choose a reason for hiding this comment

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

This probably shouldn't be commented out and then we dont' have to store interfaces for starting and stopping.


// remember old value of position
joint_pos_commands_old_ = joint_commands_[POSITION_INTERFACE_INDEX];

// do loopback on all other interfaces - starts from 1 because 0 index is position interface
Copy link
Member

Choose a reason for hiding this comment

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

Comment needs update

@mergify
Copy link
Contributor

mergify bot commented Feb 11, 2022

This pull request is in conflict. Could you fix it @livanov93?

1 similar comment
@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2022

This pull request is in conflict. Could you fix it @livanov93?

@mergify
Copy link
Contributor

mergify bot commented Mar 20, 2023

This pull request is in conflict. Could you fix it @livanov93?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants