#348 thc HAL component - min_velocity calculation error

[Marius Alksnys]

I found unclear how to understand thc.velocity-tol in thc HAL component documentation:
thc.velocity-tol float rw
The deviation percent from planned velocity

Note word "percent"

then I started to read thc.comp source. And this is what I found:
...
if(enable){
float min_velocity = requested_vel -(requested_vel*(1/velocity_tol));
...

Let's examine this equation by making a table:
requested_vel velocity_tol min_velocity min_velocity-should_be_IMO
5000 0 DIV/0! 5000
5000 1 0 4950
5000 10 4500 4500
5000 40 4875 3000
5000 99 −4949.(49) 50
5000 100 4950 0

I suggest changing this line to:
float min_velocity = requested_vel * 0.01 * (100 - velocity_tol);

or by changing a meaning of velocity_tol and documentation a bit - using velocity_tol as a tolerance ratio:
float min_velocity = requested_vel * (1 - velocity_tol);

Besides this, I noticed minor mistake:
pin in float volts_requested "min_velocityequested (SP)"
should be "min_velocity requested (SP)"

[Marius Alksnys]

requested_vel	velocity_tol	min_velocity	min_velocity-should_be_IMO
5000	0	DIV/0!	5000
5000	1	0	4950
5000	10	4500	4500
5000	40	4875	3000
5000	99	4949.(49)	50
5000	100	4950	0

[Marius Liebenberg]

I have made change to my component for this case. The idea of this parameter is to calculate a deviation of a certain percentage from the requested velocity.
I think the line should read thus.

min_velocity = requested_vel - ( requested_vel -(requested_vel*(velocity_tol/100)));

I also changed the correction_vel to be synchronized with the period in order to create a rate of change that is in line with what the axis can produce without having acceleration control.

correction_vel= ((requested_vel *(speed / 100)) * fperiod)/10;
This line is added after the previously corrected line.

In summary, the original thinking is correct, just the calculation has to change. Dont change the feature as it is important as is.

[Marius Alksnys]

The equation:
min_velocity = requested_vel - ( requested_vel -(requested_vel*(velocity_tol/100)));

mathematically simplified looks like:
min_velocity = requested_vel * velocity_tol / 100

Thus, I think it should be coded in such simple form.

I thought (and possibly, others) that "velocity_tol" means velocity tolerance. But from the formula above it means "minimum velocity percentage [tolerated]". So, when it is 80, it would mean that 801 mm/min is ok when 1000 mm/min is requested.

That's why I offered my expression:
min_velocity = requested_vel * (1 - velocity_tol);

where "velocity_tol" would mean tolerance ratio. Eg. 0.2 would mean that 801 mm/min is ok when 1000 mm/min is requested.

But to leave it compatible with existing configs, we may leave "velocity_tol", just make its description very clear.
Any better ideas of making it more correct and compatible together?