AllYarnsAreBeautiful / ayab-firmware

Contains the Arduino Firmware for the AYAB Shield
GNU General Public License v3.0
23 stars 19 forks source link

Make row buffer two rows deep #105

Open dl1com opened 1 year ago

dl1com commented 1 year ago

Currently, the firmware only stores a single row, which is the row which will be knit, respectively is current being knitted. The data for the next row will be requested on the end of the current row. The communication between the firmware and the desktop takes a moment, so the user has to wait for this communication to succeed before being allowed to move the carriage in the opposite direction and knit the next row.

We could get rid of this need to wait for the transfer of the next row by buffering an additional row in the mikrocontroller. So the controller would request two rows at the very beginning and therefore always has the n-th and n+1-th row available to knit.

Caveats:

t0mpr1c3 commented 1 year ago

The firmware will still need to signal to the desktop software when a row has been completed, and the desktop will still need to acknowledge and send a new row of data (except when starting the last row). But the firmware will not wait for this acknowledgement.

Currently the end-of-line firmware beep is blocking (due to delay() -- see #122) and that will need to change. Just changing this might make the end-of-line delay a lot shorter and less annoying, so let's do that first.

If the desktop acknowledgement lags for any reason then both the visual prompt (row number) and desktop end-of-line sound will be late.

t0mpr1c3 commented 1 year ago

Regarding changes to the API, I think the way to go is to send the first line of data along with the reqStart message and for each reqLine to request one row ahead of the line number requested. The final reqLine still needs to happen to let the desktop know that the penultimate line has finished, even though the corresponding cnfLine response will contain no data.

t0mpr1c3 commented 3 months ago

After #193 this change will require the buffer to be enlarged.

X-sam commented 3 months ago

I don't think this change is necessary anymore, after #194 you can knit as fast you can move the carriage.

jonathanperret commented 3 months ago

As @X-sam says it appears delays are no longer as necessary as they once were (I think https://github.com/AllYarnsAreBeautiful/ayab-desktop/pull/679 is the main reason though, #194 is mainly about taking advantage of the reduced latency).

The idea of introducing some safety margin in the communication still makes sense to me, though. I don't think it necessarily requires any change to the protocol though, at least not in the format or size of the messages.

My suggestion would be to have the firmware maintain two row buffers at any time: the one being knit from (driving solenoids as the carriage moves), and the one that can be updated by receiving cnfLine messages; in a very similar fashion to how GPUs usually have a "front buffer" that is currently displayed and a "back buffer" that receives updates.

Updates would then only be required to the timing of the reqLine/cnfLine messages:

  1. As soon as the firmware receives reqStart it sends a reqLine to get its "back buffer" filled with the first row's contents.
  2. Whenever a new line is started (at the first pass in front of the turnaround mark for the first line, and when carriage turns around for subsequent lines), the firmware copies its "back buffer" to its "front buffer", and emits a reqLine to get its "back buffer" filled with the next row's contents.

Note how in this setup, the firmware gets a full line of carriage movement to receive the next row's contents, compared to currently where relatively tight timing is still required to make sure the row data is here between the time the carriage turnaround is detected, and when the carriage's new trailing edge starts selecting needles.

It doesn't look to me like this suggested timing change requires any change to the message formats. There may be an impact on the desktop app's presentation of which of the pattern's rows is currently being knit, since as far as I can tell, it currently relies on the reception of reqLine messages to learn how much the carriage has already knit. This could be updated to take into account the fact that lines are requested in advance, but it might be even better to separate these concerns and have the firmware emit more indState messages to inform the app of its progress — up to the "continuous reporting" that was attempted but backed out of because it messed with the exacting timing requirements of the current setup.

jonathanperret commented 3 months ago

Forgot to add: despite thinking double-buffering rows would be a worthwhile improvement, I fully support moving this issue out of the 1.0 milestone.

jonathanperret commented 1 month ago

Moved out of 1.0.0 milestone.