 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Zer0flag
Guest
|
| My buffered putc routine and a very strange problem |
 Posted: Wed Apr 09, 2008 4:22 am |
 |
|
Hi all!
I have written a simple buffered putc routine and have a very strange problem. Actually a very similar routine is found in the book "Embedded C Programming and the Microchip PIC" written by Barnett, Cox and O'Cull and it has the same problem. I want to find out where the strange problem comes from.
| Code: |
#define TX_BUFFER_SIZE 250
char TX_Buffer[TX_BUFFER_SIZE+1]; // character array (buffer) : allows us to use array index from 0 to TX_BUFFER_SIZE
char TX_Rd_Index = 0; // index of next char to put into the buffer
char TX_Wr_Index = 0; // index of next char to fetch from buffer
char TX_Counter = 0; // a total count of characters in the buffer
//------------------------------------ write a character to the serial TX buffer
void bputc(char c)
{
while (TX_Counter > (TX_BUFFER_SIZE-1))
; // WAIT! Buffer is getting full! Wait to become empty -> PROBLEM IS HERE!!!
disable_interrupts(int_tbe); // just to make sure that the interrupt does not mix up something if it occurs here, removing this line does not affect the problem
TX_Buffer[TX_Wr_Index] = c; // put the next char into the buffer...
TX_Wr_Index++;
if (TX_Wr_Index > TX_BUFFER_SIZE) // wrap the pointer
TX_Wr_Index = 0;
// keep track of buffered chars
TX_Counter++;
// re-enable the interrupts
enable_interrupts(int_tbe);
}
//--------------------------------- RS232 Transmitter ISR
#int_tbe
void RS232_TX_ISR()
{
// if there are characters to be transmitted...
if (TX_Counter != 0) // buffer is not empty, there are chars to be transmitted
{
// send the character out the port
putc(TX_Buffer[TX_Rd_Index]); // transmit next char
TX_Rd_Index++;
// test and wrap the pointer
if (TX_Rd_Index > TX_BUFFER_SIZE)
TX_Rd_Index = 0; // wrap the read index pointer
TX_Counter--; // keep track of the counter
// if after transmitting there are no more characters in the buffer, then disable the interrupt so that it does not fire again
if (TX_Counter == 0)
disable_interrupts(int_tbe);
}
}
|
My problem is that the program hangs in the WHILE LOOP in bputc() when i send a long string to be output and the TX_Counter reaches TX_BUFFER_SIZE. Until then everything works fine. The chars are sent normally. I also checked TX_Counter, it increases and decreases normaly but when it reaches TX_BUFFER_SIZE the int_tbe interrupt somehow seems to stop occuring and the program hangs in the while loop.
Best regards,
Zer0flag |
|
 |
