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
MY CODE IS WORKING!!!! for a while if index ever get to 4 you are stuck (oh I know "that will never happen")
Please try bit more accomodating of newbies.
I DO "accomodate of newbies", just not with pat solutions from which nobody learns anything.
For those that were constructive if you consider leaving you free not to think "constructive", you will never get abnywhere.
Erik
For what it's worth:
Assuming that you are expecting a four character response (and only a four character response), I cannot see what would be wrong with your while loop.
That said, as one of the respondents indicated, it would be sensible to rethink the interrupt service routine to accumulate one character per interrupt call.
Additionally I would suggest that you ignore some of the responses that seem to crop up in this forum from the self proclaimed and/or belligerent experts. Do yourself a favour, search the forum to examine their posting history and style; and make up your own mind.
Only 4 chars will ever be used and no more in this application.
and no less.
Then it is perfectly safe to ignore that certain posters self-importance :)
;-)
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
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.