Does this code need interrupts disabled?

[ejeffrey]

Can I just ask some specific questions:

Is either of these methods ok? I know the 2nd option is, but I haven't yet sorted out a "DI/EI"

It's hard to say without seeing the rest of your code, but I think that if you:

1) mark the buffer and put_index variables 'volatile' or in some other way implement a compiler level barrier to prevent re-ordering,
2) follow the "only one task/interrupt reads and only one task/interrupt writes" restriction

then I think either works.

Note that the DI() / EI() functions *also* act as compiler barriers.  They have to, to keep the compiler from moving "protected" code to outside the critical section.

Saving the index update to the end as suggested by Sauli is also fine.  It will also save some unnecessary stores to the volatile variable, which is also nice.  Tthis is a good example of the reason you need volatile.  If you don't specify serial_transmit_buffer_put_index as volatile, it's very likely that the compiler will optimizer will hoist the update out of the loop, either putting the update before or after the loop.  If it goes after the loop that is fine, but if it goes before the loop then the ISR can read invalid data.

Also, all the above commentary is correct: volatile is a *compiler* ordering directive.  It forces the assembly/machine code to execute certain operations in the same order as the designated flow of the C code.  This is fine on (most?) uniprocessor systems: pretty much every CPU architecture guarantees that a single CPU executes code "as if" each instruction is executed sequentially.  In multi-core operations, volatile is basically never useful for inter-thread communication.  You need atomic operations that include CPU memory fences to guarantee that other CPU cores see the accesses in the correct order.  I am assuming you don't have multi-core systems in mind, but there are multi-core microcontrollers so it is certainly not something everyone can ignore.

[Berni]

In your case the following code will work just fine:

Code: [Select]

// Copy the data to the circular buffer
uint32_t index = serial_transmit_buffer_put_index[--port]; // offset from 0
for (uint16_t i = 0; i < length; i++)
{
serial_transmit_buffer[port][index++] = data[i];
index %= SERIAL_TX_BUFFER_SIZE;
}
serial_transmit_buffer_put_index[port] = index;
The code reading the data from the buffer does not need to know that there is new data in the buffer until it is all there.

Yep this is also how i would recommend to do it.

The whole idea is that until you move the write pointer over then the other thread will not try to access the extra data in the buffer, since it doesn't even know the data is there. So this means you are guaranteed to have time to finish filling the buffer. Then as the last step updating the write pointer will let the other thread know the data is there and ready to be used.

The limitation here is that you are not allowed to call this function from multiple threads. If you do there is a possibility a thread switch happens during this function, leaving the buffer half written, then another thread calling the function again tramples over the half written data. The solution for this is placing the RTOS critical section around it, preventing a task switch from happening during the writing.(Not a problem since the function finishes quickly)

Disabling interrupts is a more heavy handed method. This prevents all interrupts from executing and so wrecks the timing predictability of interrupts. This could cause a buffer overflow in something like a UART driver because it couldn't empty the receive buffer quickly enough. Also the RTOS makes use of interrupts in a few places.

As for the volatile keyword not doing the job. You can have such cases even in single core MCUs if you make use of DMA. Those typically just need a few cache invalidation calls in the right spots or disabling caching for the region of memory where the DMA keeps its buffers. But that doesn't seam to be relevant to OPs case.

[peter-h]

The limitation here is that you are not allowed to call this function from multiple threads

