Fix Spindle/Laser Control menu

[descipher]

Requirements

None

Description

ACTION_ITEM Macro has issues when an encoder is the input device. The menu item or value gets changed when the user exits the menu item. Replaced ACTION_ITEM with MENU_ITEM as the fix.

Benefits

Spindle/Laser Control menu functions as expected.

Configurations

#define LASER_FEATURE
#define REPRAP_DISCOUNT_SMART_CONTROLLER or any encoder based display

Related Issues

None

[descipher]

It's not enabled in reverse as far as I can tell.

[descipher]

Nice, one would have to be very familiar with the code to know these. Thanks for the pointers.

[descipher]

  ACTION_ITEM(MSG_CUTTER(ON), cutter.enable_forward);
  #if ENABLED(SPINDLE_CHANGE_DIR)
    ACTION_ITEM(MSG_SPINDLE_REVERSE, cutter.enable_reverse);
  #endif

Looks like my error. Not sure how it got there. But this needs more work.

[descipher]

CNC and Laser mixed functions are messy. Not sure what the best path is here.

[descipher]

Should these menus be separate?

[descipher]

I think the intent of the menu should be to enable forward/reverse mode and a have a separate on/off. That requires a new MSG_SPINDLE_FORWARD string.

[thinkyhead]

Yes, to return to your original intent…. We can't help it much when the input device itself is the problem. It will work to have two items, "Spindle Forward: On / Off" and "Spindle Reverse: On / Off", but it's awkward. It would be somewhat preferable to let the menu items change but move the highlight to the appropriate item, if there's a need to be toggling on and off repeatedly for testing, or whatever.

Let me examine the problem more closely and see what shakes out. I rather prefer the idea of adding a "spindle direction" item at the end, which can only be changed when the spindle is OFF.

[descipher]

Agreed. Thanks.

[thinkyhead]

Ok, I think I got the cosmetics about right, so hopefully the direction part actually works. To get the direction, it looks directly at the current spindle direction pin state.

[descipher]

I will pull it in the morning and test pin output results to verify, thanks for the work. You rock.

[descipher]

https://streamable.com/x5p5f8 Spindle Control has pre existing issue, needs work. Laser Control is good. Digging further ...

…

On Dec 1, 2020, at 11:50 PM, Scott Lahteine ***@***.***> wrote:  Ok, I think I got the cosmetics about right, so hopefully the direction part actually works. To get the direction, it looks directly at the current spindle direction pin state. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[descipher]

Should the Power Menu use uint16, in some cases the PWM resolution could be higher than 256.
EDIT_ITEM_FAST_P(uint8, GET_TEXT(MSG_LASER_POWER),&cutter.menuPower, cutter.mpower_min(), cutter.mpower_max(), cutter.update_from_mpower)

[descipher]

ACTION_ITEM is still an issue, tested actual pin state, (P5) dir control output is correct

[descipher]

Several issues with the cutter class are observed:
The default define for display units are:

  /**
   * Speed / Power can be set ('M3 S') and displayed in terms of:
   *  - PWM255  (S0 - S255)
   *  - PERCENT (S0 - S100)
   *  - RPM     (S0 - S50000)  Best for use with a spindle
   *  - SERVO   (S0 - S180)
   */
  #define CUTTER_POWER_UNIT **PWM255**

When the menu hits the Power ITEM it gets a perpetual 0 return value. Since the convert unit routine cannot convert a default PWM value > 100.
The Laser MENU_ITEM is correct however the Laser is not in Percent value at default, it is set and displayed as PWM 0-255.
While 0 is returned the enable Spindle menu item will always be disabled.
A strategy is required to continue. Do we want this to have PWM or Percent as a default?
Testing the M3 Commands does reveals more issues on the cutter class. Basic range/formatting and more default handling cleanup.

