Wallacoloo / printipi

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

Remove Event class #57

Closed Wallacoloo closed 9 years ago

Wallacoloo commented 9 years ago

As of now, the Event object is only ever used as a temporary. And its implementation has a few drawbacks:

The only place Events are actually used is when the State calls motionPlanner.nextStep(). After that, it immediately performs this line of code:

tupleCallOnIndex(this->ioDrivers, __iterEventOutputSequence(), evt.stepperId(), evt, [this](const OutputEvent &out) { this->scheduler.queue(out); });
_lastMotionPlannedTime = evt.time();

and then lets the event go out of scope. Without Events, this code could look something like:

_lastMotionPlannedTime = motionPlanner.processNextStep([this](const OutputEvent &out) { this->scheduler.queue(out); });

and a whole bunch of complexity could be removed elsewhere (remove AxisStepper.getEvent(), remove AxisStepper._index, let the AxisStepper take ownership of its stepper drivers, and completely remove the Event class).

Wallacoloo commented 9 years ago

Another possibility is to implement an iterator interface, which would look like:

for (const OutputEvent &e : motionPlanner.iterNextStep()) {
    scheduler.queue(e);
}

This has a few advantages: it requires less templating on the MotionPlanner side, as well as in all of its dependencies. It also allows for some more complex logic inside the loop than what a lambda could do (local variables, etc).

Wallacoloo commented 9 years ago

Of course, it might even be worth it to just replace the nextStep() function with nextStepOutputEvent().

This potentially requires the least state. The downside is that you lose any information linking each output event to a single step. This might be problematic for homing. Currently, we wait for each step to be processed before scheduling the next. This would change to waiting for each OutputEvent to occur before scheduling the next one. This would be problematic for e.g. H-bridges, although easily fixable in code.

Wallacoloo commented 9 years ago

Implemented in devel. It compiles & runs fine on the generic/cartesian machine, but I won't be able to test the changes on a physical machine until after New Year's.