Continue to Site

Welcome to our site!

Electro Tech is an online community (with over 170,000 members) who enjoy talking about and building electronic circuits, projects and gadgets. To participate you need to register. Registration is free. Click here to register now.

  • Welcome to our site! Electro Tech is an online community (with over 170,000 members) who enjoy talking about and building electronic circuits, projects and gadgets. To participate you need to register. Registration is free. Click here to register now.

Untested: 9600 baud low CPU overhead interrupt & dual timer full duplex UART

Status
Not open for further replies.

blueroomelectronics

Well-Known Member
I'm just learning C so bear with me.

It's not beautiful and I could use some advice on cleaning up and making the code more efficient. I'm porting it over from a Swordfish BASIC program I wrote recently and I've only just started learning C (two weeks so far). It's written for the 18F46K22 as a tertiary serial port on PORTB0 & 1 using Timers 2 & 4 plus INT0

I'll run it on actual hardware in the next couple of days. I'd appreciate any comments or concerns.

Untested on XC8, the Swordfish version works.
Code:
 /* 
  * Author: Blue Bill 
  * 
  * Created on June 13, 2013, 11:10 AM 
  * 9600baud interrupt driven low CPU overhead full duplex software serial port 
  * Uses Two timers and INT0 (falling edge IRQ on RX) 
  * RB.0 = RX, RB.1 = TX in this example 
  * this example uses Timers 2 & 4 (other timers can be used if desired) 
  * a 1MHz instuction clock was used for simplicity 
  */ 
 #define QBAUD 103 //  4000000 / 4 / 9600 - 1 
 union both { // QTXSend.byte = char, QTXSend.Shiftbit.B0     
     char byte; 
     struct { 
         unsigned LSB : 1, none : 6, MSB : 1; 
     } Shiftbit; 
 } QTXSend;  
   
 unsigned char QTXControl = 0; // make = 11 to transmit byte in TXQSend 
 unsigned char QRXCount = 0; // shift in counter 
 char QRXin = '\0'; // QRX recieve byte 
   
 void InitializeQART(void); 
 void interrupt ISR(void); 
   
 main(void) { 
     InitializeQART(); 
     INTCONbits.PEIE = 1; 
     INTCONbits.GIE = 1; 
     QTXSend.byte = 'a'; // test 
     QTXControl = 11; // test 
     while (1) { 
   
     } 
 } 
   
 void InitializeQART(void) { 
     TRISBbits.RB0 = 1; // QRX in (INT0\ & RB0) 
     TRISBbits.RB1 = 0; // QTX out  
     LATBbits.LB1 = 1; // Space 
     PR2 = QBAUD; // TMR2 104µS QTX clock 
     T2CON = 0b00000100; // Timer2 1:1 postscaler, T2 on, 1:1 prescaler 
     PR4 = QBAUD; // TMR4 104µS QRX clock 
     T4CON = 0b00000000; // Timer4 1:1 postscaler, T4 off, 1:1 prescaler 
     INTCON2bits.INTEDG0 = 0; // falling edge detect enabled on RB.0 
     INTCONbits.INT0IE = 1; // enable INT on falling edge RB.0 
     PIE1bits.TMR2IE = 1; // enable TMR2 (TX) interrupts 
     PIE5bits.TMR4IE = 0; // disable TMR4 (RX) interrupts 
 } 
   
 void interrupt ISR(void) { 
     // QTX 104µS (9600baud,N,8,1) 
     // check QTXControl for 0 before putting a byte into QTXSend 
     // then QTXControl = 11; to send the byte in QTXSend 
     if (PIR1bits.TMR2IF == 1) { // QTX baud clock every 104µS period 
         if (QTXControl != 0) { // copy 11 into QTXControl to send char in QTXSend 
             QTXControl--; 
             if (QTXControl == 10) { // 
                 LATBbits.LATB1 = 0; //  Start bit 
             } else { 
                 if (QTXSend.Shiftbit.LSB == 0) { 
                     LATBbits.LB1 = 0; 
                 } else { 
                     LATBbits.LB1 = 1; 
                 } 
                 QTXSend.byte >>= 1; 
                 QTXSend.Shiftbit.MSB = 1; // fill with 1's 
             } 
         } 
         PIR1bits.TMR2IF = 0; // QTX done 
     } 
     // QRX Start bit detect (INT0 falling edge) 
     if (INTCONbits.INT0IF) { // QRX pin went low 
         if (INTCONbits.INT0IE == 1) { // only process if IE enabled (Start bit) 
             TMR4 = QBAUD >> 1; // add 0.5 bit to sample clock 
             T4CONbits.TMR4ON = 1; // start timer 4 
             INTCONbits.INT0IE = 0; // disable INT on falling edge RB.0 
             PIE5bits.TMR4IE = 1; // enable TMR4 (RX) interrupts 
         } 
         INTCONbits.INT0IF = 0; // always clear IRQ request 
     } 
     // QRX shift bits from PORTB.0 into QRXin 
     if (PIR5bits.TMR4IF == 1) { 
         if (PIE5bits.TMR4IE == 1) { 
             if (PORTBbits.RB0 == 0) { // shift bit into QRXin 
                 QRXin |= 0x80; 
             } else { 
                 QRXin &= 0x7F; 
             } 
             if (QRXCount > 7) { // full byte shifted in 
                 T4CONbits.TMR4ON = 0; // Stop timer 4 
                 PIE5bits.TMR4IE = 0; // ignore timer 4 IE 
                 QRXCount = 0; // clear the bit shift counter 
                 INTCONbits.INT0IE = 1; // enable INT on falling edge RB.0 and wait for next Start bit 
             } else { 
                 QRXin >>= 1; // shift bits in 
                 QRXCount++; // increment bit shift counter and wait 
             } 
         } 
         PIE5bits.TMR4IE = 0; // always clear IRQ request 
     } 
 }
 
