richard_damon wrote on Wednesday, June 13, 2012:
You don’t specify what processor you are using, but from the names it looks like some form of a PIC32. Here are some of the issues with what you are doing.
&U1RXREG is the address of the whole register. Casting this to void*, and then accessing only 1 byte of the word (I am assuming the Queue are only storing 1 byte at a time) is architecture dependent. It normally works on “little endian” machines (where the lowest order byte of a word has the lowest address), which is what PICs are, but does not work on “big endian” machines, you have thus quietly imbedded an architectural dependance into you coding style.
Second, on some machines, the hardware specification manual specifically says that I/O registers should only be accessed with full word access instructions (Not knowing your exact processor, we can’t check this case) and your method of access here will NOT meet that restriction if it is there. Again, it may well work for your case, but you are developing a programming style that is hiding a architectural dependency.
As a more general issue, your code also now has duplicate sections doing the same things (getting the character from te buffer and clearing the interrupt). If you reversed the code and had the logic be: Data available interrupt happened, so get the data and clear the interrupt, then try to enque the data, and if that succeeds, we are done, else we need to think of what to do for error recovery; then you get cleaner code.
You also should give some more thoughts to your error recovery here. If you can be sure that this really shouldn’t happen, than maybe quietly dropping characters is ok, but if the serial port is sending data with any form of structure, it may be important for the receiving task to know that a data loss has occurred. Generally, at a minimum I set an overrun flag that can be checked, and sometimes put the serial port in a mode where I start to discard ALL characters received until the queue is emptied, and maybe even until a “record break” if the protocol makes that easy to find, in an attempt to minimize the number of records that are disturbed by the hang up. I sometimes will even then enqueue and “error character” into the queue so the receiving task knows the data has been messed up.