Handle steppers with shared enable pins

[ellensp]

Description

A number of times I have seen issue raised that M17/M18/M84 does not work correctly.
This is due to the hardware physically sharing stepper enable pins.

Added a warning that you cannot individually enable or disable individual steppers when they are on the same enable pin

Requirements

A controller board with shared stepper enable pins such as the Creality v4 series

Benefits

Hopefully stops users submitting bug reports that M17/M18/M84 is broken when they have a controller with shared enabled lines.

Related Issues

#22768

[thinkyhead]

Some boards are designed to share enable pins, and there is nothing to be done about that. If the point is to inform users that M17 / M18 / M84 may not behave as expected, this does nothing for the user of a machine with pre-compiled firmware already installed.

It would be much better to give an alert as part of the output of M17 / M18 / M84 when those commands are executed, and it would be a sensible behavior to ignore commands that turn off individual stepper motors unless all the other motors are already known to be disabled.

---

Note that items defined in pins_postprocess.h end up being defined for every compiled source file. So it is absolutely not necessary to define these new flags inside of pins_postprocess.h since the only file that refers to those flags is Warnings.cpp, and since there are no dependent conditions for these tests that preclude them from being moved into Warnings.cpp.

[thinkyhead]

This PR is bringing some … inspiration … because there are some deficits in M17/M18/M85 that should definitely be fixed up so that steppers can actually be enabled and disabled in a meaningful way. Steppers are enabled and disabled on a per-axis basis, and there may be an E stepper that has been reassigned for use as X2, for example, and theoretically that E1 stepper's enable pin might be overlapping with another, but the overlap gets lost in the mix.

So, I'm looking at extending stepper_indirection.* with functions and enabled flags to deal with ENABLE pin overlaps when they exist, but allowing those to decompose to the simple version when they don't exist. It means replacing any direct manipulation of the ENABLE pins with function calls, and adding at least one constexpr helper function to get a bitmask of the stepper axes that have overlapping ENABLE pins, and so on.

Basically, the idea is to allow axis-enable to always set the pin state, but setting flags to true only for the axes / steppers that are specifically designated to be enabled. Then we set those flags false when steppers are requested to be disabled, but only set the ENABLE pin to its OFF states when all the steppers associated with that ENABLE pin are flagged to be disabled.

The first thing I ran into when starting down this path was HAS_E_STEPPER_ENABLE, which is a very crude way to avoid turning off E steppers that share an ENABLE pin with other axes. That will have to be discarded and replaced with this new methodology.

Hopefully the final result will not add more complexity to an already challenging area of the codebase.

[ellensp]

This also needs to be tied in with these Config settings. Any that are set true that are shared are an issue...

#define DISABLE_INACTIVE_X true
#define DISABLE_INACTIVE_Y true
#define DISABLE_INACTIVE_Z true
#define DISABLE_INACTIVE_I true
#define DISABLE_INACTIVE_J true
#define DISABLE_INACTIVE_K true
#define DISABLE_INACTIVE_E true

[ellensp]

This seems to be almost a full circle, back to user issuing M17/M18/M84 and Marlin doesn't do it (because enable is shared) but no user feedback is given that it didn't happen. Or why.
(unless im miss reading the code... )
Eg X and Y enable is share. Users whats to manually move X, so sends M18 X, marlin responds OK. but X doesn't disengage, resulting in another bugreport.
It needs user feedback....

[thinkyhead]

Ok, after much effort this is now producing a proper set of overlap bits and gives a reasonable amount of feedback about overlapping axes in M17 and M18. Flashing onto a RAMPS with simple settings…

opt_set EXTRUDERS 2 MOTHERBOARD BOARD_RAMPS_14_EEB TEMP_SENSOR_1 1
pins_set ramps/RAMPS Y_ENABLE_PIN X_ENABLE_PIN E1_ENABLE_PIN E0_ENABLE_PIN
pio run --target upload -e mega2560

… gives these command responses…

> M17 X
(Y also enabled)
ok
> M17 Y
ok
> M18 Y
Y not disabled. Shared with X.
ok
> M18 X
ok
> M17 XYZE
ok
> M18 X E0
X not disabled. Shared with Y.
E0 not disabled. Shared with E1.

[ellensp]

Himm, testing on a BOARD_SANGUINOLOLU_12 as its really has shared pins

