Split toggle2nist component

[Sigma1912]

Creates separate components for handling toggle buttons and momentary buttons

• Restores the legacy functionality (as in 2.9) of 'toggle2nist' with added input debounce and a maximum output pulse length to avoid jamming up the component if 'is-on' fails to follow the 'on' / 'off' output signal.
• Fixes the toggle2nist man page description so it matches actual (legacy) behavior.
• Adds a new 'momentary2nist' component that does what the old 'toggle2nist' docs described plus the debounce and max puls length described above.

Background

Before #3193 'toggle2nist' component docs described the functionality as converting
momentary inputs to nist outputs but the actual functionality was converting toggled
inputs to nist outputs. This required using a preceding 'toggle' component to match
documented behavior.

The fix in #3193 changed the behavior to match the documentation (and added debouncing
to the inputs). While this fixed the issues reported on the forum it breaks functionality
for hal files that use the preceeding 'toggle' component as a workaround.
Also the name 'toggle2nist' does no longer reflect the actual function.

What is 'nist'

The term 'nist' seems to have been invented by the original author of the 'toggle2nist' component.
From the description:

[...] device that has separate on and off inputs
and an is-on output

Alternative

The 'toggle' and 'momentary' functionality could be combined in a single component.
However, due to the very descriptive component name, this new component should probably not be called 'toggle2nist'. Changing the name would then of course require an update to existing hal files.

See previous discussion: #4223

Tested

Both components have been tested on real hard ware using mechanical switches.

[BsAtHome]

May I suggest (reads debounce only once):

rtapi_u32 debounce_val = debounce;
if(debounce_val < 1 || debounce_val > 10000) {
    debounce_val = 2; // set a sane value
}

You may also want to cache is_on and in:

rtapi_bool inval = in;
rtapi_bool isonval = is_on;

and then use the cached values.

[BsAtHome]

statefollows in, which is a bool. Why isn't state a bool? Is there a specific reason for state to be an integer?

[BsAtHome]

debounce_val is unconditionally overwritten in the function. Having it as as variable here is expensive. Better use a local variable (see also other comment).

[Sigma1912]

@BsAtHome
Thank you for the input!

I'm a bit fuzzy on the variable types.
Maybe this needs an update: https://github.com/LinuxCNC/linuxcnc/blob/master/docs/src/hal/comp.adoc

Question:
You suggest rtapi_u32 debounce_val = debounce;
I wonder if I should use int debounce_val = debounce; instead, so I don't need to cast to 'int' here if ( debounce_cntr >= (int)debounce_val ) {

Also, should I change these things in 'toggle2nist.comp' as well?

[BsAtHome]

The pin type is ui32 (previously called u32), which is an unsigned 32-bit value. The RTAPI type to hold that value is rtapi_u32. Using an integer (int) would mean a difference going from unsigned to signed. You can also use rtapi_uint, which is the larger unsigned type.

Going from signed to unsigned or unsigned to signed is most of the time an error. There are legitimate constructs that would require it, but usually, an unsigned construct remains unsigned and a signed construct remains signed.
Mixing the two without careful design risks a lot of potential errors, where the "integer overflow" is probably the best known.

Please note that the old documentation is not yet up-to-date with all the changes that have been going on in HAL. Working on it...