It gets used from different threads but with a different buffer index. This is for four serial ports (plus a USB VCP) and it is a bit nonsensical for different RTOS tasks to be both using a given UART (well, I've documented to not do that, although one task could do TX and another do RX quite reasonably).

The whole idea is that until you move the write pointer over then the other thread will not try to access the extra data in the buffer, since it doesn't even know the data is there

I did wonder about this but could not get my head around some dependencies. Let me post more detail. I have one RX function and one TX function. I am seeing an occassional loss of 1 byte by the TX function, so a 5000 byte block sent out actually sends out 4999 (sometimes 4998) out of the UART. Obviously there are multiple ways to debugging this, like transmitting the same block repeatedly and looking at the data with a digital scope, but it still takes me back to the code driving the UART.

First I have taken on board but it has not changed anything:

Code: [Select]

// Actual buffer size is SERIAL_RX_BUFFER_SIZE-1 in order to allow buffer full/empty distinction
static uint8_t serial_receive_buffer[SERIAL_MAX_PORT][SERIAL_RX_BUFFER_SIZE];
static uint8_t serial_transmit_buffer[SERIAL_MAX_PORT][SERIAL_TX_BUFFER_SIZE];

volatile static uint32_t serial_receive_buffer_rx_index[SERIAL_MAX_PORT] = { 0, 0, 0, 0 };
volatile static uint32_t serial_receive_buffer_get_index[SERIAL_MAX_PORT] = { 0, 0, 0, 0 };
volatile static uint32_t serial_transmit_buffer_tx_index[SERIAL_MAX_PORT] = { 0, 0, 0, 0 };
volatile static uint32_t serial_transmit_buffer_put_index[SERIAL_MAX_PORT] = { 0, 0, 0, 0 };
volatile static bool serial_transmit_active[SERIAL_MAX_PORT] = { false, false, false, false };

This is the transmit function. It is fairly standard. Put the data into the circular buffer and if TX is not active, put 1 byte of it into the UART and kick off the interrupt-driven transmit.

Code: [Select]


/*
* Write a number of bytes to the transmit queue and start transmission if necessary
* Returns false if there is insufficient space for the given length
* Port is numbered 0 to 4, with 0 being the USB VCP
*
* The port 0 processing is different for port 0 because there is flow control on tx (to PC)
* which necessitates a circular buffer before it just like ports 1-4 have. The difference
* is that with serial ports the flow control is the baud rate whereas with port 0 is it
* how fast the PC application is reading the data out of the VCP.
*
* This function is NON blocking, if the data fits into the TX buffer, otherwise no action.
* However, it delays for the time g_485_DE_wait if the port is using RS485 mode (not always). Note
* that interrupts are disabled during this time!
*
*/

bool serial_transmit(uint8_t port, uint8_t *data, uint16_t length)
{
bool ok = false;

if (port > 0 && port <= SERIAL_MAX_PORT)
{
if (serial_get_opqspace(port) >= length)
{

// Copy the data to the circular buffer
port--; // offset from 0
for (uint16_t i = 0; i < length; i++)
{
#if 1
uint32_t index = serial_transmit_buffer_put_index[port];
serial_transmit_buffer[port][index++] = data[i];
serial_transmit_buffer_put_index[port] = index % SERIAL_TX_BUFFER_SIZE;
#else
__disable_irq();
serial_transmit_buffer[port][serial_transmit_buffer_put_index[port]++] = data[i];
serial_transmit_buffer_put_index[port] %= SERIAL_TX_BUFFER_SIZE;
__enable_irq();
#endif
}

__disable_irq();
if (!serial_transmit_active[port])
{

// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts
// First clear TC. The read of SR (RMW actually) and a write to DR also clears pending TC int.
serial_port_handles[port]->Instance->SR &= ~(UART_FLAG_TC);
serial_port_handles[port]->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
serial_transmit_active[port] = true;

// Enable TX empty and TX complete interrupts
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TXE);
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TC);
}
__enable_irq();

ok = true;
}
}
else if (port == 0)
{
// USB CDC port. Same as port 1-4 above but no need to mess with interrupts etc
// The circular buffer gets copied to CDC in the USB RTOS task (in main.c)

CDC_serial_transmit(data, length);
ok=true;

}

return ok;
}


This is the transmit ISR

Code: [Select]


void serial_transmit_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

port--; // offset from 0

