Add tests for ControllerInterface class.

[destogl]

This PR adds explicit tests for the ControllerInterface class where basic behaviors are tested.

The most important behavior definition here is:

update_rate parameter is read at configuration and can be updated only when controller is cleaned-up and configured again

The PR depends on #538 to be merged first.

[mergify]

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

[destogl]

@bmagyar please comment on this. Maybe you have an idea how to achieve the behavior I described.

[livanov93]

@destogl I am maybe more a fan of:

rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn 
ControllerInterface::on_configure(const rclcpp_lifecycle::State &previous_state) {

      if (get_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED)
      {
        update_rate_ = get_node()->get_parameter("update_rate").as_int();

      }
      return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
}

rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn
ControllerInterfaceImpl::on_configure(const rclcpp_lifecycle::State & previous_state)
{
   rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn cb_ret = ControllerInterface::on_configure(previous_state);
   // rest of the code
   
   return cb_ret;
}

[destogl]

This is also a valid approach, for sure. @bmagyar what do you think?

The only “drawback” from my side is that uses have to explicit call this on the parent class. If someone forgets to do this, it may come with an issue that this is not working.

[bmagyar]

we shouldn't rely on people remembering stuff for us... the code in this PR may be ugly but we don't spill complexity into user code

[livanov93]

@bmagyar @destogl We could then maybe refactor parts in the hardware_interface

[livanov93]

Generally, it looks good to me. There is only one comment regarding the configuration process.
We can decide this later when @bmagyar takes a look.

[livanov93]

@destogl I am maybe more a fan of:

rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn 
ControllerInterface::on_configure(const rclcpp_lifecycle::State &previous_state) {

      if (get_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED)
      {
        update_rate_ = get_node()->get_parameter("update_rate").as_int();

      }
      return rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
}

rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn
ControllerInterfaceImpl::on_configure(const rclcpp_lifecycle::State & previous_state)
{
   rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface::CallbackReturn cb_ret = ControllerInterface::on_configure(previous_state);
   // rest of the code
   
   return cb_ret;
}

[bmagyar]

we shouldn't rely on people remembering stuff for us... the code in this PR may be ugly but we don't spill complexity into user code

[destogl]

I going to merge this to unblock other things.