The union is a good idea, but I don't know how well the compiler handles it. I would make it simpler and just use a variable:

volatile unsigned char byte;

Volatile, because you are using it inside an ISR.
Then I would check for the LSB:

if (byte & 0x01) {
// Do stuff
}


And to shift and fill with ones:

byte >>= 1;
byte |= 0b10000000;


You can try that if you want and check the disassembly to see if it makes any difference.
Also try to keep ISR routines as short and simple as possible. I'm not saying that your ISR is too complicated.. just don't add too much functionality in it in the future.
 
Last edited:
Define the QRXin like this:

volatile unsigned char QRXin;

Then you can remove the else part (in red) from your code:

if (PORTBbits.RB0 == 0) { // shift bit into QRXin
QRXin |= 0x80;
}
else {
QRXin &= 0x7F;
}


When you shift the QRXin to the right, zeros are always added to the MSB, because QRXin is unsigned.
 
Thanks, for both suggestions. I'll give the unsigned char a shot tonight. As for the union & struct those are right out of the XC8 manual (page 51).

And keeping the ISR efficient and fast is a priority.
 
I rewrote the TX module as suggested, much cleaner to look at.
Code:
    if (PIR1bits.TMR2IF == 1) { // QTX baud clock every 104µS period
        if (QTXControl != 0) { // copy 10 into QTXControl to send char in QTXSend
            if (--QTXControl == 9) { //
                LATBbits.LB1 = 0; //  Start bit
            } else {
                if ((TXREGQ & 1) == 0) {
                    LATBbits.LB1 = 0;
                } else {
                    LATBbits.LB1 = 1;
                }
                TXREGQ = (TXREGQ >> 1) | 0x80; // shift right and fill with 1's
            }
        }
        PIR1bits.TMR2IF = 0; // QTX done
 
A few minor tips;
for() loops are more common and easier to read, so instead of manually setting QTXcontrol and testing on different lines to get 10 events, just do;

for(QTXcontrol=0; QTXcontrol < 10; QTXcontrol++)
{
blah
}
(also note that counting up is generally more common and easier unless you really need to count down for some reason.)

Your;
if ((TXREGQ & 1) == 0)
would have been better as;
if ((TXREGQ & 0x01) == 0)
as it is a bit mask operation, and HEX or binary bitmasks are preferred. You used a HEX bitmask here;
TXREGQ = (TXREGQ >> 1) | 0x80;
which was good.

People have different opinions on bracket placement, but I prefer the two {} brackets to be vertically aligned like this;
if(blah)
{
foob;
}
that really pays off when the project gets large and messy. Yes it costs a little bit of vert space, but you can save a bit here and there by leaving out brackets on single events, like your code;
if ((TXREGQ & 1) == 0) {
LATBbits.LB1 = 0;
} else {
LATBbits.LB1 = 1;
}
could become;
if ((TXREGQ & 0x01) == 0) LATBbits.LB1 = 0;
else LATBbits.LB1 = 1;

Also you were asking about fast efficient code. It's a good habit with limited resource micros to look through the asm output of your compiler. On small PICs I often have the C and ASM pages open side by side, and can save a lot of ROM words and speed by tweaking syntax.

Things like your code;
if (PIR1bits.TMR2IF == 1)
on most compilers would result in asm code of loading W with 1, then doing a SUBWF to test that regiter bit. At least a couple of asm instructions.
if you coded;
if(PIR1bits.TMR2IF)
most compilers will use a single BTFSC asm instruction.

likewise;
if (!blah)
will be faster and less ROM than;
if (blah == 0)
because again an asm bit test instruction will be used instead of a load byte and compare byte.

Compilers will vary, and some will depend on optimisation settings, but either way it's great to know the ASM your compiler makes. :)
 
Last edited:
A few minor tips;
for() loops are more common and easier to read, so instead of manually setting QTXcontrol and testing on different lines to get 10 events, just do;

for(QTXcontrol=0; QTXcontrol < 10; QTXcontrol++)
{
blah
}
(also note that counting up is generally more common and easier unless you really need to count down for some reason.)

He is not doing a loop. He is counting interrupt events that are generated each time the next bit needs to be sent (at baudrate).

Also, counting down and comparing against zero is more efficient:

count = 10;
do {
// stuff
} while (--count)
 
Last edited:
Thanks RB & misterT. As noted it's an interrupt driven routine, no loops just counters.

9,600 baud means only 104 instructions between events @ 1MIPs so those tips are very helpful. (I'll actually be using it on a 16MIPs 18F46K22).

XC8 the free version does slight optimization according to the Microchip forums, I think
if (!blah)
or
if (blah == 0)
compile as the same thing, I'll check the disassembly listing today.

Code formatting / beautifying is done by the editor in MPLABX. I've always fussed over it when programming so I'm happy to pass it off to the editor. I learned Pascal decades ago using Alice Pascal (it did all the brackets, colons, etc...) I'll start putting in 0x01 or 0b00000001 instead of the lazier 1, old habits die hard.

Because I'm just learning I'm trying to avoid shortcuts that make the code hard to read. I have to say I wish I hadn't put off learning C as it's really not bad at all.

Question: Which is customary / recommended?

functions()
main()

or

prototypes();
main()
functions()
 
Because I'm just learning I'm trying to avoid shortcuts that make the code hard to read. I have to say I wish I hadn't put off learning C as it's really not bad at all.

Never write code that is hard to read. The "impressive looking C tricks" are pretty much thing of the past. Compilers today are pretty good at optimizing code. Trying to "outsmart" the compiler by writing some weird obfuscated C usually makes things worse. If you need to really optimize something, use inline assembly. Always try to write readable, modular, well structured code. Give your variables meaningfull names, no matter how long.

Question: Which is customary / recommended?

functions()
main()

or

prototypes();
main()
functions()

If the project is small nobody cares :) .. actually I like to write prototypes on top of the file and comment them with description what the function does.
Large projects are better to split in multiple files. Prototypes (function declarations) in the header file and the actual code (function definition) in separate .c file.
Modularizing C Code: Managing large projects
 
I don't know how "into" C programming you are, but here is an excellent book.

I don't know if that is legally shared. I was suprised that the copy is freely available in pdf. Second result in google. It is $30 in amazon.

Edit: I removed the link. I don't think it was legal. The name of the book is "Expert C Programming: Deep C Secrets"
 
Last edited:
... Compilers today are pretty good at optimizing code. Trying to "outsmart" the compiler by writing some weird obfuscated C usually makes things worse.

No, actually microcontroller compilers are quite terrible at optimising, especially on small PICs. Their idea of "C optimisation" usually means loading variables onto a software stack (ie other variables) which can be of benefit during very large complex math processes, but is terrible optimisation for simple code segments like testing a byte and branching.

And I didn't suggest "writing some weird obfuscated C" I suggested getting to know exactly how the compiler compiles standard program chunks and use well written C that just happens to compile much better than OTHER well written C. ;)

...
If you need to really optimize something, use inline assembly.

I disagree, if you know your compiler well you can write good C that is almost as fast as ASM.

Inline ASM is a kludge that will limit portability and readability of the code, and should be reserved for extreme cases where absolute peak performance is needed at the expense of everything else.

Do you do much C coding where you need to pack a lot of features and speed into very small PICs MisterT? From your replies I'm getting the impression you mainly code on larger platforms where resources are fat and you can just let the compiler "optimise" everything.

To Bill, seriously if you are working with the small PICs then keep the .ASM window open in your text editor next to your .C window, and really spend some time getting to know exactly WHAT your compiler will do with your code. You have the advantage of being an advanced PIC user from the start, it's not like you're a beginner leaning C and PICs all at once.
 
I disagree, if you know your compiler well you can write good C that is almost as fast as ASM.

Inline ASM is a kludge that will limit portability and readability of the code, and should be reserved for extreme cases where absolute peak performance is needed at the expense of everything else.

Do you do much C coding where you need to pack a lot of features and speed into very small PICs MisterT? From your replies I'm getting the impression you mainly code on larger platforms where resources are fat and you can just let the compiler "optimise" everything.

There are things you can not do with C. Like with AVRs when you change the internal clock prescaler. Only inline ASM works. I was talking about that kind of "optimization". You can also write "naked" C-functions and write your own function entry and exit codes with inline asm if you want to optimize the overhead. I use that for ISR functions (ISR functions are rarely portable anyway). I write C only for microcontrollers. I use .Net for "large platforms". avr-gcc is very good in optimizing code for microcontrollers.. I don't know about PIC compilers.. Maybe the cheap/free ones are bad.
 
Last edited:
And I didn't suggest "writing some weird obfuscated C"

I didn't say you suggested that.. My reply was to blueroomelectronics who said "I'm trying to avoid shortcuts that make the code hard to read." I was worried that he thinks that "pro C" should be cryptical, obfuscated or hard to read.. which is not the case. Good C is easy to read, well structured and logical.

All the things you pointed out in post #6 are very good points.
 
Do you do much C coding where you need to pack a lot of features and speed into very small PICs MisterT?

Sorry if I'm flooding the thread, but I have to tell the story of the most "insane" code I wrote for 8-bit AVR. It was when I wrote my bachelor thesis. It was about "compressed sampling". That requires a large matrix (MxN) multiplication with a a long vector (Nx1). We wanted to test some things in 8bit AVR. I ended up writing a function that created op-code for a function that does that multiplication.
The prototype of the function was:

C:
typedef void (*function_pointer)(int16_t* matrix, int16_t* vector, int16_t* result); // Typedef for function pointer that takes in a matrix and a vector pointer, and a pointer to the result vector.

/* Prototype of the function that creates a function to multiply MxN matrix with Nx1 vector. */
function_pointer create_function(uint8_t m, uint8_t n);

This function needed to be placed in the bootloader memory because it needed to be able to write op-codes in the program memory. The function which was created was optimized for a constant size matrix MxN. Purely academic, but very educational. I used fixed point math.
 
Last edited:
XC8 the free version does slight optimization according to the Microchip forums, I think
if (!blah)
or
if (blah == 0)
compile as the same thing, I'll check the disassembly listing today.

That's not optimisation, they compile as the same thing because they are the same thing. Comparing with zero in this case is effectively the same thing as a boolean logical NOT.

What good embedded C compilers (including XC8) will do is efficiently use the single bit test operations which are available on the target architecture. For example on a PIC it could use btfsc.
 
if ((TXREGQ & 1) == 0)
would have been better as;
if ((TXREGQ & 0x01) == 0)
as it is a bit mask operation, and HEX or binary bitmasks are preferred.

Preferred by who? Certainly not by the compiler. I think it should be made explicitly clear where code is actually inefficient vs. stylistic preference.

It doesn't matter if you express 1 as 1, 0x1, 0x01, 0b1. Coding style is like religion or politics.
 
That's not optimisation, they compile as the same thing because they are the same thing. Comparing with zero in this case is effectively the same thing as a boolean logical NOT.

What good embedded C compilers (including XC8) will do is efficiently use the single bit test operations which are available on the target architecture. For example on a PIC it could use btfsc.

Could it? Does it? Have you checked? ;)

