CNC Mode M3,M4 Sxxx GCode PWM not consistently interpreted

SmartCNC3D

Only the first M3,4 Sxxx spead is read unless a M5 is sent the rpm doesnt change

I 'fixed' it.......

void CNCDriver::spindleOff()

{

    spindleRpm=0;

    spindleSpeed=0;

    if(direction == 0) return; // already off

    if(EVENT_SPINDLE_OFF){

      #if CNC_ENABLE_PIN > -1

        WRITE(CNC_ENABLE_PIN,!CNC_ENABLE_WITH);

      #endif

    }

    HAL::delayMilliseconds(CNC_WAIT_ON_DISABLE);

direction = 0;

}

/** Turns spindle on. Default implementation uses a enable pin CNC_ENABLE_PIN. If

CNC_DIRECTION_PIN is not -1 it sets direction to CNC_DIRECTION_CW. rpm is ignored.

To override with event system, return false for the event

EVENT_SPINDLE_CW(rpm)

*/

void CNCDriver::spindleOnCW(int32_t rpm)

{ 

  if  (rpm > CNC_RPM_MAX)

    rpm=CNC_RPM_MAX;

 

  spindleSpeed=map(rpm,0,CNC_RPM_MAX,0,CNC_PWM_MAX);  // linear interpolation

  spindleRpm=rpm;                                     // for display

  if(direction == 1)

        return;

    // spindleOff();

    // spindleRpm=rpm;// for display

    direction = 1;

    if(EVENT_SPINDLE_CW(rpm)) {

      #if CNC_DIRECTION_PIN > -1

        WRITE(CNC_DIRECTION_PIN, CNC_DIRECTION_CW);

      #endif

      #if CNC_ENABLE_PIN > -1

        WRITE(CNC_ENABLE_PIN, CNC_ENABLE_WITH);

      #endif

    }

    HAL::delayMilliseconds(CNC_WAIT_ON_ENABLE);

}

/** Turns spindle on. Default implementation uses a enable pin CNC_ENABLE_PIN. If

CNC_DIRECTION_PIN is not -1 it sets direction to !CNC_DIRECTION_CW. rpm is ignored.

To override with event system, return false for the event

EVENT_SPINDLE_CCW(rpm)

*/

void CNCDriver::spindleOnCCW(int32_t rpm)

{ if  (rpm > CNC_RPM_MAX)

    rpm=CNC_RPM_MAX;

  spindleSpeed=map(rpm,0,CNC_RPM_MAX,0,CNC_PWM_MAX);  // linear interpolation

  spindleRpm=rpm;                                     // for display

  if(direction == -1)

      return;

  spindleOff();

  spindleRpm=rpm;// for display

  direction = -1;

  if(EVENT_SPINDLE_CCW(rpm)) {

     #if CNC_DIRECTION_PIN > -1

        WRITE(CNC_DIRECTION_PIN, !CNC_DIRECTION_CW);

     #endif

        #if CNC_ENABLE_PIN > -1

        WRITE(CNC_ENABLE_PIN, CNC_ENABLE_WITH);

     #endif

    }

    HAL::delayMilliseconds(CNC_WAIT_ON_ENABLE);

}

#endif

Repetier

I see in official version that it tests same direction. So fine as long as rpm is not relevant. Fix for official driver would then be

void CNCDriver::spindleOnCW(int32_t rpm)

{

  spindleSpeed=map(rpm,0,CNC_RPM_MAX,0,CNC_PWM_MAX);// linear interpolation

  

    if(direction == 1 && spindleRpm == rpm)

        return;

    if(direction == -1) {

    spindleOff();

    }

    spindleRpm = rpm;// for display

    direction = 1;

    if(EVENT_SPINDLE_CW(rpm)) {

#if CNC_DIRECTION_PIN > -1

        WRITE(CNC_DIRECTION_PIN, CNC_DIRECTION_CW);

#endif

#if CNC_ENABLE_PIN > -1

        WRITE(CNC_ENABLE_PIN, CNC_ENABLE_WITH);

#endif

    }

    HAL::delayMilliseconds(CNC_WAIT_ON_ENABLE);

}

I also added spindleOff to execute only when direction changes. I thing the extra pause is a good thing instead of stopping on full speed.

Setting the speed would be done in EVENT_SPINDLE_CW which is then hardware specific.

SmartCNC3D

and testing for CNC_RPM_MAX ? , I think there should be also an CNC_RPM_MIN under this spindle must go to zero RPM a stalled spindle will overheat and kill itself.

In hindsight I think a simple #ifdefine in conditions., with a simple switch statement or ifthenelse in extruder  would solve the problem for 90% of the applications the input to PWM_POS[] is either 
1. Thermal Input in FFF mode
2. spindleSpeed in CNC mode
3. LazerIntensity in Laser mode to a
chosen PWM channel, there are very few options left in anycase.
and output a selected PWM channel.

 No need for coding and amateurs like me to scratch in the code, i like it simple.

SmartCNC3D

I have asked my CNC friends and appart from cutting left hand thread or finishing with a toolchange no one thinks a CCW is important, probably beyond the application for your users, a PWM output then solves all problems as the PWM output can be a simple on off signal as well, both for thermal/laser/cnc or proportional. very clean and simple 

Repetier