Matro
Guest
|
|
| Posted: Wed Apr 09, 2008 5:26 am |
|
|
Are there some variables that are modified otherwhere (for example in the main())?
Could you post a complete "compilable" code that demonstrates the problem?
Matro |
|
|
Ttelmah
Guest
|
|
| Posted: Wed Apr 09, 2008 5:44 am |
|
|
First, shift the disable_interrupt in the interrupt handler. So:
| Code: |
//--------------------------------- RS232 Transmitter ISR
#int_tbe
void RS232_TX_ISR()
{
//If you arrive in the routine, there _must_ be a character to output
//really no reason to test, but you can if you want.
if (TX_Counter) {
// send the character out the port
putc(TX_Buffer[TX_Rd_Index]); // transmit next char
TX_Rd_Index++;
// test and wrap the pointer
if (TX_Rd_Index > TX_BUFFER_SIZE)
TX_Rd_Index = 0; // wrap the read index pointer
TX_Counter--; // keep track of the counter
}
// if _at any time_ there are no more characters in the buffer, then
//disable the interrupt so that it does not fire again
if (TX_Counter == 0)
disable_interrupts(int_tbe);
}
|
Problem was, that if the interrupt _did_ fire with nothing in the buffer, the interrupt would never get cleared, leading to a hang up. It shouldn't happen, but better safe...
Now, the only reason for what you describe, is that the code is arriving from somewhere, either with the TBE interrupt disabled, or the global interrupt flag cleared. Modify the send to:
| Code: |
//------------------------------------ write a character to the serial TX buffer
void bputc(char c)
{
while (TX_Counter > (TX_BUFFER_SIZE-1)) {
enable_interrupts(GLOBAL);
enable_interrupts(INT_TBE);
}
disable_interrupts(int_tbe); // just to make sure that the interrupt does not mix up something if it occurs here, removing this line does not affect the problem
TX_Buffer[TX_Wr_Index] = c; // put the next char into the buffer...
TX_Wr_Index++;
if (TX_Wr_Index > TX_BUFFER_SIZE) // wrap the pointer
TX_Wr_Index = 0;
// keep track of buffered chars
TX_Counter++;
// re-enable the interrupts
enable_interrupts(int_tbe);
}
|
Which will ensure the wait is done wth the interrupts enabled.
Best Wishes |
|
|
Wayne_
Joined: 10 Oct 2007
Posts: 681
|
|
| Posted: Wed Apr 09, 2008 6:20 am |
|
|
There is another reason how this could problem happen. I am not sure if this is apparent in CCS and PIC code but...
You can verify this by looking at the assembler lst file.
Just did a check myself and it looks like CCS reads the value every time so this should not be your problem, good to know anyway!
When the compiler optimises the code for the while loop it doesn't know that TX_Counter may be modified elsewhere so only reads the value once before entering the while loop.
So although the interrupt handler changes this value it is never actually read again in the while loop.
This means that once the value gets above TX_BUFFER_SIZE-1 the loop is endless.
Now there is away to force the compiler to read this every itteration of the loop before doing the check and that is to define the variable as volatile.
volatile int TX_Counter;
Basically you are telling the compiler that this value could change at any time and to make sure that the real value is read when using it. |
|
|
Zer0flag
Guest
|
|
| Posted: Wed Apr 09, 2008 7:02 am |
|
|
Thank you for the hints! Now I know the problem is with the function bputc. I call it from other interrupts, e.g. INT_RDA in order to echo incoming chars. Now the compiler says "Interrupts disabled during call to prevent re-entrancy" for bputc and all interrupts are disabled when calling it, so the program cannot transfer further chars while waiting for the buffer to become free :(
Is there any workaround for this? |
|
|
Matro
Guest
|
|
| Posted: Wed Apr 09, 2008 7:19 am |
|
|
Basically, each of your isr shall not call functions but only set/clear flags that are polled in the main endless loop and that are used to call the appropriate functions.
That is a general rule that avoid bugs and make the code better.
Matro. |
|
|
Zer0flag
Guest
|
|
| Posted: Wed Apr 09, 2008 8:56 am |
|
|
Well, the problem is that if I want to echo all chars that I receive from RS232 I have to put them somehow into my SW transmit buffer and this would make buffer management too complicated. I cannot do this at one place in main() because there are other bputc() calls everywhere in main() and the order of the chars for output will be mixed up. This is why I need to call bputc() from my INT_RDA ISR directly. At least I think this is the only way here.
To bypass the problem caused by the compiler diabling my interrupts during bputc() I modified bputc() like this:
| Code: |
//------------------------------------ write a character to the serial TX buffer of the program
void bputc(char c)
{
// WARNING!!! GLOBAL INTERRUPTS WILL BE DISABLED BY THE COMPILER IN THIS FUNCTION IF CALLED FROM AN INTERRUPT TO PREVENT REENTRANCY
while (TX_Counter > (TX_BUFFER_SIZE-1))
{
while (TXIF==0) // Poll for HW transmit buffer to become empty, because interrupts are disabled by the compiler in this function!
;
putc(TX_Buffer[TX_Rd_Index]); // transmit pending char from SW buffer
TX_Rd_Index++; // move the index
// test and wrap the index
if (TX_Rd_Index > TX_BUFFER_SIZE)
TX_Rd_Index = 0; // wrap the read index pointer
TX_Counter--; // keep track of the buffer fill counter
}
// now handle the new char
TX_Buffer[TX_Wr_Index] = c; // put the next char into the buffer...
TX_Wr_Index++; // move the index
if (TX_Wr_Index > TX_BUFFER_SIZE) // wrap the pointer
TX_Wr_Index = 0;
// keep track of buffered chars
TX_Counter++;
// re-enable the INT_TBE interrupt
enable_interrupts(INT_TBE);
}
|
Now I poll TXIF in my while loop (nothing happens there anyway) instead of waiting for the INT_TBE interrupt to occur which did not not happen because of the globally disabled interrupts by the compiler. When TXIF indicates that the HW has finished transmission I do the same thing I also do in my INT_TBE ISR, i.e. I send the pending char and decrease my buffer fill counter. After doing so there is a free position in my buffer and I can insert the new char.
Of course if bputc() is called from an interrupt and busy waits on TXIF no other interrupts will occur during that time :(
Best regards,
Zer0flag |
|
|
Matro
Guest
|
|
| Posted: Wed Apr 09, 2008 9:11 am |
|
|
You will always have problems with this implementation.
Imagine that your buffer is just full and you receive a byte through RS-232.
What will happens?
The isr will call the bputc() that will wait for the buffer not to be full and...
That's the dead lock.
I think there is only 2 solutions :
- When a byte is received through RS-232, you have to choose what to do if this byte is too much for the buffer : ignore it, overwrite, ...
Or
- Use a flow control with RS-232 to stop communication when buffer is full.
In all other cases you can find a situation where the application will be locked.
Matro. |
|
|
Zer0flag
Guest
|
|
| Posted: Wed Apr 09, 2008 9:26 am |
|
|
| I think that my last version of bputc() posted above should not block because in case of a full buffer it can send out one char from the buffer and free the buffer without waiting for an interrupt to occur. |
|
|
Matro
Guest
|
|
| Posted: Wed Apr 09, 2008 9:32 am |
|
|
Sure. But maybe during the execution time of bputc() another byte has come through the USB and as the interrupts are disabled you will never see it.
So in this case your choice is implicitly to ignore extra byte(s).
Matro. |
|
|
Ttelmah
Guest
|
|
| Posted: Wed Apr 09, 2008 9:39 am |
|
|
Yes.
What you post now, should work.
There is some other solutions as well.
Have _two_ bputc routines. One called 'bputc', which you use in the main code, and uses the original style, and one called 'ibputc, which is called in the interrupt, and uses the new style. This has the advantage that interrupts won't be disabled in the 'main', when the routine is called.
The other one is to ensure that message sizes are small enough so that the buffer shouldn't overflow. Then if it does (since this should now be an 'unexpected' event), simply throw away the oldest character.
Best Wishes |
|
|
Matro
Guest
|
|
| Posted: Wed Apr 09, 2008 9:49 am |
|
|
| Ttelmah wrote: | Yes.
What you post now, should work.
There is some other solutions as well.
Have _two_ bputc routines. One called 'bputc', which you use in the main code, and uses the original style, and one called 'ibputc, which is called in the interrupt, and uses the new style. This has the advantage that interrupts won't be disabled in the 'main', when the routine is called.
The other one is to ensure that message sizes are small enough so that the buffer shouldn't overflow. Then if it does (since this should now be an 'unexpected' event), simply throw away the oldest character.
Best Wishes |
It thought about having 2 functions but this is very dangerous due to the use of the counters. If an interrupt occurs between array updating and new counter value updating, a charachter will be overwritten.
If the counter is incremented before array updating, the problem is reverted and a non-written gap will exist in the array.
I really think that you can always find a situation with problems where you have to choose the behavior you want. ;-)
Matro. |
|
|
Ttelmah
Guest
|
|
| Posted: Wed Apr 09, 2008 2:41 pm |
|
|
If you look at the original code, the interrupts are disabled for the counter updates. This should be done for the buffer updates, and counter updates anyway, since they don't occur as a single operation...
Best Wishes |
|
|
Matro
Guest
|
|
| Posted: Thu Apr 10, 2008 1:36 am |
|
|
Indeed, but you take the risk to miss a incoming RS-232 byte.
Anyways, the ability of the PIC to process all data without loss will be an equation between MIPS, code execution time and RS-232 baudrate. ;-)
Matro. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
