ros-controls / ros2_control

Generic and simple controls framework for ROS 2
https://control.ros.org
Apache License 2.0
465 stars 283 forks source link

Simplify implementation of transmissions #1305

Open yashi opened 7 months ago

yashi commented 7 months ago

This issue is a follow-up of https://robotics.stackexchange.com/q/107590/32825

The current implementation of transmissions has high cognitive load for me. In my mental model, a transmission is simply a function that takes a few inputs and returns a few outputs. It would be great if they were something like:

#include <iostream>
#include <utility>

class Transmission {
};

class SimpleTransmission : public Transmission {
public:
    double actuator_to_joint(double a1) const {
        return a1 * 2;
    }
};

class DifferentialTransmission : public Transmission {
public:
    std::pair<double, double> actuator_to_joint(double a1, double a2) const {
        return std::make_pair(a1 * 2, a2 * 2);
    }
};

int main() {
    SimpleTransmission simple;
    DifferentialTransmission diff;

    auto joint = simple.actuator_to_joint(3.14);
    auto joints = diff.actuator_to_joint(1.1, 2.2);

    std::cout << "Simple: " << joint << std::endl;
    std::cout << "Differential: " << joints.first << ", " << joints.second << std::endl;

    return 0;
}

@christophfroehlich told me to look at the ros2_control's design draft. And I found that there was (is?) some intention to simplify them. To quote:

The Transmission interface and base class should be merged and simplified.

It also has a proposal

template <class JointData, class ActuatorData>
class Transmission
{
public:
  virtual ~Transmission() {}

  virtual void actuatorToJoint(const std::string& interface_name,
                               const ActuatorData& act_data,
                                     JointData&    jnt_data) = 0;

  virtual void jointToActuator(const std::string& interface_name,
                               const ActuatorData& act_data,
                                     JointData&    jnt_data) = 0;
};

template <class JointData, class ActuatorData>
class ActuatorToJointTransmissionHandle
{
public:
  ActuatorToJointTransmissionHandle(const std::string&  name,
                                    Transmission<JointData, ActuatorData>* transmission,
                                    const ActuatorData& actuator_data,
                                    const JointData&    joint_data)
    : TransmissionHandle(name, transmission, actuator_data, joint_data) {}

  void propagate()
  {
    transmission_->actuatorToJoint("hardware_interface::PositionJointInterface", actuator_data_, joint_data_);
    transmission_->actuatorToJoint("hardware_interface::VelocityJointInterface", actuator_data_, joint_data_);
    transmission_->actuatorToJoint("hardware_interface::EffortJointInterface", actuator_data_, joint_data_);

    if(my_custom_check)
    {
       transmission_->actuatorToJoint("awesome_interface/FooJointInterface", actuator_data_, joint_data_);
    }

  }
};

I'd like to ask if there is still an intention to change transmission APIs or add new simplified APIs. If so, how should I approch it? I don't have any problem creating PRs but I'd like to know a direction I should take.

Oh, and I haven't used all transmissions yet. All I have used are 'Simple' and 'Differential' ones. So, if there are some features I'm missing, please give me a pointer. (I'm sure there is/was a good reason to use a pointer to double in 'ReadOnlyHandle'.)

Thanks.

willian-henrique commented 4 months ago

Hello @yashi,

It seems like we're facing similar challenges.

I'm currently working on a project that utilizes the EtherCAT driver for ROS2 (https://github.com/ICube-Robotics/ethercat_driver_ros2). Upon reviewing the code and referencing @destogl's input in https://github.com/ros-controls/ros2_control/issues/798#issuecomment-1264629481, it appears that the hardware driver should support transmission. However, the EtherCAT driver doesn't currently offer this capability.

Initially, I explored the possibility of implementing a controller hierarchy using the ChainableControllerInterface. However, I encountered a limitation: Chainable Controllers cannot provide status interfaces to other controllers. This feature is currently under development and can be tracked at: https://github.com/ros-controls/ros2_control/pull/1021.

To address this issue, I'm considering implementing the necessary functionality within the hardware interface driver itself. However, if I need to change the driver I will need to implement it again

I'd appreciate any insights or suggestions you might have on how to approach this challenge.

Thank you!

yashi commented 4 months ago

@willian2005 "hardware side" he mentioned is not the driver but "Hardware Components" of ros2_control, I assume.

willian-henrique commented 4 months ago

@yashi I think that we are talking about the same thing, I was calling the "Hardware Components" as a driver.

yashi commented 4 months ago

Multi-posted: ICube-Robotics/ethercat_driver_ros2#115

christophfroehlich commented 4 months ago

Not directly related, but I tried to add a section about transmission interfaces to the docs: https://github.com/ros-controls/ros2_control/pull/1497 As you were trying to work with them recently, could you please have a look and add whatever you would have needed to understand the current implementation more easily?