> PIN:  14   Port: D6 (A17)  E0_ENABLE_PIN                          protected 
> .                          X_ENABLE_PIN                           protected 
> .                          Y_ENABLE_PIN                           protected 
> PIN:  26   Port: A5 (A 5)  Z_ENABLE_PIN                           protected 

Test 1, looks good

M17 X
> (Y E0 also enabled)
> ok

Test 2, X,Y and E0 should be all enabled. from test1

M18 X
> ok

no warnings given

Edit:
It doesn't seem to work how I thought it would.
Surely M17 X should also set the Y and E0 flags as enabled?
It doesn't seem to
If I also do a M17 Y M17 E then a M18 X now it errors with X not disabled. Shared with Y E0.

[thinkyhead]

The M18 X is working as intended. When you disable X there is no complaint because Y and E0 were not explicitly enabled, so they are allowed to be silently disabled. If some internal operation enables them –or if M17 Y E0 were explicitly given– then M18 X would give the warning that the other axes are marked for use and therefore X cannot be disabled.

[thinkyhead]

…and now it's time for me to get some sleep. I seem to be living on NZ time.

[ellensp]

So your saying that in this example since Y has not been requested to be enabled, we can just ignore that the IO line is toggling. That is rather tricky.

[ellensp]

Done some more testing, seems to work well.

[thinkyhead]

we can just ignore that the IO line is toggling

The current approach does work well. I certainly considered whether it made sense to mark as 'enabled' the other axes that share the same enable pin, but that would have complicated things, especially for internal usage. At this point I'm interested in the question of whether CoreXY, switching extruders, MMU, etc. will behave reasonably when there is overlap. One particular case is E_DUAL_STEPPER_DRIVERS which has…

#define  ENABLE_AXIS_E0() do{  ENABLE_STEPPER_E0();  ENABLE_STEPPER_E1(); }while(0)
#define DISABLE_AXIS_E0() do{ DISABLE_STEPPER_E0(); DISABLE_STEPPER_E1(); }while(0)

It's probably never going to be the case where this would be used in combination with a setup where E0 shares ENABLE with some other axis, while E1 overlaps with some other axis or has its own pin, so I'm not going to sweat it too much. But special extruders and custom kinematics are certainly something to watch out for.

Even now, it's an open question whether M17/M18 should apply directly to enumerated stepper motors or to axes, which may use more than one stepper motor. At this time I have committed to axes, not drivers, because it simply makes more sense with the XYZIJKE axis letter designators. But, for the case of a mixing extruder, maybe you would want M18 E0 to only disable the first stepper motor and not all the E stepper motors.

Looking over the way I've associated E_STEPPERS with the enabled flags, I see that it isn't quite right for the mixing extruder, which does enable or disable all the MIXING_STEPPERS at once. Fortunately, there is the conditional define E_MANUAL which is 1 for the mixing extruder and equivalent to EXTRUDERS for most other cases. It looks like E_MANUAL should also be 1 for E_DUAL_STEPPER_DRIVERS and HAS_PRUSA_MMU2. E_MANUAL is used for menu behavior and has some application to SWITCHING_EXTRUDER and SWITCHING_NOZZLE there. Unless there is something exotic going on, I believe I can extend the use of E_MANUAL to the axis_enabled flags for the E axis and M17/M18 as well.

[thinkyhead]

The updated code does increase the basic build size a bit, likely due to calling static stepper methods where it used to do direct pin manipulation. This can probably be optimized further to get back to the original build size for all the non-overlapping cases, but it will require some cleverness. The most recent commit removes the E parameter for the single extruder case, reducing code size by ~130 bytes.

Sample sizes for a very minimal build:

=== No Overlaps ====

Single Extruder:
  Before Refactor:  53650 / 2441
  After:            53942 / 2443  ...  +292 /  +2
  Optimized E:      53812 / 2443  ...  +162 /  +2

Dual Extruders:
  Before Refactor:  57242 / 2582
  After:            57662 / 2584  ...  +420 /  +2

=== With Overlaps (X/Y E0/E1) ====

Single Extruder:
  Before Refactor:  53650 / 2441
  After:            54566 / 2451  ...  +916 / +10
  Optimized E:      54440 / 2451  ...  +790 / +10

Dual Extruders:
  Before Refactor:  57242 / 2582
  After:            58548 / 2594  ... +1306 / +12

[thinkyhead]

Moved to a new branch and PR…