Can you post your compiler ASM output for if(var==0) and if(var==1) and if(var.F0==0) and if(var.F0==1)?

...
re; if ((TXREGQ & 0x01) == 0)
as it is a bit mask operation, and HEX or binary bitmasks are preferred.


Preferred by who? Certainly not by the compiler. I think it should be made explicitly clear where code is actually inefficient vs. stylistic preference.

It doesn't matter if you express 1 as 1, 0x1, 0x01, 0b1. Coding style is like religion or politics.

If it is a bit mask then it's good coding to use HEX or binary mask;
& 0x01 (or)
& 0b00000001

the reason is because the following code might use a different bit;
& 0x40 (or)
& 0b01000000

if you don't keep to the convention of using HEX or binary for bitmasks, what do you do?
Use the decimal radix equivalent of the bit, like in your & 1 example?
& 64 ???
That's more than a "stylistic preference".

What if the mask has 2 or 3 bits? Using a decimal radix would be very poor coding style compared to using HEX or binary.
 
Could it? Does it? Have you checked? ;)

Can you post your compiler ASM output for if(var==0) and if(var==1) and if(var.F0==0) and if(var.F0==1)?

Yes, I did check before posting using the most recent XC8 in both pro and free mode with a very large project. In both the compiler is sensible enough to understand that single bit operations can be achieved with a btfsc/btfss (target was 18F).

