Improve VIRTSER performance #7528
Conversation
|
This does affect steno, as it uses VIRTSER. Are you using this with Steno? Eg, has it been tested and verified that it doesn't break the steno implementation? |
|
No, I'm using it with a custom virtser_recv handler. I cannot check steno since I don't have the hardware. However, the chibios implementation of virtser does call it in a loop here: |
|
@germ can you verify this PR does not break steno, this would affect your boards more than others. |
|
I can test in about a week or so, I'm out of the country and didn't bring a laptop with me. I'll dump it in the Plover discord and see if someone can verify nothing breaks. |
|
Awesome, thanks, @germ. |
Wait, they let you out of Canada? |
|
Hey! I'm not Australian or anything, don't get us confused :P |
|
Hey! 👋 (I'm from the plover discord) I just tested this with the georgi's default firmware, and everything seemed to work okay. I should say that I don't normally use a virtser-based protocol for steno, but I checked the behavior of this PR against master and didn't find any differences. As far as I know, the georgi uses the Gemini protocol, which means that TX Bolt is still untested, so if you want, I could try that as well. (On the other hand, the steno implementation doesn't even seem to use |
There was a problem hiding this comment.
Looks okay to me, ChibiOS's implementation does something similar already.
There's no CDC_Device_ReceiveByte equivalent for a buffer in LUFA, so normalising the implementations to match the ChibiOS one isn't really possible. Seems like this is as close as we're going to get.
| @@ -869,7 +869,7 @@ void virtser_recv(uint8_t c) { | |||
| void virtser_task(void) { | |||
| uint16_t count = CDC_Device_BytesReceived(&cdc_device); | |||
| uint8_t ch; | |||
| if (count) { | |||
| for (; count; --count) { | |||
| ch = CDC_Device_ReceiveByte(&cdc_device); | |||
There was a problem hiding this comment.
Hm, possibly a small bug here: the return type of CDC_Device_ReceiveByte() is int16 and will return -1 if no data was received.
https://www.fourwalledcubicle.com/files/LUFA/Doc/170418/html/group___group___u_s_b_class_c_d_c_device.html#gaf02a74dffdcde55f4e522989e2ed49c1
It's probably fine, due to the for loop on count - when 0, ReceiveByte() will not be called.
Description
Improve performance of virtser. Currently it consumes 1 byte at a time, which took for me over 30ms when sending 49 bytes. With a simple change, it can send 10x that in 30ms.
Types of Changes
Issues Fixed or Closed by This PR
Checklist