Serial: Tx use FIFO plus fixes #90
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
per1234
added
type: enhancement
|
I have done some test runs with WIFI code as well as BLE and this morning they ran. So probably time to start the review process |
|
Memory usage change @ 4cd4a58
Click for full report table
Click for full report CSV |
This is a reasonably major update to the Serial(UART) code base, which is mostly concentrated on the Output(TX) side of it. Simple part of the description: The currently released code, did not use the FIFO object that was built into the Serial object. Instead it would output one character at a time and then wait for a callback from the lower level code base, which could be TDRE or TEND (mostly likely TDRE) before it would return from the call. There were several other methods that did not work or were not implemented including: flush(), availableForWrite(). Also the write with buffer and count did not work as it did not have the same parameters as the one in the Print class, so the print class simply enumerated each byte. **More Complex stuff:** **Write Call** I added a 16 byte buffer to the UART class that I use to pass down to the R_SCI_UART_Write calls, with as much data as I have up to 16 from the write method if a write is not currently in process. If not active and the FIFO is empty. I detect that and bypass using FIFO up to the first 16 characters. If write is active, the new data is added to the FIFO. **UART_EVENT_TX_DATA_EMPTY** Note: I mention TDRE here, but also cases for FIFO which are different. When lower level completes the write, it will do a callback with the event: UART_EVENT_TX_DATA_EMPTY. First try was to simply grab what is available up to 16 bytes and recall R_SCI_UART_Write with that. But that can lose data, as the callback is called when the ISR code has put the last byte into the TDR register, and the Write call always tries to put the first character out, which if the TDRE state is not set, will lose the last byte. Found I needed to handle two cases: 1) If TDRE I call the write function again. 2) if not TDRE, I update the Buffer/count member variables and reset which interrupt events which will happen. **TIMING** I was still running into conditions where either data was lost or the transfers did not work correctly. RingBuffer: Found in several cases we were spending too much time in the ISR and we could lose interrupts on either UARTS or the same UARTS but different Interrupt. Found a lot of the time was spent in the SafeRingBufferN code, where it creates a class object which disables all interrupts and maybe reenables them at the end. This is needed as the API/ RingBufferN code is inherently unsafe to use here. I created my own versions of these classes that make the RingBuffer class safer such that the Safe version could be a minimal subset. That decreased the overhead a lot. FIFO Size: When using one of the UARTS with FIFO, we filled the RX before any was returned. So the ISR might need to process 16 characters, which took awhile. I have it currently set to 8. I updated all of the Variants to allow each to set their preference. **RX** As mentioned above, when we receive bytes and the ISR is triggered, it calls our callback with the UART_EVENT_RX_CHAR event. For simple double buffer UARTS, we simply store one character away. However for FIFO UARTS, it will callback to us one time for each character in the FIFO. Which before took a lot of time. As for character it would setup the callback, call it, where we disabled the interrupts... The new cod instead extracts the remaining characters from the FIFO and stuffs them directly back into our fifo queue. To keep from having to look each time at, am I running on a FIFO queue or not, I have two different callbacks, so they don't have to keep asking. **DEBUG CODE** Currently I have a fair amount of debug code built into the Serial class, to help debug, including fast functions to set/clear/toggle digital pins as well as to maybe save some state information. I have hopefully all of this under #ifdef
KurtE
force-pushed
the
SerialX_use_write_fifo
branch
from
4cd4a58
to
e0c50a9
Compare
this includes a lot of debug code. A better implementation for ringbuffers
pennam
pushed a commit
to pennam/ArduinoCore-renesas
that referenced
this pull request
Jan 5, 2024
Storage: LittleFs example update
cristidragomir97
pushed a commit
to cristidragomir97/ArduinoCore-renesas
that referenced
this pull request
May 20, 2024
Storage: LittleFs example update Former-commit-id: 2a9b446
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE:
I started this as a draft PR as it is a reasonably complex change. Difficult to split into smaller simpler changes, as for example simply fixing the method: write to have the const ... Will end up causing data to be lost.
I am still testing different test sketches, The last run I did was to test USB To Serial sketch that I have setup as a loop back (tie the RX to TX pin). Worked fine for both MINIMA and WIFI on either Serial1 or the Serial object that you can create on SCL/SDA pins which uses FIFO.
Thought it would be good to give everyone a heads up, to see if anyone has some preliminary comments.
Resolves: #44
This is a reasonably major update to the Serial(UART) code base, which is mostly concentrated on the Output(TX) side of it.
Simple part of the description:
The currently released code, did not use the FIFO object that was built into the Serial object. Instead it would output one character at a time and then wait for a callback from the lower level code base, which could be TDRE or TEND (mostly likely TDRE) before it would return from the call. There were several other methods that did not work or were not implemented including: flush(), availableForWrite().
Also the write with buffer and count did not work as it did not have the same parameters as the one in the Print class, so the print class simply enumerated each byte.
More Complex stuff:
Write Call
I added a 16 byte buffer to the UART class that I use to pass down to the R_SCI_UART_Write calls, with as much data as I have up to 16 from the write method if a write is not currently in process. If not active and the FIFO is empty. I detect that and bypass using FIFO up to the first 16 characters. If write is active, the new data is added to the FIFO.
UART_EVENT_TX_DATA_EMPTY
Note: I mention TDRE here, but also cases for FIFO which are different. When lower level completes the write, it will do a callback with the event: UART_EVENT_TX_DATA_EMPTY. First try was to simply grab what is available up to 16 bytes and recall R_SCI_UART_Write with that. But that can lose data, as the callback is called when the ISR code has put the last byte into the TDR register, and the Write call always tries to put the first character out, which if the TDRE state is not set, will lose the last byte. Found I needed to handle two cases:
TIMING
I was still running into conditions where either data was lost or the transfers did not work correctly.
RingBuffer:
Found in several cases we were spending too much time in the ISR and we could lose interrupts on either UARTS or the same UARTS but different Interrupt. Found a lot of the time was spent in the SafeRingBufferN code, where it creates a class object which disables all interrupts and maybe reenables them at the end. This is needed as the API/ RingBufferN code is inherently unsafe to use here. I created my own versions of these classes that make the RingBuffer class safer such that the Safe version could be a minimal subset. That decreased the overhead a lot.
FIFO Size:
When using one of the UARTS with FIFO, we filled the RX before any was returned. So the ISR might need to process 16 characters, which took a while. I have it currently set to 8. I updated all of the Variants to allow each to set their preference.
Interrupt Priority:
I setup to Interrupt code to allow us to set a different priority for the RX ISR versus the TX ISR. As if a TX ISR is delayed it
simply could mean we delay when the next characters output. However, if we an RX interrupt is delayed, and this
is a double buffer port like Serial1 and Serial on WIFI, then if it is delayed the amount of time it takes to receive the next
character, a character will be lost with the overflow condition set. I have the RX now set at a higher priority (lower value)
RX
As mentioned above, when we receive bytes and the ISR is triggered, it calls our callback with the UART_EVENT_RX_CHAR event. For simple double buffer UARTS, we simply store one character away.
However for FIFO UARTS, it will callback to us one time for each character in the FIFO. Which
before took a lot of time. As for character it would setup the callback, call it, where we
disabled the interrupts...
The new cod instead extracts the remaining characters from the FIFO and stuffs them directly back into our fifo queue.
To keep from having to look each time at, am I running on a FIFO queue or not, I have two
different callbacks, so they don't have to keep asking.
DEBUG CODE
Currently I have a fair amount of debug code built into the Serial class, to help debug, including fast functions to set/clear/toggle digital pins as well as to maybe save some state information. I have hopefully all of this under #ifdef