grblHAL / core

grblHAL core code and master Wiki
Other
334 stars 88 forks source link

Unexpected Behaviour when Turning On/Off Multiple Spindles #598

Open engigeer opened 1 month ago

engigeer commented 1 month ago

I've been playing around with adding the capability for controlling a second simultaneous spindle to my machine and have encountered an unexpected quirk with the on/off behaviour. For background, this is on a FlexiHAL board with the default primary spindle configured as PWM0, and the additional/secondary spindle set to ONOFF1_DIR from the spindle plugin.

It seems that somehow the firmware may not be correctly parsing the M03/M05 commands for the primary spindle ($0), depending on on the most recent state of the secondary spindle ($1) and vice versa. Here is an example test sequence to replicate the issue:

M03 $0; (SPINDLE 0 TURNS ON) M03 $1; (NOTHING HAPPENS)

M05 $1; (NOTHING HAPPENS) M03 $1; (SPINDLE 1 TURNS ON)

M05 $0; (SPINDLE 0 TURNS OFF) M05 $1; (NOTHING HAPPENS)

M03 $1; (NOTHING HAPPENS) M05 $1; (SPINDLE 1 TURNS OFF)

I have done some preliminary digging through the spindle configuration code, thinking that perhaps somewhere a flag is being reset when it should not be, but so far have not had any luck in determining the source of this issue. I was hoping you might be able to advise if what I am attempting is unsupported, or if perhaps there is something else I might try to narrow down the source of the issue.

engigeer commented 1 month ago

I believe I may have found the root of this behaviour. It seems that the following check is comparing the current modal command (M3/M4/M5) to the existing state / previous command (M3/M4/M5) without consideration of whether the current and previous modal commands are referring to the same spindle id:

https://github.com/grblHAL/core/blob/323dd84b7972c6acab745faa6add0f32da91d2b6/gcode.c#L3337

As a temporary fix, I have (in an earlier section of the gcode parsing function) set the value of gc_state.modal.spindle.state.reserved to the result of a boolean comparison between gc_block.words.$ and gc_state.spindle.hal.id. Likely there is a more elegant way to go about this, but it does seem to resolve this issue.

terjeio commented 1 month ago

It has been a while since I started implementing multiple spindle support and I'll take a fresh look at it. IIRC I have to implement separate modal states for each spindle, and advanced functionality (such as constant surface speed and spindle sync) might be hard to get perfect if more than one spindle is running at the same time with such functionalirt enabled.

dresco commented 1 month ago

Hi Terje, if you are looking at the spindle code, I think issue #118 is still there for spindle overrides.

As I recall, rapid and feedrate overrides are working as expected, just not spindle..

engigeer commented 1 month ago

Thanks! I will plan to test out the new code and will report back on how things go. Please feel free to advise if there is any feedback in particular that would be helpful.

Also, I'm sure if this is the best place to discuss this, but I have experienced some additional quirks with enabling SPINDLE_ONOFF1 that I think are specific to the STM32F4XX driver. It seems that both the spindle plugin and the driver code, implement the ONOFF1 spindle definition which creates some duplication and conflicts (although it does still compile without errors). Based on the merge dates, I've been assuming the driver code is 'legacy', but perhaps you can confirm if this is not the case?