Add new joint range limiter for hardware interfaces

[saikishor]

Add new hardware joint range limiter to integrate in the resource_manager

[destogl]

I was going the same way with JointStaturation Limiter when extending for double limits. What I see here that we have very similar calculation in the RangeLimmiter and the JointSaturationLimiters. So here the question. Why not to add additional on_enfoce method to the interface that implements also this kind of limiting and then extend JointSatutrationLimiter (where we reuse most of the calculations).

This way we make sure that when we implement actually another type of limiter, we will make sure that all methods are implemtend too.

[destogl]

Why to check this? is it possible that this happens? This is a mandatory paramter, so I would propose to check this differently, i.e., not add big if, but just exit the method with error if this argument is not correct.

[saikishor]

If we are integrating this inside the ResourceManager/ResourceStorage right now we don't have this, and may be we don't want to log things in realtime, that's one more reason for it to check if this interface is set or not. I think it is important.

[destogl]

we should check if logger is provided differently. This is a bit strange way to do that. Maybe to add setter that returns error if value is not correct. Because we are checking function arguments using external method. I don't find this very nicely solved.

[destogl]

And also you are doing check after everyting is assigned. Should't we do this at the begin of the method?

[saikishor]

I can change it to use it properly. This is a new init method and this will take a vector of LimitsType and you will need to check if this size is good or not w.r.t joint_names size

[saikishor]

Let's add the JointLimits into the SoftLimits

[destogl]

here we shouldn't have the argument name commented out.

[saikishor]

OK make sense. I will remove it

[destogl]

I was going the same way with JointStaturation Limiter when extending for double limits. What I see here that we have very similar calculation in the RangeLimmiter and the JointSaturationLimiters. So here the question. Why not to add additional on_enfoce method to the interface that implements also this kind of limiting and then extend JointSatutrationLimiter (where we reuse most of the calculations).

This way we make sure that when we implement actually another type of limiter, we will make sure that all methods are implemented too.

[saikishor]

I was going the same way with JointStaturation Limiter when extending for double limits. What I see here that we have very similar calculation in the RangeLimmiter and the JointSaturationLimiters. So here the question. Why not to add additional on_enfoce method to the interface that implements also this kind of limiting and then extend JointSatutrationLimiter (where we reuse most of the calculations).

This way we make sure that when we implement actually another type of limiter, we will make sure that all methods are implemtend too.

@destogl sorry that I didn't leave any proper description in the PR. I went through the math inside your JointStaturationLimiter it is very well done for trajectories but the same cannot be applied to the CommandInterfaces and StateInterfaces

• For instance, When you are in position control may be the velocity control is not active, then the desired velocity inside this handle will be zero and then basically you will make the position not move at all and also there are different cases that doesn't correspond to individual usecases. Likewise, there are other couple of things that mightn't work, so we will need one more thing for sure.
• You are checking the dynamics constraints between the points, which is completely makes sense for those trajectories, and for the joint commands, we will have many issues. We can discuss about this.
• We have to keep the joints data separate in the ResourceManager as if the joints belong to different HW components, the activation and deactivation of those particular joints shouldn't hinder the JointLimits in any way, so in this way it is more detached and modular in my opinion

What do you think about it?

[saikishor]

@destogl thanks for taking time to do the review. I have left my comments, we can discuss it over here or over a call as you prefer.

[saikishor]

After the call, We have decided to the merge the joint_range_limiter into the joint_saturation_limiter. So the idea is to reuse the header of the JointSaturationLimiter and create different .cpp files to have different plugin types, and this way we will be able to change the type and create plugins.

We have also discussed about adding the hard limits into the SoftLimits struct or create a whole new structure with hard_limits and optional soft limits, some thing like below

// We could think of a proper naming
struct JointURDFLimits
{
   std::optional<JointLimits> hard_limits_;
   std::optional<SoftJointLimits> soft_limits_;
}