CNMAT / OSC

OSC: Arduino and Teensy implementation of OSC encoding
cnmat.berkeley.edu/oscuino
Other
727 stars 135 forks source link

Teensy buffer overflow being ignored #146

Closed geocom closed 1 month ago

geocom commented 2 months ago

Been working on a project now for a bit and been having issues with commands not being completed until more data flows in.

The project is an ETC EOS encoder board programming is around the softkeys, 12 OSC strings that are sent quickly one after another the data is there it just wont show as available.

I now have finally tracked down the the issue and its with how the Teensy reports the number of bytes available when the buffer has gone beyond one packet.

Which leads me to the fix. Teensy after you have read the first chunk of data available will report a < 0 and as such it cant continue reading data. However calling peek will not report a -1 it will return the next byte of data you can continue to read the bytes this way until a -1 is reported or a 255 and this seems to be the only way to effectively return all the data in these instances as with the current code the next packet sent from the computer will set the buffer size to the value of that packet so data just keeps on backing up and not clearing.

Below is the code I changed to SLIPSerial.

92 uint8_t cnt = serial->available(); changed to uint8_t cnt; uint8_t peek = serial->peek(); if (peek == -1 || peek == 255){ cnt = 0; }else{ cnt = 1; }

77 if(serial->available()) changed to if(serial->peek() >= 0)

I can submit a pull request but want to check first if your happy for this as is or if you would like some defines to make this only work on teensy or another way entirely.

adrianfreed commented 2 months ago

I am confused. -1 is not a valid value for an unsigned int. Also 255 is a potentially valid character in a slip stream so I can't be used this way.


From: Geoff Evans @.> Sent: Thursday, April 25, 2024 10:35 PM To: CNMAT/OSC @.> Cc: Subscribed @.***> Subject: [CNMAT/OSC] Teensy Large buffer overflow being ignored (Issue #146)

Been working on a project now for a bit and been having issues with commands not being completed until more data flows in.

The project is an ETC EOS encoder board programming is around the softkeys, 12 OSC strings that are sent quickly one after another the data is there it just wont show as available.

I now have finally tracked down the the issue and its with how the Teensy reports the number of bytes available when the buffer has gone beyond one packet.

Which leads me to the fix. Teensy after you have read the first chunk of data available will report a < 0 and as such it cant continue reading data. However calling peek will not report a -1 it will return the next byte of data you can continue to read the bytes this way until a -1 is reported or a 255 and this seems to be the only way to effectively return all the data in these instances as with the current code the next packet sent from the computer will set the buffer size to the value of that packet so data just keeps on backing up and not clearing.

Below is the code I changed to SLIPSerial.

92 uint8_t cnt = serial->available(); changed to uint8_t cnt; uint8_t peek = serial->peek(); if (peek == -1 || peek == 255){ cnt = 0; }else{ cnt = 1; }

77 if(serial->available()) changed to if(serial->peek() >= 0)

I can submit a pull request but want to check first if your happy for this as is or if you would like some defines to make this only work on teensy or another way entirely.

— Reply to this email directly, view it on GitHubhttps://github.com/CNMAT/OSC/issues/146, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMMDDEQG4CKWZ7DGUH3KRLY7HRT7AVCNFSM6AAAAABG2EKPRSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI3DIOJZG44DMMQ. You are receiving this because you are subscribed to this thread.Message ID: @.***>

geocom commented 2 months ago

https://www.arduino.cc/reference/tr/language/functions/communication/serial/peek/ "The first byte of incoming serial data available (or -1 if no data is available) - int"

My guess would be 255 is -1 as its overflowed back to 255 however this is how the data is being received.

adrianfreed commented 2 months ago

yes 255 is -1 interpreted as an 8-bit unsigned int but your code isn't robust if the sent values include 255 as a value in the OSC message.


From: Geoff Evans @.> Sent: Friday, April 26, 2024 3:47 PM To: CNMAT/OSC @.> Cc: adrian adrianfreed.com @.>; Comment @.> Subject: Re: [CNMAT/OSC] Teensy buffer overflow being ignored (Issue #146)

https://www.arduino.cc/reference/tr/language/functions/communication/serial/peek/ "The first byte of incoming serial data available (or -1 if no data is available) - int"

My guess would be 255 is -1 as its overflowed back to 255 however this is how the data is being received.

— Reply to this email directly, view it on GitHubhttps://github.com/CNMAT/OSC/issues/146#issuecomment-2080205716, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMMDDF32JWZ47G5W5OYESLY7LKPHAVCNFSM6AAAAABG2EKPRSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBQGIYDKNZRGY. You are receiving this because you commented.Message ID: @.***>

geocom commented 2 months ago

Ok have only been testing with strings thus far. Better option then would be to do this at 92

uint8_t cnt; if (serial->peek() == -1){ cnt = 0; }else{ cnt = 1; }

adrianfreed commented 2 months ago

Does this work for you? https://github.com/CNMAT/OSC/pull/147

I am not set up to test this but I am attempting to address the problem in a way that works for both Teensy and other devices

geocom commented 2 months ago

Yes that has resolved the issue. Thanks for the quick fix.