I have a strange problem in my code. I cannot receive characters via the serial port.
I've read up on the TI and RI interrupts and I think I am handling these correctly.
The program goes to the serial I/O isr, but just sits at the gets(comin,4) line. When I examine comin in the watch window and input chars via the SIN = xx (where xx is an ascii code), I can see the comin array remains empty.
Below is the serial I/O interrupt routine (my application does not need Tx)
pre void uart_rx_isr (void) interrupt 4 using 3 { signed char index=0; EA=0;
if (RI == 1) { gets(comin,4); command = atoi(comin); } RI=FALSE; /* finished isr - clear flag */ TI=FALSE; /* TI will not be used - always clear it*/ EA=1; }
/pre
Here is a fragment from main() - you can see that I set TI=1 initially to set the UART up
pre
TI=TRUE; /* always set TI=1 initially to allow serial printing */ RI=0;
loop: //RI=0; //IDLE
while ((1));
goto loop; } /pre
Appreciate some pointers here.
Jason
Only 4 chars will ever be used and no more in this application. 1) that may be 'famous last words' 2) you will 'hang' if ever there is any noise on the line 3) why put a 'hang' in the iSR when you state "it will never happen" 4) one of the characteristics of good code is that it does not 'die' on unexpected events
Erik
This is my interpretation of the situation...
[Origional Source...]
void uart_rx_isr (void) interrupt 4 using 3 { signed char index=0; EA=0; if( RI == 1 ) { gets( comin, 4 ); command = atoi( comin ); } RI=FALSE; /* finished isr - clear flag */ TI=FALSE; /* TI will not be used - always clear it*/ EA=1; }
TI=TRUE; /* always set TI=1 initially to allow serial printing */ RI=0; loop: //RI=0; //IDLE while( (1) ) ; goto loop; }
This got changed to (by Jason Van Ryan)...
/*------------------------------------------------*/ /* Interrupt service routine to handle the Rx /* input which comes in via RxD on pin 11 */ /* of the 89LPC922 */ void uart_rx_isr (void) interrupt 4 using 3 { int index=0; EA=0; if (RI==1) /* if RI is set then execute the RxD routine, else jump over and exit*/ do { comin[index] = (getchar()); index++; } while (index<4); RI = FALSE;/* clear the RI interrupt flag and exit */ EA = 1; } /*---------------------------------------------*/
--Cpt. Vince Foster 2nd Cannon Place Fort Marcy Park, VA
I personally would have changed it to something like this:
(Main.C Module):
#define DO_NOTHING #define UART_ONE 1 // depending upon which uC #define UART_TWO 2 // switch( n ) selection #define UART_THREE 3 #define UART_FOUR 4 extern void Init_UART_X( u8 ); // UART_ONE ... UART_FOUR extern void Error_Handler( void ); // if fails FOREVER loop /* **====================================================================== ** main **====================================================================== ** ** Dump main-line example ** **---------------------------------------------------------------------- ** ** Parameters Passed: <void> ** Parameters Returned: <void> ** Notes: ** **---------------------------------------------------------------------- */ void main( void ) { /*------------------------------------------------------------------. ; Initialize the UART used ; ;-------------------------------------------------------------------; ; (Not a clue why he is doing the TI=TRUE thing now ... ) ; ;-------------------------------------------------------------------; ; TI=TRUE; /* always set TI=1 initially to allow serial printing ; ; RI=0; ; '------------------------------------------------------------------*/ Init_UART_X( UART_ONE ); // configures entire UART[ n ] parameters /*------------------------------------------------------------------. ; Enable Interrupts ; '------------------------------------------------------------------*/ EA = 1; // Master Enabler for the 8051 /*------------------------------------------------------------------. ; The INFINITE LOOP ; '------------------------------------------------------------------*/ while( TRUE ) { DO_NOTHING; } /*------------------------------------------------------------------. ; Failed the infintate loop--try to gracefully shut-down/reset ; '------------------------------------------------------------------*/ Error_Handler( ); }
Then the ISR.C Module:
#define MAX_UART_x_BUFF_SIZE ((u8)20) // 20 bytes of bufferable RxD? #define MAX_FIFO_BUFFER_SIZE ((u8) 4) // maximum FIFO size (pre-ISR) extern int getchar( void ); // routine to pull data from the FIFO stack extern int COM_Input_Stream[ MAX_UART_x_BUFF_SIZE ]; // extern u16 RxD_Isr_Glitch; // global data-store for tracking false ISRs extern u8 Flag_UART_Message; // { TRUE | FALSE } flag extern u8 Flag_RxD_Msg_Overrun_Error // { TRUE | FALSE } flag /* **====================================================================== ** UART_RxD_isr [ ISR ] **====================================================================== ** ** This shall extract the UART FIFO's contents. This assumes that an ** isr was triggered after the FIFO is filled, or at least ready to be ** emptied (could be triggered on less than max FIFO depth, like 1/2 ** etc signals associated with the FIFO). ** ** This extracts the FIFO data and stores the results into a buffer ** that is either module-global, or system-global (unknown). The data ** within this data-store must be processed prior to entering this ** ISR since this ISR handler over-writes the same data-store index ** locations each time it is triggered. ** **---------------------------------------------------------------------- ** ** Parameters Passed: <void> ** Parameters Returned: <void> ** Notes: ** ** Highly hardware dependant upon the FIFO characteristics. ** MUST process the data, or at least re-buffer the contents ** prior to re-entering this isr due to the over-write potential. ** ** CAUTION: Also this uses 'using' for reg-bank selection ** **---------------------------------------------------------------------- */ void UART_RxD_isr( void ) interrupt 4 using 3 { u8 index = 0x00; // (0 through MAX_FIFO_BUFFER_SIZE) < 256 /*------------------------------------------------------------------. ; Depending upon the rest of the system's requirements, either ; ; disable (0) ALL other interrupts, or keep it on (1) ; '------------------------------------------------------------------*/ EA = 0; // Disables ALL other interrupts /*------------------------------------------------------------------. ; if RI is set then execute the RxD routine, else jump over and exit; '------------------------------------------------------------------*/ if( RI == 1 ) { /*--------------------------------------------------------------. ; Allow the ISR to be flagged again while we're still processing; ; the previous' four bytes. ; '--------------------------------------------------------------*/ RI = 0; // clear the RI interrupt flag ASAP /*--------------------------------------------------------------. ; Get the next FIFO'd characters ; ; I guess the FIFO buffers the bytes, and this part just reads ; ; the FIFO contents. This implies that the ISR is generated ; ; after the MAX_FIFO_BUFFER_SIZE has filled up. ; '--------------------------------------------------------------*/ do { /*----------------------------------------------------------. ; the getchar( ) routine returns an int, load it into the ; ; 'global' data-store. These are FIFO'd in. ; '----------------------------------------------------------*/ COM_Input_Stream[ index ] = (int) ( getchar( ) ); index++; // bump index to next array slot } while( index < MAX_FIFO_BUFFER_SIZE ); /*--------------------------------------------------------------. ; Alert the world that we have an input on the COM_Input_Stream ; '--------------------------------------------------------------*/ Flag_UART_Message = TRUE; // sticky flag } /*------------------------------------------------------------------. ; ISR was falsely triggered, monitor that. ; '------------------------------------------------------------------*/ else { /*--------------------------------------------------------------. ; bump the false RxD ISR occurances ; '--------------------------------------------------------------*/ if( RxD_Isr_Glitch < MAX_SATURATION_RxD_GLITCHES ) { RxD_Isr_Glitch++; } } /*------------------------------------------------------------------. ; If we are still in this routine, and it was triggered again, then ; ; declare a message over-run error ; '------------------------------------------------------------------*/ if( RI == 1 ) // Should have been cleared by now ! { Flag_RxD_Msg_Overrun_Error = TRUE; // sticky flag } /*------------------------------------------------------------------. ; Depending upon the rest of the system's requirements, either ; ; disable (0) ALL other interrupts, or keep it on (1) ; '------------------------------------------------------------------*/ EA = 1; // Enables interrupts }
NOE: NOT Compiled, just written, so I'm not sure if I have any spelling errors or other simple compile errors.
Jason,
Then you get the people who like to create a mass of [uncompiled, untried and questionable] text (with very little code) for a simple job ;)
It's then typically followed by something starting with 'my code monkey manual states that...'
I also picked up on the fact that EA=0 disables all interrupts.
But my code does not exit the isr until the index is incremented to the compare value (in my case this is 4) and then it exits - i.e. all the chars are collected in one go.
After exiting the isr, I immediately go and process comin[]. I suppose a more robust way of doing this is to collect the chars 1 by one (so interrupts are NOT disabled) and then to use the \n at the end of the string and to then go an process the result (comin[]). My immeadiate problem now however is getting atoi to work - I've posted this in another thread.
Once I've got the thing running functianaly, I'll go back and look aat making some of this code a bit more elegant.
Part of the learning process.
cheers
BTW, I had no luck with
command_string = gets(comin,4)
Seems the string functions are problematic.
total misuse of the interrupts
you are mixing polled and interrupt and the result is that your other code (which you may not have implemented yet) will hang.
the basic form of a Uart interrupt is
if RI { RI=0; buffer[ReadIndex] = SBUF0; ReadIndex++; } if TI ......... then in main if ReadIndex > ??? [ ....
erik
yes, this is not optimum, but I doubt this is causing the atoi problem.
Going step at a time, especially when learning, is a good philosophy.
So, you think atoi is problematic. Have you checked to see what the string passed to the function actually is? Is it a valid ASCII numeric string? Has it got a valid terminator?
If the answer to these are yes, then why not pass a pre-built (maybe a constant) string to the function to ensure you get what you would expect.
Then you might build up confidence in the library functions you are using. Consider that the Keil library functions have existed for quite a while and any serious problems would have been sorted long ago.
Finally, I would be extremely cautious of responses like "total misuse of the interrupts". Like many things in life, there is no absolute right or wrong. An interrupt is a mechanism like many others that you, as a programmer, are free to use in any way that is suitable for the situation in hand.
The more you understand what you are doing (and the ramifications of your choices), the better a programmer you can become.
I would advise you not to do it the way you are doing, but you have the final say.
"...your other code (which you may not have implemented yet) will hang.
Depending upon how you write that code influences the answer as to whether it might hang.
It is totally false to assume that it will hang.
It is perfectly acceptable to write code that both uses interrupts and polling techniques - So long as it is done correctly.
I have seen pure interrupt code that hangs and I have seen pure polled code that hangs. I have seen code that uses a combination of the two work.
Please don't assume any favoured approach from that however. The choice depends upon the situation.
It is perfectly acceptable to write code that both uses interrupts and polling techniques - So long as it is done correctly. sure, but the way it is done all processing will come to a screeching halt from the arrival of char 1 till the arrival of char 4. no interrupts, no main NOTHING!
"... a screeching halt from the arrival of char 1 till the arrival of char 4."
All depends upon the implementation of getchar. Could easily (and quite reasonably) have a timeout check included.
I've done that very same type of thing myself - Just got to make sure it's all done properly.
alright, but I for one would not want to write code that relies on such premises. Yes it can be done and it will work, but it is not intuitive to the naked eye - or in other words a maintenance nightmare. Not on my plate, please.
Tamir,
Sorry but I think you're missing the point.
I'm saying that it could be done that way, not that it should be done that way.
I then went on to indicate that when done properly (i.e., with careful commenting etc), it can be useful. It would NOT be a premise if it were clearly documented.
Not all requirements are the same and therefore not all methods for achieving those requirements are the same.
I've seen your pre-emptive scheduler. It's quite impressive; but there are things you do in that which are normally considered 'off limits' in an application.
I'm simply saying that needs vary - And I believe that simply knocking newbie code with a warning of "it will hang" is (to say the least) not very constructive. The OP might like to know that alternatives may sometimes be acceptable.
And I've just noticed an earlier false problem report by the same person with regards to the OPs code (re: an uninitialised variable).
Whoops.
It would NOT be a premise if it were clearly documented.
Should read:
It would NOT be a problem if it were clearly documented.