This morning I created a small test project to see if I could replicate it (to make sure before I replied here!). I used the following C code which is representative of a real world single bit set and test. I tested the variations alternately so that I could be sure optimisation wasn't having any effect.

Code:
#include <xc.h>

int main(int argc, char** argv) {
    IRCF2 = 1;
    IRCF1 = 1;
    IRCF0 = 1;

    // One of these lines tested in each instance
    while (!IOFS);
    //while (IOFS == 0);

    while(1);
}

I have stripped a few blank lines and comments below. In free mode it seems the compiler actually pads with some useless jumps and includes unreachable bloat code like a jump back to start (!), but I'd need to spend more time looking at that.

Code and .lst files in attachment.

Free mode, no optimisation, logical NOT and == 0, both use a btfss.

Code:
   525  001FE8                     l596:
   527  001FE8  8CD3               	bsf	c:(32414/8),(32414)&7	;volatile                        
   530  001FEA  8AD3               	bsf	c:(32413/8),(32413)&7	;volatile                     
   533  001FEC  88D3               	bsf	c:(32412/8),(32412)&7	;volatile                    
   536  001FEE  D000               	goto	l11
   538  001FF0                     l12:
   539                           opt subtitle "HI-TECH Software Omniscient Code Generator (Lite mode) build 49521"
   540  001FF0                     l11:
   541  001FF0  A4D3               	btfss	c:(32410/8),(32410)&7	;volatile
   542  001FF2  D001               	goto	u11
   543  001FF4  D001               	goto	u10
   544  001FF6                     u11:
   545  001FF6  D7FC               	goto	l11
   546  001FF8                     u10:
   547  001FF8  D000               	goto	l14
                     
   552                           opt pagewidth 120
   553  001FFA                     l14:
   554                           opt subtitle "HI-TECH Software Omniscient Code Generator (Lite mode) build 49521"
   555  001FFA                     l15:
   556  001FFA  D7FF               	goto	l14
   557                           opt subtitle "HI-TECH Software Omniscient Code Generator (Lite mode) build 49521"
   558  001FFC                     l16:             
   561  001FFC                     l17:
   563  001FFC  EF00  F000         	goto	start

