LinuxCNC / linuxcnc

LinuxCNC controls CNC machines. It can drive milling machines, lathes, 3d printers, laser cutters, plasma cutters, robot arms, hexapods, and more.
http://linuxcnc.org/
GNU General Public License v2.0
1.78k stars 1.14k forks source link

limit3 not obeying maximum limit #2316

Closed PetteriAimonen closed 11 months ago

PetteriAimonen commented 1 year ago

Here are the steps I follow to reproduce the issue:

  1. Run HAL commands:

    loadrt limit3 addf limit3.0 servo setp limit3.0.max 50 setp limit3.0.in 100

  2. Check signal values with halshow

This is what I expected to happen:

Expected limit3.0.out to be limited to 50

This is what happened instead:

Saw limit3.0.out is 100.

From halshow:

Component Pins:
Owner   Type  Dir         Value  Name
     4  bit   IN           TRUE  limit3.0.enable
     4  float IN            100  limit3.0.in
     4  bit   IN          FALSE  limit3.0.load
     4  float IN             50  limit3.0.max
     4  float IN          1e+20  limit3.0.maxa
     4  float IN          1e+20  limit3.0.maxv
     4  float IN         -1e+20  limit3.0.min
     4  float OUT           100  limit3.0.out
     4  u32   IN     0x00000002  limit3.0.smooth-steps
     4  s32   OUT          3492  limit3.0.time

If I set limit3.0.maxv to 1000, it starts working. But e.g. 10000 does not work, so it is not a simple float overflow.

Versions tested:

Does not work for me in LinuxCNC 2.8.2 nor in newest git version.


@zultron I don't quite understand the logic in limit3.comp. Maybe you have some insight to what could be happening?

petterreinholdtsen commented 1 year ago

[Petteri Aimonen]

From halshow:

Component Pins:
Owner   Type  Dir         Value  Name
     4  bit   IN           TRUE  limit3.0.enable
     4  float IN            100  limit3.0.in
     4  bit   IN          FALSE  limit3.0.load
     4  float IN             50  limit3.0.max
     4  float IN          1e+20  limit3.0.maxa
     4  float IN          1e+20  limit3.0.maxv
     4  float IN         -1e+20  limit3.0.min
     4  float OUT           100  limit3.0.out
     4  u32   IN     0x00000002  limit3.0.smooth-steps
     4  s32   OUT          3492  limit3.0.time

Looking at the code, it seem like the load pin control if the out pin is updated to within min and max. Perhaps the code there should be moved outside the if block:

if (load) {
// Apply first order limit
in_pos_lim = fmin(max_, fmax(min_, invalue));
SET_NEXT_STATE(in_pos_lim, in_pos_lim);
return;
}

But I do not really know the code and have probably misunderstood something.

-- Happy hacking Petter Reinholdtsen

PetteriAimonen commented 1 year ago

Yeah, for me it would make sense to apply the min/max to input value before other computations. That is the way it worked before the previous refactoring.

andypugh commented 11 months ago

This appears to be caused by the nature of discrete time calculations. If the accelleration is < 1e7 then the component works as-expected (and passes quite a large suite of tests). I have reduced the default max accel and added an explanatory note. 994abb4627213607d77385fceacd1dc0cfadd1aa