Wallacoloo / printipi

3d printing directly through the Raspberry Pi's GPIO pins
MIT License
141 stars 43 forks source link

Make drv::A4988::getEventOutputSequence output a 1 uS STEP pulse #46

Open Wallacoloo opened 10 years ago

Wallacoloo commented 10 years ago

In src/drivers/a4988.h, the time between a LOW and a HIGH pulse on the a4988 driver is set to 8 uS. In actuality, this only needs to be 1 uS. But with backends like DMA, that quantize events, two events happening 1 uS apart may actually be scheduled simultaneously. Thus, 8 uS was picked to try to avoid this.

The proper thing to do is to signal somehow that events must occur at least T time apart. There are a few ways to do this - add a quantization parameter to getEventOutputSequence, but I think the best way is to add another boolean data field to OutputEvent called forceMinInterval, which instructs the scheduler to force the interval between this event and the previous event to be at least (event2.time - event1.time).

This new data field is only meaningful in an OutputEvent stream, so it could help to keep OutputEvent the way it is and instead introduce a new class, OutputEventStream, that manages this data. But I think that's overkill.

Wallacoloo commented 10 years ago

On second thought, the suggested solution has some gaps in it - if we have 2 step events at t1 and t1 + 2 uS, and ideal timings for the STEP pin look like: {LOW at t1, HIGH at t1+1uS, LOW at t1+2uS, HIGH at t1+3uS}, the quantized times might look like: {LOW at t1, HIGH at t1+2uS, LOW at t1+2uS, HIGH at t1+4uS}. This is still a conflict (the second and third OutputEvents are contradictory).

A possibility is to have the first STEP signal of the second event also set forceMinInterval to true, but then it will delay even when independent signals are happening - like the fan pwm.

It could also be changed such that forceMinInterval=X is interpreted to mean "force there to be at least X time between this event and any previous event involving this pin, and delay future events in this sequence by any extra time added to this one." This may still be limiting in certain situations. For example, for the A4988 steppers, it requires that if one event is delayed, then all future events in the current sequence must also be delayed by the same interval, because of the fact that the direction pin should be present by the time the step pin is toggled. Delaying future events in the sequence is understandable, as the events do represent a "sequence".

But consider more complex functionality, if it should ever be beneficial: set DIR pin at t=-2 uS, but place it at least 1 uS after the previous STEP pin alteration. Pull step LOW at t=0 uS. Pull step HIGH at t=1 uS. In the case that the DIR pin is delayed by 1 uS, we don't want to push back the rest of the sequence - it'll still work, it's just best to set the DIR pin beforehand when possible for maximum protection against timing fluctuations.

It is possible to expose a portion of the Scheduler interface into getEventOutputSequence such that the function can query when the Scheduler last toggled a given pin and perform more complex logic. But this introduces more dependencies that probably aren't necessary. It also makes the desired behavior less evident, so that the scheduler can no longer easily enforce the minimum intervals in the cases where it normally could (eg soft-scheduling where we just sleep until the event, and occasionally end up over-sleeping).

Finally, the scheduler could just default to interpreting ALL desired OutputEvent intervals as minimums. But then it would forever be falling behind schedule and would have no way of catching up.

I think the solution in paragraph 3 is satisfactory, and it could be extended somewhat trivially to cover the case presented in paragraph 4 if ever need-be by adding a forceMinIntervalPin, to indicate that the minimum interval applies between this pin and a different pin.