grblHAL / core

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

Unexpected Spindle Linearization Treatment of PWM/rpm Min Values #555

Open engigeer opened 4 months ago

engigeer commented 4 months ago

It seems that when spindle linearization is enabled, the values of $31 (Setting_RpmMin) and $35 (Setting_PWMMinValue), are not used for computing the conversion from pwm_data to pwm_value.

https://github.com/grblHAL/core/blob/1c1519dfb259da485886cf3f883748e662e8f1c8/spindle_control.c#L716

This could potentially lead to unexpected results (if these settings are non-zero), since the values are still used when computing the gradient.

https://github.com/grblHAL/core/blob/1c1519dfb259da485886cf3f883748e662e8f1c8/spindle_control.c#L772

I’m wondering if there is any reason not to compute the pwm_data the same as without spindle linearization, i.e.,:

pwm_value = floorf((pwm_data->piece[idx].start * rpm - pwm_data->rpm_min - pwm_data->piece[idx].end) * pwm_data->pwm_gradient) + pwm_data->min_value;

I think this should still work with the spindle linearization python code so long as the values remain unchanged from those used to generate the input data.

engigeer commented 3 months ago

Feel free to advise if this is not relevant feedback and I will close out the issue.