🐛 Fix bool evalution of XYval and similar types

[sjasonsmith]

Description

XYval and similar types has an operator bool() which should allow them to be easily tested for zero values in conditional expressions. This was not working, because operator T*() was being implicitly used instead, which then always evaluated to true.

By making the pointer conversion explicit, it is still accessible for use, but will not be automatically used in a boolean expression.

This was found while debugging problems in BACKLASH_COMPENSATION, where the following statement evaluated to false, even though all referenced variables contained zeroed-out data.

if (!correction && !residual_error) return;

Requirements

N/A

Benefits

Remove unintuitive behavior

Configurations

N/A

Related Issues

N/A

[sjasonsmith]

@tombrazier, with this change the backlash compensation behavior is going to change. Previously this bypass would never execute, but now it can:

if (!correction && !residual_error) return;

Do you think that is going to break anything in backlash compensation? I think it will most likely just reduce overhead when this branch is taken, but I haven't examined everything closely.

[tombrazier]

@tombrazier, with this change the backlash compensation behavior is going to change. Previously this bypass would never execute, but now it can:

if (!correction && !residual_error) return;

Do you think that is going to break anything in backlash compensation? I think it will most likely just reduce overhead when this branch is taken, but I haven't examined everything closely.

Short answer: no I don't think it will break anything.

Long answer: The check is for the case where backlash is disabled by setting the amount of correction to zero. In that situation any previously unapplied correction that is recorded in Backlash::residual_error must still get applied. However once that has happened there is no need to do the rest of the backlash logic as it will always just result in zero steps being applied. i.e. the backlash calculations are redundant. The bug that is fixed here resulted in this redundant calculation happening anyway. That is both inefficient and also not what a user would expect if they have disabled backlash compensation. I think the fix will restore the more efficient and expected behaviour.

[tombrazier]

@thinkyhead After a long conversation on Discord there are several things I want to draw to your attention about this PR.

• It fixes a bunch of problems, including the input shaping infinite loop in 2.1.2.2.
• Consequently releasing a 2.1.2.3 with this PR included sounds like a good idea.
• The reason the cast to pointer operator masked the cast to bool operator is that the cast to bool operator recently became const. When used with a non-const variable the compiler chooses to implicitly cast to pointer and then cast that to bool rather than do a const cast and then use the bool cast operator.
• The ambiguity suggests that we might have gone overboard with cast operators.
• Consequently, the explicit fix here might not be the best. It is possible that the cast to pointer operator is never actually used for XYval and related types. Maybe it should be removed completely.
• And maybe the cast to bool operator should be made an explicit member function named something like all().
• Finally, if we keep the cast operators, some kind of test code would be ideal to prevent this happening again in the future. We thought about using static_assert but I am not sure that will work.

[thisiskeithb]

Patch for 2.1.x submitted:

• [2.1.x] 🐛 Fix bool evalution of XYval and similar types #27026