Due HardwareSerial Transmit not interrupt-driven...

[WestfW]

I would have expected that the Due Serial ports would be interrupt driven for both transmit and receive, just like the AVR serial ports. But in fact the Due transmit code (UARTClass::write and USARTClass::write uses busy-loops:

size_t USARTClass::write( const uint8_t uc_data )
{
// Check if the transmitter is ready
while ((_pUsart->US_CSR & US_CSR_TXRDY) != US_CSR_TXRDY)
;

// Send character
_pUsart->US_THR = uc_data ;
return 1;
}

[MarkTillotson]

Also those two classes are basically identical since the hardware registers for UART and USART peripherals deliberately have exactly the same layout for asynch use.

The USARTClass could be a shim inheriting from UARTClass

[MarkTillotson]

Also having looked at those classes in detail there are a number of little issues,
parity errors are not cleared, end() fails to wait for the last byte to drain,
interrupts are enabled in UART_IER before the NVIC_EnableIRQ call (specifically warned against in the datasheet 35.4.3). Also interrupts only
need enabling for RXRDY and OVRE since PARE and FRAME are set
simultaneous to RXRDY.

[robertstuart]

I would second these comments. On my current project the code broke when I ported it to the Due since it expected an immediate return from a serial write. I hacked a solution by creating my own buffer and writing a byte out every 100 uSec or so--a really ugly hack. Is there anything we can do to get this on the to-do list? Beg? Money? Threats? Cajoling?

[PaulStoffregen]

I'm curious about your code that broke without the buffering. Is it available somewhere?

[robertstuart]

No. I probably have it archived somewhere but I simply had my program in a tight loop that needed to respond to a condition within 50 uSec or so. A blocking write obviously made this impossible. I have since made both hardware and software changes that remove this restriction. Still, a non-blocking write would be nice. The other Arduino platforms have it as do all operating systems so I think it is reasonable to expect it to be available on the Due.

From: Paul Stoffregen [mailto:notifications@github.com]
Sent: Thursday, December 04, 2014 4:53 PM
To: arduino/Arduino
Cc: robertstuart
Subject: Re: [Arduino] Due HardwareSerial Transmit not interrupt-driven... (#2271)

I'm curious about your code that broke without the buffering. Is it available somewhere?

—
Reply to this email directly or view it on GitHub #2271 (comment) . https://github.com/notifications/beacon/AEH6_pTJC5aYIyvK7vTgkYQgmLNr8PFXks5nUOtWgaJpZM4CcVT1.gif

[collin80]

I'm actually very disappointed and perplexed that the Due serial code is so poor. The processor has DMA for Heaven's sake and we're sitting in a spinlock?!? I guess this was very early code to get it up and running and nobody ever fixed it. There is no excuse for this on a processor with so many resources and so much RAM. I think the core can afford to allocate a 128 byte buffer for serial transmission. I'll see about fixing it for myself and try to push it back to the main repo.

[robertstuart]

It is good to see that someone else is equally disappointed and perplexed. I have code that reads and IMU 400 time/sec (2500 uSec) . The IMU takes about 2000 uSec per read. The processor then sends out status information to an XBee for each read. This all works perfectly well on an Arduino Uno. This, of course doesn’t work on the Due since the SerialX.write() blocks and causes the next IMU read to be missed. There are several hacks to get around this. The one that I am using at the moment is to have a timer interrupt every 200 uSec or so which then does a single byte SerialX.write() to empty my own write buffer. This works just fine but it seems very ugly to move to another processor, which is roughly 10 times as fast and then have to resort to a hack like this to get it to perform adequately.

Good luck. If you need help making noise about this or help testing your code, please count me in.

From: Collin Kidder [mailto:notifications@github.com]
Sent: Saturday, December 20, 2014 11:06 AM
To: arduino/Arduino
Cc: robertstuart
Subject: Re: [Arduino] Due HardwareSerial Transmit not interrupt-driven... (#2271)

I'm actually very disappointed and perplexed that the Due serial code is so poor. The processor has DMA for Heaven's sake and we're sitting in a spinlock?!? I guess this was very early code to get it up and running and nobody ever fixed it. There is no excuse for this on a processor with so many resources and so much RAM. I think the core can afford to allocate a 128 byte buffer for serial transmission. I'll see about fixing it for myself and try to push it back to the main repo.

—
Reply to this email directly or view it on GitHub #2271 (comment) . https://github.com/notifications/beacon/AEH6_pdwjhQO61iymvrBaJfkxtsMJ1qOks5nZbHvgaJpZM4CcVT1.gif

[collin80]

Yesterday I got TX interrupts working for the USART class. It seems to work fine. The only question is how to handle the buffer being full. The two choices are to drop extra characters or to spinlock (like we do now) until the TX buffer has an opening. I have chose the second option for now as it is more novice friendly. I'm contemplating also supporting DMA based transmission of buffers. This allows larger transmissions to be done without blocking. The default buffer size for Arduino RingBuffer objects is 64 bytes so right now my implementation has a 64 byte TX buffer.

[WestfW]

The Due TX should do the same thing as the AVR-based uarts (block when buffer full.)
(Did you implement the availableForWrite API as well?)

DMA for uart-like devices is tricky, and I'm not sure that it's worthwhile give the other APIs for serial (and in particular, the lack of an "actually start output" call, along with the "small" sizes of the average write.

[cmaglie]

@collin80
thank you for looking into this one.

If the buffer is full the write must block until transmission is completed.
We recently provided a method Serial.availableForWrite() that returns the number of bytes that can be transmitted without blocking.

[robertstuart]

I agree that the write should block if the buffer is full. Most working code expects it since that is how it currently works on other platforms. In my own case, dropping characters has serious consequences since it breaks the protocol. BTW, it might not hurt to bump the standard buffer size up a little. The Due has (relatively) a lot of SRAM.

it will be great to see this code up and working. Thanks.

[collin80]

I have it working pretty well in the USART class and I've been testing it over these past couple of days. I will commit my changes to my fork soon and then discuss doing a pull request. As per Robert's suggestion I also increased the space used by RingBuffer to 128 characters instead of 64. The Due does have a huge amount of RAM (for a little processor) and the extra space allocation only adds up to less than an extra kilobyte of RAM usage.

And now for the more contentious suggestion. Currently there is exactly zero real difference between the UART and USART devices on the Due. We configure them exactly the same, the code is exactly the same. The data registers used are exactly the same. Does it make any sense to implement everything twice just to change what everything is called subtly but not the actual memory offsets.

Take these two lines:
_pUart->UART_CR
_pUsart->US_CR

On these lines UART_CR = US_CR. The only difference is what we call them. The USART uses the same configuration space layout as the UART it just has extra stuff afterward. So, I see no reason that we couldn't implement everything once. Personally I'd go with using the USART class for all hardware serial. That way if more features are ever added they can be added and all that will happen is that the UART hardware will ignore anything sent to the configuration space it doesn't use. I don't see any harm in this. The UART will still work as a UART this way but won't require special code to support it.

[collin80]

I just created a pull request that satisfies this issue: #2511

RingBuffer has been increased to 128 bytes, UART and USART classes merged (they did basically the same thing anyway) and transmit interrupts implemented. I've tested a USART port to still work reliably so presumably the rest do as well but further testing wouldn't hurt.

[methos10019]

I tested your modifications and found a little bug. If you do a write when the hardware buffer is empty and the soft buffer is not, the write is done immediately and not put in the soft buffer. This result in bytes not send in the good order.
I changed:
if ((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY)) //is the hardware currently busy?

by:
if (((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY) | (_tx_buffer->_iTail != _tx_buffer->_iHead)) //is the hardware currently busy?

and everything is send in order.

[collin80]

You're right. I have made your changes and pushed so the pull request
should now show your proposed change. Thank you.

On Tue, Dec 30, 2014 at 10:05 PM, methos10019 notifications@github.com
wrote:

I tested your modifications and found a little bug. If you do a write when
the hardware buffer is empty and the soft buffer is not, the write is done
immediately and not put in the soft buffer. This result in bytes not send
in the good order.
I changed:
if ((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY)) //is the hardware
currently busy?

by:
if (((_pUart->UART_SR & UART_SR_TXRDY) != UART_SR_TXRDY) |
(_tx_buffer->_iTail != _tx_buffer->_iHead)) //is the hardware currently
busy?

and everything is send in order.

—
Reply to this email directly or view it on GitHub
#2271 (comment).