Not owning a real CNC I'm not really familiar with all the wishes here.
CNC_RPM_MIN do you mean there is a minimum and otherwise a spindle would stall? Because we do not measure real speed. In fact rpm gets computed from interpolation for pwm, that is all.

PWM is the other problem. For V2 I have already a solution for it as we there can define PWM inputs that get mapped to a pwm function of choice, which is preferably a hardware pwm. For V1 we have no generic PWM and only use software PWM with 15Hz or faster with lower resolution. That normally is not suitable for laser/cnc input I guess, which is why I never wrote a general software pwm solution here. And on Mega boards we use already some timers internally so there are only 2 HW timers for pwm creation free, but we have no general system for it. Hence the not so nice trick with own event to provide the correct code for it.

SmartCNC3D

Repetier said:

Not owning a real CNC I'm not really familiar with all the wishes here.
CNC_RPM_MIN do you mean there is a minimum and otherwise a spindle would stall? Because we do not measure real speed. In fact rpm gets computed from interpolation for pwm, that is all.

PWM is the other problem. For V2 I have already a solution for it as we there can define PWM inputs that get mapped to a pwm function of choice, which is preferably a hardware pwm. For V1 we have no generic PWM and only use software PWM with 15Hz or faster with lower resolution. That normally is not suitable for laser/cnc input I guess, which is why I never wrote a general software pwm solution here. And on Mega boards we use already some timers internally so there are only 2 HW timers for pwm creation free, but we have no general system for it. Hence the not so nice trick with own event to provide the correct code for it.

Yes if under a minimum speed it will stall, exactly the same problem you have on fan.  simplest spindle is a PWM brush motor, or an openloop control and they dont have much torque at low speed,  to do a constant torque zero speed vector control on a spindle is a bit extreme and I would not think your target market will have CNC machines with this hardware.

That PWM is perfectly suitable for both Heater/Spindle?Laser ,  after I remapped spindleSpeed to the HE2 channel on the Rumba board it drives a spindle perfectly. I have done a little board that takes the PWM from HE2 isolate it and drive 15A in a small spindle.
If you want the design and PCB layout I am happy to share it.
You can do the general PWM remapping, its working and I think that's all the users are asking for, not a full blown CNC with tool offsets, tool diameter etc etc.

On the laser I use the pin which you have already implemented, redirect it to PORT30 on the RUmba and will use that as a gate, LaserOn/LaserOff, the PWM could be mapped exactly like a spindle with a little interface to pulse the power, no need for high resolution or high speed on those PWM's.

(**********************) Would you consider this request?
In hindsight I think a simple #ifdefine in conditions., with a simple switch statement or ifthenelse in extruder  would solve the problem for 90% of the applications the input to PWM_POS[] is either 
1. Thermal Input in FFF mode
2. spindleSpeed in CNC mode
3. LaserIntensity in Laser mode to a

im not sure if I explained myself well, your code is perfect as is ,just allow  a HE, spindleSpeed and LazerIntensity share the same PWM, It is the users responsibility to on a hardware level interface that PWM to a
1. Heater/ThermoFan
2. Spindle
3. Laser

You have already implimented all the code with FAN_Thermo ,  bar 3 instruction.

#if FAN_THERMO_PIN > -1          /*********** DJC This will be PWM for Spindle in CNC Mode *********************/      

        if(act == &thermoController) 

          {      

            if(act->currentTemperatureC < Printer::thermoMinTemp)

                pwm_pos[PWM_FAN_THERMO] = 0;

            else if(act->currentTemperatureC > Printer::thermoMaxTemp)

                pwm_pos[PWM_FAN_THERMO] = FAN_THERMO_MAX_PWM;

            else 

               {

                // Interpolate target speed

                float out = FAN_THERMO_MIN_PWM + (FAN_THERMO_MAX_PWM - FAN_THERMO_MIN_PWM) * (act->currentTemperatureC - Printer::thermoMinTemp) / (Printer::thermoMaxTemp - Printer::thermoMinTemp);

                out = CNCDriver::spindleSpeed; //****************  DJC ****************

                if(out > 255)

                    pwm_pos[PWM_FAN_THERMO] = FAN_THERMO_MAX_PWM;

                else

                    pwm_pos[PWM_FAN_THERMO] = static_cast<uint8_t>(out);

               }            

            continue;

            

          }

all it need is this..

if mode==FFF then
    out = FAN_THERMO_MIN_PWM + (FAN_THERMO_MAX_PWM - F
elsif  Mode==CNC 
   out = spindleSpeed  /// RPM mapped to 0..255 with M3,M4,M5
elseif Mode==Laser
   out = LaserIntensity /// controlled by M3 0...100%


This makes your software universal and simple without any extra CPU load.
I wouldnt even need to code under event handlers.

The user can with a simple switch or pin interlock that the HE doesnt heat up if he use a spindle or lazer
In reality I think the user will plug in a HE or a Laser or a Spindle leaving all of them on a machine doesnt make sense

best regards

Repetier

Interesting extension to my new pwm system for V2. I could there solve the min pwm with a general class that inputs a normal pwm and makes it min value sensitive. I also checked the same pin for all problem and this also gets solved with the new system. So one more point why the new solution is superior.

For V1 I will not change it. That gets too confusing with all the special cases I would have to consider if only 2 share same pin, or none plus that the system is not that flexible so implementation for general solution starts using many if/#if cases. One of the reasons I started with V2 as this system has proven to be too unflexible for the raising need of special solutions.