Pro mode, default optimisation. both logical NOT and == 0 generate a btfss:

Code:
   55                           ;main.c: 4: IRCF2 = 1;
    56  001FF4  8CD3               	bsf	4051,6,c	;volatile
    57                           
    58                           ;main.c: 5: IRCF1 = 1;
    59  001FF6  8AD3               	bsf	4051,5,c	;volatile
    60                           
    61                           ;main.c: 6: IRCF0 = 1;
    62  001FF8  88D3               	bsf	4051,4,c	;volatile
    63  001FFA                     l11:
    64  001FFA  A4D3               	btfss	4051,2,c	;volatile
    65  001FFC  D7FE               	goto	l11
    66  001FFE                     l14:
    67  001FFE  D7FF               	goto	l14

Interestingly the free mode obviously does some optimisation, as changing the "optimization set" option to "none" instead of "default" (--opt=none on the commandline) actually makes the compiler generate worse code in some instances. This is very noticeable with the large project I compiled where free mode with no optimisation produces the most dumb code imaginable.

And when testing you need to be careful if running from MPLABX, sometimes setting "free" mode is ignored if you have a valid pro license (or the 30 day trial)!

Compiler optimisation is a fun thing to poke around at, but maybe we could start a thread for further discussion if required?
 

Attachments

  • XC8 optimisation test.zip
    87.8 KB · Views: 250
Strange
unsigned char QTXControl is a number from 0 to 10
if (QTXControl != 0) works
if (!QTXControl) does not

Is the shortcut only for boolean?

Also I kind of like using 0bxxxxxxxx for bit compares, it just makes the code very easy to follow till I get the hang of C
 
Status
Not open for further replies.

Latest threads

New Articles From Microcontroller Tips

Back
Top