bdring / FluidNC

The next generation of motion control firmware
Other
1.61k stars 383 forks source link

Allow rewinding of commands sent via UART buffer #1345

Closed dymk closed 1 month ago

dymk commented 1 month ago

Allows for flow control like repeat to be used via the serial buffer. The lookback itself is limited to a fixed buffer size, but is useful for simple loops such as repeating a few gcode commands.

This was so I could add more comprehensive tests to the flow control logic before any refactors. Tests an edge case for repeat with negative values (casts anything <= 0 to "no repeats").

Tested with a gcode fixture, fixture_tests/fixtures/flow_control_repeat.nc

MitchBradley commented 1 month ago

Could the functionality be implemented with std::queue instead of a new circular buffer class?

MitchBradley commented 1 month ago

Or perhaps std::deque, which is fast at both ends.

MitchBradley commented 1 month ago

It is possible that deque might cost more RAM than the circular buffer. It would be good to find out. RAM is in short supply given the FluidNC feature creep that has been occurring.

dymk commented 1 month ago

Thanks for taking a look - indeed, both std::queue and std::deque will allocate on each push (or every N pushes), as they're implemented internally with linked lists of elems or chunks of elems. The motivation for a new circular buffer class was to do a single heap allocation on start, and never allocate again after that.

dymk commented 1 month ago

In terms of overhead, there would be at least one additional pointer for each element for std::queue, and two for std::dequeue. So for the Buf<char> case, we'd have at least a 4 or 8 bytes per stored element. std::vector will only be around two std::size_t of fixed overhead.

MitchBradley commented 1 month ago

Okay I'm convinced. I wasn't sure that deque was necessarily done with linked lists. That is of course an obvious way, but sometimes C++ libs have clever optimizations for degenerate cases.

dymk commented 1 month ago

It's up to the underlying implementation; std::deque looks like it tends to allocate space for ~8 elements at a time, so that lowers the overhead. But that would mean an allocation every 8 chars, and I worry memory fragmentation would eventually bite us.