This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Serial Rx problem

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

  • "... 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.

  • All depends upon the implementation of getchar. Could easily (and quite reasonably) have a timeout check included.

    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'

    Erik

  • 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.

    cheers

    Jason

  • . . . 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

    command_string = gets(comin,4)

  • ".. 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.

    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'

    (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.

  • Jason,

    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.

  • Not would, the word should be might. As I've quite cleary shown, it depends on other factors.

    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 ????

    Erik

  • "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.

  • Erik is a realist

    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.

    what nonsense!!, now you are talking nonsense

    If a noise pulse or a difference in the input creates more than 4 RI ints, the code (the code posted before my reply) WILL hang in the UART ISR i.e. die WHATEVER THE REST OF THE CODE DOES.

    Since you have nitpicked my language, where did you learn to spell 'dependant'

    Erik

  • "Erik is a realist"

    Not the word I was thinking of ;)

    "what nonsense!!, now you are talking nonsense"

    I suggest you read and understand the WHOLE thread - Heck, I hope you don't read data sheets in this manner!

    "Since you have nitpicked my language, where did you learn to spell 'dependant'"

    Perfect example again of your failure to read (and understand) what is written!

    I said: I'm a native English speaker

    Adj. 1. dependant - contingent on something else

    See www.thefreedictionary.com/dependant

    Oh, and by the way, it wasn't nitpicking. I really couldn't understand that text you previously wrote. It was just totally meaningless.

  • The original code did use getchar().

    The Keil documentation says:

    The getchar function reads a single character from the
    input stream using the _getkey function. The character
    read is then passed to the putchar function to be
    echoed.
    
     Note
    
    This function is implementation-specific and is based
    on the operation of the _getkey and putchar functions.
    These functions, as provided in the standard library,
    read and write characters using the microcontroller's
    serial port. Custom functions may use other I/O
    devices.
    

    A customer-supplied _getkey() that has a timeout would solve the deadlock problem. The bad thing is that the code just waits for four characters without checking if valid characters are found or not. It doesn't contain any code to verify if a custom _getkey() function has had a timeout. And it doesn't contain code to synchronize after spurious noise.

    All protocols should have some form of synchronization. Either unique timing requirements, where characters in a packet _always_ arrives with a short time delay compared to the delay between packets. Or the protocol should make use of unique characters to synchronize the transfer.

    And if applicable, transfer protocols should consider two-way transfers with acknowledge, and the use of checksumming or similar to make sure that a correct message was received.

    A protocol with resend should also consider sequence numbers or similar to make sure that a resend after a lost ack isn't treated as a new message.

    And as already noted, atoi() does work with C strings. A four-digit number requires (at least) five characters in the array, since the four digits must be followed by a terminating zero. White space may be added before the first digit, or after the last digit. An optional sign may be added before the first digit. Your code should very explicitly make sure that the received array gets terminated. Source code is easier to maintain if you think about locality of reference. Place the code that terminates the string close to the lines that assigns the four characters. Even if the array is global and initially zero-filled by the C startup code, your code should explicitly assign a termination. Both to make the code easier to read, and to make the code self-repairing. If your code expects the fifth character position to have been zero-initialized by the startup code, and this byte once every third year gets overwritten, your code will then spend the next years with whatever termination there may be somewhere later in the memory. An embedded application should detect corruption and report/abort, and/or auto-repair.

    What would happen if the fifth position in the array happens to be overwritten with the value '7', and the six position (or the variable directly after the 5-element array) contains the value zero? You send the value 1234 to your device, and atoi() finds 12347 in the array.