// Is there anything left to send?
if (serial_transmit_buffer_tx_index[port] == serial_transmit_buffer_put_index[port])
{
// Disable the TXEmpty interrupt
__HAL_UART_DISABLE_IT(huart, UART_IT_TXE);
}
else
{
// Send the next byte
//huart->Instance->SR &= ~(UART_FLAG_TC); // now done only at the start of transmission
huart->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
}

}

I reckon moving the write pointer only at the end should work, and it saves a little bit of exec time. Nothing actually happens at the UART until the whole block has been loaded into the buffer, anyway. And it appears I don't need to disable interrupts around the buffer fill.

[Berni]

Well those are specifics that could be down to an off by 1 error in some counter or loop, or something to do with the UART hardware of placing the data in there at the wrong time. For example you don't actually check if the UART TX buffer is empty before placing it in there. The buggy peripheral hardware on STM micros can be pretty terrible sometimes.

Also i don't think it is sensible to disable global interrupts while configuring your TX interrupt. Typically this is done by simply enabling the interrupt as the very last step. Typically you also disable just that 1 interrupt you are configuring before starting to configure. In your case since you are enabling two it makes sense to enable the TX complete interrupt first, since enabling the TX empty interrupt will likely directly shoot you off into the interrupt handler(since the TX buffer is likely empty at that point), but the TX complete will only fire after you actually transmit something.

[peter-h]

Good points!

There should not be a need to check TXE before placing the byte in, because the TX Complete ISR sets a flag

Code: [Select]


void serial_transmit_complete_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

// Transmit complete
serial_transmit_active[port-1] = false;

// Clear and disable the TC interrupt
//huart->Instance->SR &= ~(UART_FLAG_TC); // Not needed
__HAL_UART_DISABLE_IT(huart, UART_IT_TC);

}

I will continue to investigate. The "not needed" flag above doesn't seem to make much difference either way. One is really probing the peculiarities of the 32F417 chip at this level.

[paulca]

To give you an idea of how far this rabbit whole goes, in Uni we spent a year with Jean Bacon's book on the topic:  https://www.amazon.co.uk/Operating-Systems-Concurrent-Distributed-International/dp/0321117891

But...  "You can get away with more on a single core.", stops being true once you have more than one device communicating.  If you need consistent state between them, then you are back to Jean's book again.

This is why in software engineering for the big distributed, multi-CPU, multi-server architectures, we use CPUs with specifically designed features, specifically designed MCU (Memory control unit) and specifical cache and shared cache mechanics in hardware . It's why we use general purpose OSs with stable efficient IPC like Linux.  It's why we reuse large amounts of code in frameworks, middleware, distributed filesystems, databases, database clusters to take care of the most obvious of concerns.  The only places that would be exclude a lot of this are in custom low latency / real time use cases where optimisation has removed as much of the bloat caused by the normal approach.

Your usecase is all too familar to my own "hobby space" problem.  Only in my case I have 4 or more I2S streams arriving on an SPI bus and I need to mix them together and forward them out another SPI port.  It's callback and buffer soup.   How do you handle a late buffer, how do you handle creeping buffers.  If your buffer is 1ms and your streams arrive at 100uS, 500uS, 700uS, 900uS...  there isn't enough time left to mix, process and send on before something somewhere runs out of data.  I'm still trying to decide if I should actually "fix" anything.  If the steam alignment like that does occurr, the easiest solution to fix it is to just ignore the 700uS and 900uS frames as being "late" and pick them up next time.  The output buffer has an audible "CLICK" of 1ms silence and the buffers all align next time.  I'm resisting the need to do any really synchronisation just by being quick.  (Note buffer content alignment is not required, buffers can slip 1ms if they must just not very often!).

[wek]

I am seeing an occassional loss of 1 byte by the TX function, so a 5000 byte block sent out actually sends out 4999 (sometimes 4998) out of the UART.

Show us the ISR.

JW

[paulca]

