Marlin: 32 bit: Temp ISR starvation on STM32F1

[xC0000005]

Description

Now that the temp timer rates are restored by #8833, my temps are stable, until the steppers start moving during a print. I can pause octo-print and the temps will go right back to stable, but the temp ISR is being starved by the stepper ISR.

To get my printer back up and running, I switched my ISRs to be priority based instead of enable/disable based (and since an ISR can't pre-empt itself on STM32, this works - Temp ISR is a lower priority than the stepper ISR, maintaining the behavior where the stepper ISR can execute even if the temp one is already running).

But...this probably isn't the right way to fix it. I see there's a minimum delay 'STEP_TIMER_MIN_INTERVAL' added by #9985. Is there any guidance on what this should be set to? Or is it simply a case of "set it larger until things work?"

Steps to Reproduce

Using a Malyan M200 V1 or V2, set temps to something like 195. Watch them stabilize nicely, and then go into wild swings during a print unless paused.

Expected behavior: [What you expect to happen]

Stable temperatures even during printing, if known good PID values are used.

Actual behavior: [What actually happens]

When steppers are moving (aka, commands contain a bunch of movement), temperatures swing wildly. Pausing allows temp to return to normal.

[p3p]

I see there's a minimum delay 'STEP_TIMER_MIN_INTERVAL' added by #9985. Is there any guidance on what this should be set to?

I don't really know the inner workings of the STM32F1 platform, but 8us between steps (the default) should be plenty (it's the same for AVR) assuming the timer functions are doing what they are supposed to.

[xC0000005]

Made some progress on this:

1. At least on this machine, the NVIC Grouping is set to 0 by default, meaning 4 bits of sub priority, zero bits for preemption. I added code that sets the stepper ISR to a higher priority than the temp timer one.
2. By changing the temp timer prescaler back to 1 (what it is in the builds that work), I'm getting both non-stuttering movement and stable temps.

I haven't created a PR for this because I'm trying to understand if I can bypass the enable/disable functionality completely. The general rule seems to be "The Temp ISR can be interrupted by the stepper ISR." By placing the stepper ISR at a higher priority (a lower number in STFM103s), the stepper ISR should automatically pre-empt the temp ISR with none of this "in temp isr" stuff needed.

[p3p]

It's been a while since I set up the ISRs on the LPC176x platform but I think setting up the priorities correctly is required, system > step > temp > Serial / USB (or other low priority peripherals if in use ETH, CAN ect). Now I think about it I think I do remember stuttering issues when I first started before I set up the priorities for all sub systems.

[thinkyhead]

By placing the stepper ISR at a higher priority (a lower number in STFM103s), the stepper ISR should automatically pre-empt the temp ISR with none of this "in temp isr" stuff needed.

Good "question." If the priority already prevents it from interrupting the stepper ISR in-progress, then disabling the temperature ISR within the stepper ISR (or advance_scheduler_isr) might be redundant. Then the question is, does AVR still require it too?

My suggestion would be to hook up an oscilloscope to a spare pin and call TOGGLE on that pin at the top of Temperature::isr like so: if (in_temp_isr) { TOGGLE(OSC_PIN); return; }. Then you can play around with the interrupt on/off behavior and see whether it ever gets triggered.

[p3p]

Then the question is, does AVR still require it too?

AVR has no interrupt priority system so all these workarounds are to stop re-entry into the temp ISR because the stepper Interrupt is active during the temp ISR and the stepper ISR re-enables the temp Interrupt at the end.

[thinkyhead]

@p3p — Then it sounds like we can conditionalize all that stuff out for ARM, or make macros that are empty for ARM but contain the disable/reenable for AVR.

[thinkyhead]

Added a PR as a starting-point for additional changes.

[xC0000005]

This has been fixed for a while. Closing.

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.