Babystepping and linear advance on I2S stepper stream

[vivian-ng]

As of today, babystepping and linear advance does not work on I2S stepper stream. In Marlin, the pins file for the MRR ESPE will #undef LINEAR_ADVANCE, and if you try to do babystepping on the LCD, it will trigger a reset.

According to this comment,

@vivian-ng I think the problem with both is that when using I2S, the "real" stepper ISR (see https://github.com/MarlinFirmware/Marlin/blob/1955eea1b8f13697259e0e2403d47279d2d1ac6a/Marlin/src/module/stepper.cpp#L1338) never gets called but only Stepper::pulse_phase_isr() and Stepper::block_phase_isr() (see https://github.com/MarlinFirmware/Marlin/blob/1955eea1b8f13697259e0e2403d47279d2d1ac6a/Marlin/src/HAL/HAL_ESP32/i2s.cpp#L156) whenever new steps are needed to fill the I2S buffer.
One would probably have to add the logic to call advance_isr() and babystepping_isr() there, too. Unfortunately it does not seem like an easy task on first sight since one probably needs a good understanding of the stepper timings to be able to succeed

I don't think anyone other than @simon-jouet, the author of the Marlin ESP32 HAL's I2S code, has that kind of good understanding of the I2S stepper timing to be able to resolve this. So it may take a while for this issue to be resolved. Of course, the good thing about open source software is that anyone is free to give a try at finding a solution to this. Feel free to use this thread for discussions on this, and hopefully, a report of success somewhere in the future.

[simon-jouet]

@vivian-ng Sorry I didn't get back to most message, I've been out of this for a while, far too many other things to do.

Linear advance and baby stepping are indeed currently not supported, when I wrote the I2S code it was just a proof of concept and that pretty much where it remained.

The code for I2S stepping is fairly straightforward there is a lot of it that can be abstracted away. You can pretty much abstract away the entire i2s.cpp and only consider that there is a ring buffer of steps. IIRC the stream is configured to run at 250khz to make each step 4us.

So each entry in the ring buffer represent 4us, if the bit is set to 1 then it's going to send a step to the driver for 4us if it's 0 it's not. And you just need to keep the buffer full all the time.

In the current code the stepper ISR is entirely disabled and we use the notification that one of the buffer in the ring has been consumed to plan the next ~4000 steps (here a step is a discrete 4us time period not a stepper step)

baby stepping is probably just adding an entry at the end of the buffer. For Linear advance i'm not too sure because I haven't had a look, but I guess it's just about the same, just need to schedule the steps when they are needed.

P.S: I'm doing this from memory so I might be forgetting something

[vivian-ng]

@simon-jouet Thank you for chipping into the discussion. I think anyone attempting to implement this will need both an understanding of the I2S code (in terms of how it deals with the stepper pulses) and more importantly, how is babystepping and linear advance and other stuff in stepper_isr() implemented. Understanding the I2S code is probably the easy part, since it was written by one person; the code in stepper.cpp is really a challenge to figure out because many people have contributed to the code over the years.

Maybe we should post a FR in Marlin repo to see if anyone wants to give it a try.