One thought when scanning your code.  Appologies of this is miss placed.

You seem to be moving singular "circular" buffers around and/or moving data between them.  It might be advantagous to move the multiplicity and circularity up one level.

So instead of "a" circular buffer for a port Rx/Tx use a queue of buffers.  Obviously heavy on SRAM, but it makes the synchronisation easier with it being less finely grained.  Instead of a circular array buffer, have an circular array OF buffers.

In similar Rx/Tx In/Out etc. naming I prefer to use the term "readyBuffer" or "freeBuffer".  Because all too often your Rx buffer in one function immediately becomes your Tx buffer in another.  This adds unneeded confusion.  "readyBuffer", such as in

whlie( readyBuffer == NULL ) {
         readyBuffer = ports->getReadyBuffer();
}
// blah blah blah
ports->releaseBuffer(readyBuffer);

Then you can just expand the buffers/queues horizontally or vertically.  More buffers or longer buffers etc.

You can have a queue per port, or a combined queue for all ports and a struct which keeps track.  Have a queue per output port as well.  Then the main code is just routing synchronised queue pops and pushes.  All the interrupt / IPC stuff is wrapped in your queue / buffer handling functions, your main code doesn't need to know.

[peter-h]

Show us the ISR.

Code: [Select]


void serial_transmit_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

port--; // offset from 0

// Is there anything left to send?
if (serial_transmit_buffer_tx_index[port] == serial_transmit_buffer_put_index[port])
{
// Disable the TXEmpty interrupt
__HAL_UART_DISABLE_IT(huart, UART_IT_TXE);
}
else
{
// Send the next byte
//huart->Instance->SR &= ~(UART_FLAG_TC); // now done only at the start of transmission
huart->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
}

}


void serial_transmit_complete_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

// Transmit complete
serial_transmit_active[port-1] = false;

// Clear and disable the TC interrupt
huart->Instance->SR &= ~(UART_FLAG_TC);
__HAL_UART_DISABLE_IT(huart, UART_IT_TC);

}


[wek]

I mean the xxx_IRQHandler(), where these functions are called from.

JW

[tggzzz]

To give you an idea of how far this rabbit whole goes, in Uni we spent a year with Jean Bacon's book on the topic:  https://www.amazon.co.uk/Operating-Systems-Concurrent-Distributed-International/dp/0321117891
<snipped good, solid points>

Yes, it is indeed a deep rabbit hole, one that is too often hidden by the layers of infrastructure between the user applications and the hole

Yes, that is an excellent book, which covers very important topics that are too often omitted from university courses. (Especially in favour of which button to push in IDEs and the currently fashionable language)

Which university and course used Bacon's book?

[Sauli]

I am fairly certain that you don't need to use the TC interrupt at all. At least I have not used it in any STM32 code I have written.

In the buffer write routine you can replace this:

Code: [Select]

__disable_irq();
if (!serial_transmit_active[port])
{

// Transmitter is not already running so load the first byte and enable the TXE and TC interrupts
// First clear TC. The read of SR (RMW actually) and a write to DR also clears pending TC int.
serial_port_handles[port]->Instance->SR &= ~(UART_FLAG_TC);
serial_port_handles[port]->Instance->DR = serial_transmit_buffer[port][serial_transmit_buffer_tx_index[port]++];
serial_transmit_buffer_tx_index[port] %= SERIAL_TX_BUFFER_SIZE;
serial_transmit_active[port] = true;

// Enable TX empty and TX complete interrupts
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TXE);
__HAL_UART_ENABLE_IT(port_handles[port], UART_IT_TC);
}
__enable_irq();
with this:

Code: [Select]

// Enable transmit interrupt if it is disabled.
if( __HAL_UART_GET_IT(serial_port_handles[port], UART_IT_TXE) == RESET)
{
__HAL_UART_ENABLE_IT(serial_port_handles[port], UART_IT_TXE);
}