Waiting for input.

 // Convert a cpower (e.g., SPEED_POWER_STARTUP) to unit power (upwr, upower),
  // which can be PWM, Percent, Servo angle, or RPM (rel/abs).
  static const inline cutter_power_t cpwr_to_upwr(const cutter_cpower_t cpwr) { // STARTUP power to Unit power
    const cutter_power_t upwr = (
      #if ENABLED(SPINDLE_FEATURE)
        // Spindle configured values are in RPM
        #if CUTTER_UNIT_IS(RPM)
          cpwr                            // to RPM
        #elif CUTTER_UNIT_IS(PERCENT)     // to PCT
          cpwr_to_pct(cpwr)
        #elif CUTTER_UNIT_IS(SERVO)       // to SERVO angle
          PCT_TO_SERVO(cpwr_to_pct(cpwr))
        #else                             // to PWM
          **PCT_TO_PWM(cpwr_to_pct(cpwr))**
        #endif
      #else
        // Laser configured values are in PCT
        #if CUTTER_UNIT_IS(PWM255)
          PCT_TO_PWM(cpwr)
        #else
          cpwr                            // to RPM/PCT
        #endif
      #endif

[descipher]

I would think % is be better for display consistency and formatting, considering we have 4X20 LCD's to deal with. MSG strings would need abbreviation to fit in some cases. For diagnostic and calibration the PWM display wins. Use cases would be helpful if we have any knowns. Then there is "give the user a choice in the menu*" that's my preferred direction if I have cycles.

[descipher]

Question: Is it possible to have an ACTION_ITEM Menu call that can ignore the encoder and only process the use_click.
This would solve the issue where a menu item only needs to toggle a MSG_STRING and then fire an action call. Was looking at the code, too much entanglement in there for me to do.

[descipher]

The intent is to keep the menu modal focused since controlling a laser or spindle is a manual and dangerous operation. For example if you are working with CO2 power levels of 40W+ and invisible 10600nm waves you will need to be careful and aware of the state. Not sure what you mean by "the power stays off unless the value is changed, I will test the states more but could you expand on that to be clear?

[shitcreek]

In that case, I think the submenu should still timeout, but will turn off the laser and set power level to 0.

The power is off when you are inside the laser power control menu (where you adjust the power), when you press the knob to return to the parent laser control menu, the power should come back on at the power level you've set to or where it was at. However, now without adjusting the power, when you press the knob to return/set power, the power stays off while the 'laser toggle' remains in the 'on' state as well.

[descipher]

That's not what I am observing on the scope. The only odd thing I see is if you turn the laser on and the value was at zero it appears to have a previous behavior default that brings the value to 80%. This is confirmed on the scope PWM P6 output.

[descipher]

What if you are doing a laser alignment on a K40, it would not be the preferred behavior for that function.
What display hardware are you using? I have tested what you described however I am not observing those states.

[shitcreek]

Here's a video of what's going on:
https://photos.app.goo.gl/BCnMnHMbqYVmkix3A

[descipher]

I suspect two possible issues:
Either it's CUTTER_UNIT_IS(PWM255) vs CUTTER_UNIT_IS(PERCENT) or the encoder handling.
Could you please check it using
#define CUTTER_POWER_UNIT PERCENT

[descipher]

Looks like that's a 12864 LCD, I have one coming next week for testing, what board are you running?
I have the correct behavior on PWM255 so that's not the issue. Send up the configs.

[descipher]

https://streamable.com/6skwvb
Here is my test result, the only exception is the % pull request is added. (formatting only)

[descipher]

What pin are you driving the laser with? I do not see the inconsistencies you're seeing on the power meter. I will do a video with just power control and the P6 pin output (non inverted) so you can see what's up.

[descipher]

Output states of AUX D6 RAMPS 1.4 (M3 Sn) and Laser Control Menu video https://streamable.com/i9vvp6

[shitcreek]

I'm using the Creality Board v2.1 : SPINDLE_LASER_PWM_PIN 9
I see that your laser stays on when you enter the submenu to adjust the power while it's ON - it's not the case for me.

[descipher]

Isn't that board a basis of pins_SANGUINOLOLU_11.h (MELZI)
I suspect you don't have the correct pin for M3/M4/M5 gcodes using PIN 9 its normally pin 4, are you doing a Part Fan mod?

[shitcreek]

no, it uses pin_RAMPS.h
The laser works, so it can't be the pin.

[descipher]

Then you're doing a heater mod because D9 is a Heater pin and if that the case it's normally 12VDC so its needs a diode for protection or you pick off 5V from the D9 MOSFET gate pin. What is the Laser's TTL wire connected to on the board? Did you redirect the HEATER 1 #define?
#elif ENABLED(IS_RAMPS_EEF) // Hotend, Hotend, Fan
#define HEATER_1_PIN RAMPS_D9_PIN
#elif ENABLED(IS_RAMPS_EEB) // Hotend, Hotend, Bed
#define HEATER_1_PIN RAMPS_D9_PIN

[shitcreek]

yes it's 12v, but it's one of the two fan pins which for some reason are the exact same pin. I use a resistor.

#ifndef FAN_PIN
  #if EITHER(IS_RAMPS_EFB, IS_RAMPS_EFF)          // Hotend, Fan, Bed or Hotend, Fan, Fan
    #define FAN_PIN                 RAMPS_D9_PIN
  #elif EITHER(IS_RAMPS_EEF, IS_RAMPS_SF)         // Hotend, Hotend, Fan or Spindle, Fan
    #define FAN_PIN                 RAMPS_D8_PIN
  #elif ENABLED(IS_RAMPS_EEB)                     // Hotend, Hotend, Bed
    #define FAN_PIN                            4  // IO pin. Buffer needed
  #else                                           // Non-specific are "EFB" (i.e., "EFBF" or "EFBE")
    #define FAN_PIN                 RAMPS_D9_PIN
  #endif
#endif

[shitcreek]

I'm using a K40 so there isn't a TTL.

[descipher]

It's a 5V TTL level that controls the laser but you are pulling it up to 12V when the MOSFET is open using a resistor. It is designed to pull up to 5V via an opto coupler, I do not recommend 12V as it can do damage to the opto if the resistor value is too low.
Can you redirect the FAN_PIN to something unused to rule out fan control interference with the LASER_FEATURE functionality.

[descipher]

This is the circuit you are driving. Power is controlled using a potentiometer or a PWM signal on input P input L is the ENABLE.

[shitcreek]

My resistor is sufficient and it works. Also the board outputs 10v at most from my measuring with a multimeter and oscilloscope.
I have D9 set to -1, and I'm using pin 9, thus there is no pin set for the fan for there to be any interference.

[descipher]

I tested the code on pin 9 and it outputs correctly without shutting down the pin as you stated, so the issue must be with your driver mods. If you have a scope then I suggest you check the output without the Laser PSU connected.