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
HI.
i have tidyed yo're code.
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; }
they're a lot of things wrong.
the bigest i think is yo're interrupt. it gets called when they're is one char from uart. but you call 'gets" to get more char's. where do u think they will come from?
u must change yo're isr to collect the char's. one at a time.
also u must clear ri if it is set. not evry time.
r u using tx anywear? if u do not then u do not set ti.
Alway's yo're freind.
Zeusti.
I don't know this processor architecture so I don't know if the following is even supported, but maybe the OP is using serial communication with a FIFO which explains why he tries to read more than one character...?
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.
"...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.
And I believe that simply ... with a warning of "it will hang" is (to say the least) not very constructive .. you would see that I gave examples of how e.g. a noise pulse on the serial line would lock the uC up.
And I believe that simply knocking newbie code
not "simply knocking" but, instead of providing pat solutions that help nobody in the long run. suggesting that the op does this absolutely awful thing commonly know as 'thinking' and providing a 'pointer'
Thanks stephen.
I am sending
printf("9012");
from the Tx side.
Additionally, if I use the simulator andd enter
SIN = 57 SIN = 48 SIN = 49 SIN = 50
I get the correct numbers in comin.
I am gount to try and put a space or similar into the first position and try that later today.
. . . but my codes are getting collected and are in comin. So what's going on here?
I checked again today, and this hangs the MCU
".. you would see that I gave examples of how e.g. a noise pulse on the serial line would lock the uC up."
Not would, the word should be might. As I've quite cleary shown, it depends on other factors.
(Let's just ignore the atrocious use of punctuation, grammar etc.)
Who is even suggesting that there is anything wrong with thinking? In my opinion, it is an inherent requirement of being a successful developer. My concern is the quality of the teachers and the 'pointers' they choose to give.
Just look at the post with the heading 'study time' - QED.
Regardless of the cross conversation going on in this topic, I would suggest that you re-visit the ISR and consider re-structuring it.
At this point in your learning, you might find that it makes it easier for you to follow the problem through.
Regards.
as I said
4) one of the characteristics of good code is that it does not 'die' on unexpected events
it is WILL, I can not count how many pieces of 'working code' I have seen that was faultly coded by ignoring 'might' and surprised the coder by 'might' being 'will' some time later.
Your statements about "nurturing newbies by letting them go ahead with marginal code" how very well intended are deterimental to the future of those. What a newbie picks up is almost impossible to eradicate later.
Just look at the post with the heading 'study time' I see in the above posts that drawing the OPs attention to values made someone think I was ignorant of what they were, as in if I say "do you see the sunset" I am unaware of it ????
"one of the characteristics of good code is that it does not 'die' on unexpected events"
I totally agree with that.
"it is WILL..."
Absolute nonsense. As I quite clearly showed, it is totally dependant upon the rest of the code. Neither you nor I know what that code consists of - So neither you nor I can be 100% certain whether it will work or fail.
Your statements about "nurturing newbies by letting them go ahead with marginal code"
Are you intentionally misreading what was written? I never mentioned the nurturing of marginal code. I am trying to encourage open thinking. I believe a beginner can learn a lot from experimentation and any mistakes they encounter. I know I did.
I see in the above posts that drawing the OPs attention to values made someone think I was ignorant of what they were, as in if I say "do you see the sunset" I am unaware of it ????
Sorry, I'm a native English speaker and I cannot figure out what that is saying. I just hope the comments in code you produce are more comprehensible.
View all questions in Keil forum