Leave the transmit interrupt handler as is. You don't need the transmit complete interrupt handler or the serial_transmit_active flags.

I don't use HAL, so my code may not be 100% correct, but you get the idea.

[paulca]

To give you an idea of how far this rabbit whole goes, in Uni we spent a year with Jean Bacon's book on the topic:  https://www.amazon.co.uk/Operating-Systems-Concurrent-Distributed-International/dp/0321117891
<snipped good, solid points>

Yes, it is indeed a deep rabbit hole, one that is too often hidden by the layers of infrastructure between the user applications and the hole

Yes, that is an excellent book, which covers very important topics that are too often omitted from university courses. (Especially in favour of which button to push in IDEs and the currently fashionable language)

Which university and course used Bacon's book?

It was the Open Uni.  Course was M301 Concurrent software development and OOP.  The course centred around 2 core items.  Jean's book for theory and a rather contrived distributed banking system as individual java apps.  It was a little light on detailed coding semantics, such as while it discussed exception propagation the examples and exercises where far too easy and single dimensional.  It did have to cover a lot of other "box ticking" topics for the degree requirements, like profressional ethics, security, design and project management methodologies.  In a year. 

The OU had the advantage of being "agile" and able to write and present courses much easier and faster than brick unis.  I know the Queen's University under grad computing where still doing case studies on Mini-Computers and courses written on paper in 1983.  Still passing out the same photocopied notes 20 years later.  The OU got grief for still presenting a course (Microprocessor based systems) from 1993 which has  PIC microcontroller practical module and people said it was out of date.  Hmmm.  How things change.  Now with the "IoT" and mobile internet MCUs are really back in fashion!

[paulca]

A caution with the STM32 having an NVIC and I believe interrupts are interruptable.

There is an application's note somewhere around correctly disabling interrupts or modifying the int mask.  Both of which are themselves critical zones subject to out-of-ordering.

The issue is, there exists a potential for an interrupted function to have modified the interrupt mask register, but still get interrupted.  Combinations exist where, when you disable the interrupts, carry out your critical task and re-enable them, that you are actually re-enabling them for another interrupted function as well, causing concurrency risks.

The basic idea in the note was to save the interrupt mask in a local variable, change it to suite your ciritcal zone, then restore it again.  This may, or may not be what the HAL functions do, it's unlikely it's what the macros do.

[tggzzz]

To give you an idea of how far this rabbit whole goes, in Uni we spent a year with Jean Bacon's book on the topic:  https://www.amazon.co.uk/Operating-Systems-Concurrent-Distributed-International/dp/0321117891
<snipped good, solid points>

Yes, it is indeed a deep rabbit hole, one that is too often hidden by the layers of infrastructure between the user applications and the hole

Yes, that is an excellent book, which covers very important topics that are too often omitted from university courses. (Especially in favour of which button to push in IDEs and the currently fashionable language)

Which university and course used Bacon's book?

It was the Open Uni.  Course was M301 Concurrent software development and OOP.  The course centred around 2 core items.  Jean's book for theory and a rather contrived distributed banking system as individual java apps.  It was a little light on detailed coding semantics, such as while it discussed exception propagation the examples and exercises where far too easy and single dimensional.  It did have to cover a lot of other "box ticking" topics for the degree requirements, like profressional ethics, security, design and project management methodologies.  In a year. 

Such exercises have to be simplistic, since the objectives have to be understood within a reasonable time and the code marked within a short time. Nonetheless it should be possible to illustrate the core issues.

When interviewing I used a simplistic example to assess a candidate's skills and to introduce them to how the company worked. The example was "a toy manufacturer sells cars pushed around roads on a floormat, and wishes to add a working traffic lights accessory. What would you suggest?" I told them there was no single right answer. I wanted to see them ask extra questions to refine the problem statement, and to present a range of possible solutions.

The range of responses was illuminating in both directions

