Non-blocking reads#104
Conversation
and remove _most_ blocking reads
| #define UART_CTS 14 // The ESP32 CTS pin (eZ80 RTS) | ||
|
|
||
| #define COMMS_TIMEOUT 100 // Timeout for VDP commands (ms) | ||
| #define COMMS_TIMEOUT 200 // Timeout for VDP commands (ms) |
There was a problem hiding this comment.
timeout here has got a little bit longer. on testing out Greg's snake game we found that the transfer for the very last bitmap would timeout when this was 100ms. increasing to 200ms appears to be much safer.
our speculation is that on rare occasions SD card reads tip the latency just over 100ms. difficult to know for certain. this just makes timeouts incredibly unlikely for genuine transfers.
| for (auto n = 0; n < length; n++) sample->data[n] = readByte_b(); | ||
| auto remaining = readIntoBuffer((uint8_t *)data, length); | ||
| if (remaining != 0) { | ||
| // Failed to read all data | ||
| debug_log("vdu_sys_audio: sample %d - data discarded, failed to read all data\n\r", sampleIndex); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
all "buffer" reads now use this kind of new technique. they can all now timeout.
it is assumed that if a timeout has occurred then no more data for that buffer will come, so there is no need to discard any more bytes.
| uint32_t color = readLong_b(); | ||
| uint32_t color; | ||
| auto remaining = readIntoBuffer((uint8_t *)&color, sizeof(uint32_t)); | ||
| if (remaining > 0) { | ||
| debug_log("vdu_sys_sprites: failed to receive color data\n\r"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
this is a non-blocking equivalent of the old readLong_b - tolerant of bytes going missing
| // which should be zero if all bytes were read | ||
| // but will be non-zero if the read timed out | ||
| // | ||
| uint32_t VDUStreamProcessor::readLong_b() { |
There was a problem hiding this comment.
the block readLong is no longer used anywhere, so it's been deleted.
| if (available > remaining) { | ||
| available = remaining; | ||
| } | ||
| // debug_log("readIntoBuffer: reading %d bytes\n\r", available); |
There was a problem hiding this comment.
for those curious, in my tests typically the VDP will process blocks of about 66 bytes at a time.
| if (remaining < readSize) { | ||
| readSize = remaining; | ||
| } | ||
| if (readIntoBuffer(buffer.get(), readSize, timeout) != 0) { |
There was a problem hiding this comment.
no need to check for timeouts explicitly within discardBytes - if this read fails to grab all the bytes we've asked for then we've timed out.
|
an added bonus of this PR is that I'd experienced some strange issues with the new version of the Arduino IDE with reads on the comms channel stalling. this revised approach, coupled with using the latest version of the ESP board manager (2.0.13) appears to fix that issue. the new board manager version offers improved comms performance and leaves enough free internal memory for all the screen modes to work (which 2.0.11 didn't). |
Updates almost all comms code between the z80 and VDP to be non-blocking, and to timeout.
(The only blocking read commands that remain are those inside
hexload.h)Adds a
readIntoBuffercall, which is used for all places that read data from the VDU stream into a buffer. This call is given a buffer pointer and the number of bytes we're expecting to read, and will return zero when the buffer has been read successfully, or if the read times out the number of bytes that were missing. The timeout is based on the time from when data was last received, rather than when the call was made. This way larger transfers shouldn't timeout unless there is a genuine delay.The performance of
readIntoBufferin testing is identical to @astralaster's earlier comms performance enhancement code. (It uses a very similar technique.)discardByteshas also been updated to be non-blocking.A great plus with these changes is that the VDP will no longer hang waiting for data that will never come on dodgy/flakey VDU calls. For example, attempting to use an "upload bitmap" or "upload sample" call from the CLI often/usually results in a command that doesn't include all the expected data. (Sending a byte instead of a word for one of those calls typically results in the VDP expecting far more data than the user intended.) When this happens the VDP would be left in a state waiting for data that could never come, requiring the user to press the "reset" button to recover.
With these changes the command times out, the data will be discarded, and the computer is usable again.
(NB typically when such faulty commands are sent, the VDU system will also be sent other data such as the
>prompt in BASIC, which will be captured as part of the data stream. If the user has sent a duff command from the BASIC CLI they'll be left on an empty line with no visible prompt, but they can enter commands.)