The OU had the advantage of being "agile" and able to write and present courses much easier and faster than brick unis.  I know the Queen's University under grad computing where still doing case studies on Mini-Computers and courses written on paper in 1983.  Still passing out the same photocopied notes 20 years later.  The OU got grief for still presenting a course (Microprocessor based systems) from 1993 which has  PIC microcontroller practical module and people said it was out of date.  Hmmm.  How things change.  Now with the "IoT" and mobile internet MCUs are really back in fashion!

I was horrified and delighted how little had changed in the embedded arena three decades after I started: still C on 8-bit micros. The differences, including going to 32 bit were merely smaller, faster, cheaper.

The main differences I could see were vastly improved ADCs and DACs, and power reductions to the nanopower/energy harvesting level. (And that I could afford the dev kit!)

[Berni]

Yep a lot of modern MCUs support nestable interrupts.

This does bring a few new considerations on what is thread safe code. But you can still disable nestable interrupts to get the more classical one after the other interrupt execution. Also typically an interrupt of higher priority is needed to interrupt an interrupt, so if you keep all your interrupts on the same priority level they never get interrupted. Yet you could still make one interrupt higher priority where you do some tight timing stuff, just be careful what you write in it.

Another way to get around the interrupt mask writing race condition is to instead configure the peripheral to not generate an interrupt while leaving the interrupt channel on the interrupt controller always enabled.

[wek]

A caution with the STM32 having an NVIC and I believe interrupts are interruptable.

There is an application's note somewhere around correctly disabling interrupts or modifying the int mask.  Both of which are themselves critical zones subject to out-of-ordering.

Can you please provide a link.

Thanks,

JW

[paulca]

A caution with the STM32 having an NVIC and I believe interrupts are interruptable.

There is an application's note somewhere around correctly disabling interrupts or modifying the int mask.  Both of which are themselves critical zones subject to out-of-ordering.

Can you please provide a link.

Thanks,

JW

Can't find the original, which was either an ST App Note or a ST community response for an ST employee, but here is an example and the basic jist.

https://stm32f4-discovery.net/2015/06/how-to-properly-enabledisable-interrupts-in-arm-cortex-m/

[paulca]

Are STM32 interrupts re-enterable and recursive?  That often creates some fun too.  Recursive when an interrupt handlers raise interrupts.  Re-entrant in that the same ISR can be entered multiple times and be servicing multiple interrupts at the same time within the same code.  The compiler will give you a fresh stack and a new set of locals, but everything else is in god knows what state.  Avoiding recursive interrupts is probably a way to make your life easier and re-entrancy can be controlled with semaphores if required.

[peter-h]

I am fairly certain that you don't need to use the TC interrupt at all.

The problem is that I am trying to not post too much code, because of I do, I will get no response

The TC interrupt is used to turn off an RS485 driver, on two of the four (or five) serial ports. That is a standard way to do that.

This is the complete TC ISR:

Code: [Select]


// This ISR turns off the 485 driver on p3/p4
// https://peter-ftp.co.uk/screenshots/202210015115900110.jpg

void serial_transmit_complete_irq_handler(UART_HandleTypeDef *huart, uint8_t port)
{

if ( (port == 3) || (port == 4) )
{
// If this is an RS485 port and auto driver mode is set, disable the driver now
if (serial_driver_mode[port-1] == 2)
{
serial_485_driver_control_internal(port, 0);
}
}

// Transmit complete
serial_transmit_active[port-1] = false;

// Clear and disable the TC interrupt
huart->Instance->SR &= ~(UART_FLAG_TC);
__HAL_UART_DISABLE_IT(huart, UART_IT_TC);

}

The function serial_485_driver_control_internal() just waggles some GPIO.

[NorthGuy]

I am seeing an occassional loss of 1 byte by the TX function, so a 5000 byte block sent out actually sends out 4999 (sometimes 4998) out of the UART.

Obviously it's not because of loss of synchronization between the buffer and the index. So, disabling/not disabling interrupts has nothing to do with the problem.

First, verify your receiver. If the problem is with the receiver, you'll waste the rest of eternity debugging your code for nothing. Is there any flow control? Is it working Ok? Does slowing down the baud rate fixes the errors? Do the errors augment if you push the baud rate to the limit?

Verify the value of the indices at the end. This way you'll find out whether they incremented 5000 times or less. If less, the problem is with the code that puts the data into the buffer. If 5000, then the problem is with the transmitting code.

Then start sending consecutive bytes - 0x00, 0x01, 0x02 etc. in a loop. Inspect what you receive on the other end. See where the losses are - at the beginning, at the end, at the buffer rollover points (change buffer size to test this) etc.

[Sauli]

The problem with the serial_transmit_active flag is that it is still set for one character time after the TXE interrupt has been disabled, and the serial_transmit function wont trigger a new transmit during that time. I would still recommend using the TXE enabled bit to do that decision. You can keep the TC interrupt for turning off the RS485 driver.

[peter-h]

No flow control; I am sending data at a rate chosen so the baud rate is not limiting. I am also watching it on a scope and timing-wise it looks OK.

Indeed my debug plan is to later just keep repeating the same packets and watch the data on a scope. It's more complicated though: with 5000 byte packets, the loss (if there is any) is~1 byte, but with short packets e.g. 100 bytes the loss can be much bigger, which I don't get at all.

I know the number of bytes is correct all the way through the chain, there and back (it actually goes RS232 in, 38.4k, via one box, over ETH, back via another box, RS232 loopback 115.2k, back over ETH, and back out via RS232 at 38.4k. I have byte counts at different parts of that chain and all of them are 100% rock solid, hence the data loss must be at the very end, at the final serial TX function. The equipment generating the test data and receiving it and comparing it is a custom built programmable protocol converter which I've been selling for 30 years and that works error-free if I do just a piece of wire for loopback. Yes; the timing is not the same then and there could be an issue; I will re-test with lower baud rates.

This discussion has raised great points. I will test moving that flag:

The problem with the serial_transmit_active flag is that it is still set for one character time after the TXE interrupt has been disabled, and the serial_transmit function wont trigger a new transmit during that time. I would still recommend using the TXE enabled bit to do that decision. You can keep the TC interrupt for turning off the RS485 driver.

It's a good point. However I don't get

I would still recommend using the TXE enabled bit to do that decision.

Do you mean using the TXE irq enable bit as the flag?

[NorthGuy]

It's more complicated though: with 5000 byte packets, the loss (if there is any) is~1 byte, but with short packets e.g. 100 bytes the loss can be much bigger, which I don't get at all.

38400 is slow. Therefore, when you send a lot, the buffer is always full. With 100 characters the buffer is half full (or half empty) and never get full.

What characters do you lose? At the beginning? At the end? In the middle?

How many characters do you lose when you transmit 100 characters?

I know the number of bytes is correct all the way through the chain, there and back (it actually goes RS232 in, 38.4k, via one box, over ETH, back via another box, RS232 loopback 115.2k, back over ETH, and back out via RS232 at 38.4k. I have byte counts at different parts of that chain and all of them are 100% rock solid, hence the data loss must be at the very end, at the final serial TX function. The equipment generating the test data and receiving it and comparing it is a custom built programmable protocol converter which I've been selling for 30 years and that works error-free if I do just a piece of wire for loopback. Yes; the timing is not the same then and there could be an issue; I will re-test with lower baud rates.

So, you're saying that you verified that after you transmit 5000 bytes the "serial_transmit_buffer_tx_index[port]" is equal to (5000 %= SERIAL_TX_BUFFER_SIZE), but you only 4999 characters appear on the physical TX line?

[Sauli]

Do you mean using the TXE irq enable bit as the flag?

Yes, that is exactly what I mean.