Moving on to C

[blueroomelectronics]

MPLABX default template for XC8 is

void main(int argc, char** argv)

always wondered why?

MPLABX also has a source code formatter.

 

[Mr RB]

I am a little surprised a compiler does that (bolded) for setting a port bit in the 16F/18F series.
...

It does BSF PORTB,5 on PIC 16F, and BSF LATB,5 on PIC 18F. Is that what you are asking?

My main point is that a good embedded compiler will use a single instruction to set a port bit, as that is the smallest/fastes/best code it can make.

...
I am less sure about using IORWF since that is a byte operation, but I could not find a definitive statement, i.e., from Microchip, saying it was safe.
...
Edit#2: Here is a PicList link that states andwf, iorwf, and xorwf are not "safe" from r-m-w (as suspected). Since no one challenged it, I assume it is accurate:

Interesting. I'm curious now what asm code other compilers generate for a C command to set a port bit?

 

[jpanhalt]

My comment was not intended to be a question.

In the PIC's, writing to a port with bsf or bcf, as your MikroeC apparently does, can subject one to the Read-Modify-Write problem (r-m-w). A nice description of it from Mikroe is here: https://www.mikroe.com/download/eng/documents/compilers/mikroc/pro/pic/help/rmw.htm

I was a bit surprised with your claim that the MikroeC compiler uses the BSF instruction for the affected chips, which I did not see as a virtue (if it is really done that way). One way around r-m-w is to use a shadow register, which may be what the "lesser" compilers you reference are actually doing; although, that is not entirely clear from the code you posted. In short, I don't think this is solid ground on which to compare the virtues of one C compiler over another, unless one is absolutely certain the disassembly listing shows exactly what was done (See: Dario's comment, below).

As you noted, in the 18F, one can and should use the port latches (LATx). That applies to the enhanced 16F series chips as well.

Now for an interesting tidbit, which I was not aware of, a disassembly listing of C may show something like "bsf porta,5", when in fact what was done was "bsf lata,5" . That quirk is explained by Dario on the Microchip forums: "If I am not wrong, since PORTx and LATx have the same address but different bank, the Disassembly windows is simply showing a "dumb" name for that SFR at that address - but the "job" is done correctly!"

Regards, John

 

[Overclocked]

I'm listing few observations about your code.

- You should be able to use simple "void main(void)" instead of "void main(int argc, char** argv)".
- You are not calling the "IntADC()" -function properly.. the line "IntADC;" does nothing.
- It would be better style not to omit braces {}. Makes the code easier to read and prevents future bugs (you might add more lines and forget to add the braces).
- Learn to write macros.. makes writing code easier and results in more readable and flexible code..and prevents bugs. Just don't go crazy with macros.

I generally like your style. You could separate the ADC conversion to a function like "int read_adc();" which starts ADC conversion, waits it to finish and returns the value.

Thanks. I forgot where I found the function stuff from, I think I may have saw how someone used it to define the internal osc, and so figured I could make a function for setting up the ADC and do it the same way. However, Im having a little big of trouble. Ive read the manual and searched around, but I cant figure out how to properly call the function.

The program is also very small for some reason, the hex file shows just 3 words. I know the program is small, but not that small! Plus, I programmed it, and it didnt work. Something tells me it may have to do with those functions. I could always stick it in my main program but then I think it would be hard to follow

ADD: I think I got it working, but Im leaving my reply here just to make sure I did call the function correctly. I basically did it as such :

...
(SetupClock());
    (IntADC());
...

The LED lights, but not at 2.5v (>512). Instead it lights up at around 20mV..But the PIC must be reading something, right?

Is it possible to write to two different bytes of a 16 bit word? ie, byte 1 and byte 2 of the 16 bit variable.

 

[nsaspook]

Is it possible to write to two different bytes of a 16 bit word? ie, byte 1 and byte 2 of the 16 bit variable.

There are about 10 was to do it but it's common to use the union method:
https://www.tutorialspoint.com/cprogramming/c_unions.htm

timer.lt is the 16 bit value
timer.bt[0] is the lower 8 bit value of timer.lt
timer.bt[1] is the upper 8 bit value of timer.lt

From the C18 headers

#include <p18cxxx.h>
 #include <timers.h>
 
 /********************************************************************
 *    Function Name:  ReadTimer1                                     *
 *    Return Value:   char: Timer1 16-bit value                      *
 *    Parameters:     void                                           *
 *    Description:    This routine reads the 16-bit value from       *
 *                    Timer1.                                        *
 ********************************************************************/

 unsigned int ReadTimer1(void)
 {
   union Timers timer;
 
   timer.bt[0] = TMR1L;    // Read Lower byte
   timer.bt[1] = TMR1H;    // Read upper byte
 
   return (timer.lt);      // Return the 16-bit value
 }

 /******************************************************************************
  // *                  TIMERS PERIPHERAL LIBRARY HEADER FILE
  ******************************************************************************
  * FileName:                timers.h
  * Dependencies:        See include below
  * Processor:               PIC18
  * Compiler:                MCC18
  * Company:                 Microchip Technology, Inc.
  ...
  *****************************************************************************/
 #include <pconfig.h>
 
 /* PIC18 timers peripheral library. */
 
 /* used to hold 16-bit timer value */
 union Timers
 {
   unsigned int lt;
   char bt[2];
 };

 

[Pommie]

@John,

The RMW problem only occurs when pins are excessively loaded or two bits are changed in two consecutive instructions such that the capacitance of the pin causes it to read back wrong. In the 20 years (Wow, is it that long) I've been programming pics, I've never encountered the RMW problem. Hence, compilers should use the most efficient method and not worry about a very rare problem.

Mike.

 

[Mr RB]

... In the PIC's, writing to a port with bsf or bcf, as your MikroeC apparently does, ...
... your claim that the MikroeC compiler uses the BSF instruction ...
... (if it is really done that way) ...
...

Ouch! Three doubts? It sounds like you think I am telling fibs or that I really don't have a clue what the compiler is doing?

Please remember I have been a hardcore PIC enthusiast since PICs were ceramic and had windows on top. And MPASM was a blue screen running under DOS!

Here is the output of my two PIC C compilers; MikroC v8 and MikroC PRO v4.15.

// compiled for PIC 12F675 using MikroC v8;
void main ()
{
  CMCON =  0b00000111;        // comparators OFF
  GPIO.F2 = 1;
}

// result;
$0000	$2804			GOTO	_main
$0004	$	_main:
;blah.c,19 :: 		void main ()
;blah.c,23 :: 		CMCON =  0b00000111;        // comparators OFF
$0004	$3007			MOVLW	7
$0005	$1303			BCF	STATUS, RP1
$0006	$1283			BCF	STATUS, RP0
$0007	$0099			MOVWF	CMCON
;blah.c,26 :: 		GPIO.F2 = 1;
$0008	$1505			BSF	GPIO, 2
;blah.c,30 :: 		}
$0009	$2809			GOTO	$

---------------------------------------------------
// compiled for PIC 18F1320 using MikroC v8;
void main ()
{
  TRISA =  0b00010000;
  LATA.F2 = 1;
}

// result;
$0000	$EF04	F000			GOTO	_main
$0008	$	_main:
;blah.c,1 :: 			void main ()
;blah.c,3 :: 			TRISA =  0b00010000;
$0008	$0E10	    			MOVLW	16
$000A	$6E92	    			MOVWF	TRISA, 0
;blah.c,4 :: 			LATA.F2 = 1;
$000C	$8489	    			BSF	LATA, 2, 0
;blah.c,5 :: 			}
$000E	$D7FF	    			BRA	$

---------------------------------------------------
// compiled for PIC 12F675 using MikroC PRO v4.15;
void main ()
{
  CMCON =  0b00000111;        // comparators OFF
  GPIO.F2 = 1;
}

// result;
;blah.c,1 ::                 void main ()
;blah.c,3 ::                 CMCON =  0b00000111;        // comparators OFF
        MOVLW      7
        MOVWF      CMCON+0
;blah.c,4 ::                 GPIO.F2 = 1;
        BSF        GPIO+0, 2
;blah.c,5 ::                 }
        GOTO       $+0

---------------------------------------------------
// compiled for PIC 18F1320 using MikroC PRO v4.15;
void main ()
{
  TRISA =  0b00010000;
  LATA.F2 = 1;
}

// result;
;blah.c,1 ::                 void main ()
;blah.c,3 ::                 TRISA =  0b00010000;
        MOVLW       16
        MOVWF       TRISA+0 
;blah.c,4 ::                 LATA.F2 = 1;
        BSF         LATA+0, 2 
;blah.c,5 ::                 }
        GOTO        $+0

I'm still not sure what you were saying. If you are against the compiler using BSF as there is a potential hardware problem on PIC16F series, what would be better?

I originally stated this;
MOVLW (mask)
IORWF PORTB
was bad, and this;
BSF PORTB,2
was good.

On PIC 18F, neither can get the RMW bug (if you use LATB), but the second example (MikroC) is twice as fast and only uses half the ROM.

On PIC 16F, BOTH HAVE RMW BUG EQUALLY but the second example is twice as fast and uses half the ROM.

Please show the result from YOUR compiler if you think it is better.

Unlike Pommie I have had issues with RMW bug in real apps, but not for a lot of years since I started good habits of writing to port pins at the right times or in some cases using a shadow register. However this is not a problem with the compiler, it's a hardware problem to do with pin loadings, which is the user's job to keep on top of. The compiler simply has the job of making the smallest fastest code, and BSF is it.

 

[Ian Rogers]

I find all this talk about RMW on mid range PIC's academic.... There is a work round and I would assume most of the compilers out there use it.

The RMW problem is only apparent when driving capacitive loads... Most programmers don't even see a problem in normal use.

As a work round ... Pace a small delay between port changes or use a shadow port buffer as the pic18's do....

If I want a fast port pin toggle, I will design around the problem or use a pic18..

 

[jpanhalt]

I find all this talk about RMW on mid range PIC's academic.... There is a work round and I would assume most of the compilers out there use it.

I agree with your assumption regarding what most good compilers would be expected to do, because their writers have no way of knowing what the load on a port pin may be.

As for dismissing the subject as academic, capacitive loads are probably not all that uncommon at the hobbyist level. We see a lot of examples on ETO of driving mosfets directly, and the r-m-w issue is frequently raised on the Microchip forum. Finally, hardly a day goes by that some poster on ETO is not reminded to consult the data sheet for a device. Here is what Microchip says:

Source=Microchip
To avoid this with PIC16 devices, you never bsf/bcf a port, you simply read the port, write it to another register, modify that register, then read that register and write it back to the port. This is known as "shaddowing". The PIC18 is easier, since instead of needing to do that, you write to the LATx register directly, thus skipping the read step in hardware.

That is why I expressed surprise that the "best" compiler* didn't use one of the available work arounds (e.g., a mask and movwf, or a delay) and didn't follow the unambiguous instruction of the chip manufacturer. Perhaps, the real purpose of the extra steps in the "lesser" compilers we are warned about is to introduce a delay as you (Ian) suggest.

I have written MikroElektronika to gets its response as to setting bits in a port.

John

*

Source=Mr RB
MikroC is probably the best on the market re PIC 16F/18F and has extremely comprehensive library functions ready to go for just about any peripheral. It also accepts bit manipulation and defines, which compile to good assembler, so
PORTB.F5 = 1;
assembles to a single instruction;
BSF PORTB,5

beware of lesser compilers that will compile a single bit set instruction to slower/fatter/worse assembler like;
MOVLW (mask)
IORWF PORTB

 

[Ian Rogers]

I had a project that required the use of several TTL Mosfets ( On reflection this was a bad idea, but anyway) they would misbehave badly... Sometimes not switching off.

Point is I had to sort out the driving of the mosfet.... ( ended up with darilingtons ) but the mosfets just required a largish resistor to ground to get rid of the capacitance and all was well....

It was, according to most, a RMW error as the switching of several fets on the port the wrong way to do it... I was BSF and BCF ing ( in C ) but at the end of he day.. he resistors prevailed.. not the shadowing of the port..

 

[jpanhalt]

Good news and another work around.

BTW, as a measure of the frequency of concerns (which I consider distinct from showing an actual cause and effect), I searched Microchip (all forums) for some terms and got the following results:

Search             Pages

16F628           278
16F877           188
16F84             298
RMW               >300
16F1519        <1 (6 posts total)

Perhaps it is just coincidental that the 16F1519 has port latches.

John

 

[Mr RB]

...
That is why I expressed surprise that the "best" compiler* didn't use one of the available work arounds (e.g., a mask and movwf, or a delay) and didn't follow the unambiguous instruction of the chip manufacturer.

There aren't any compiler "work arounds". Any option would be invasive. Using shadow registers robs the user of 1 RAM per port, and would require some very invasive compiler activity to constantly analyse all source code and decide what affects ports and what does not. It would be a nightmare!

...
Perhaps, the real purpose of the extra steps in the "lesser" compilers we are warned about is to introduce a delay as you (Ian) suggest.

Wow this is going from bad to worse! Now you are suggesting PIC C compilers should generate slow, bloated code, just in case the user has too much capacitance on a port pin???

How much should the compiler bloat it? Enough to tolerate 100pF on a port pin? Or should they bloat it out REALLY SLOW to allow for users who might have 10nF on a port pin?

...
I have written MikroElektronika to gets its response as to setting bits in a port.
...

There was no need. I provided FOUR cases of dissassembly proof above of exactly what two of their compilers ACTUALLY DO. And their web page states that writing to the ports using their compiler has RMW issues (as do all PIC C compilers).

I did kind of expect an apology, since you expressed 3 lots of doubt about my statement (and not too politely), and then I took the time to prove I was right by creating 4 examples and posting the dissassembly data.

Source=Microchip
To avoid this with PIC16 devices, you never bsf/bcf a port, you simply read the port, write it to another register, modify that register, then read that register and write it back to the port. This is known as "shaddowing". ...

This quote is wrong too. You cant READ the port, modify data and write back. That is exactly what causes RMW. The only correct way to use a shadow register is to keep it totally separate from port reads, and only EVER WRITE data to the shadow reg, then only WRITE the shadow reg to the port using MOVWF.

 

[Pommie]

Source=Microchip
To avoid this with PIC16 devices, you never bsf/bcf a port, you simply read the port, write it to another register, modify that register, then read that register and write it back to the port. This is known as "shaddowing". The PIC18 is easier, since instead of needing to do that, you write to the LATx register directly, thus skipping the read step in hardware.

This will still have the RMW problem if a pin is excessively loaded. Where did you find this?

Mike.

 

[misterT]

Well this stranded far away from C programming.. I would like to talk about C programming.

 

[killivolt]

Well this stranded far away from C programming.. I would like to talk about C programming.

What? we did, When?

Doh, I'm going to have to read the hole thing again.....

Just when I thought I was getting it

 

[Overclocked]

There are about 10 was to do it but it's common to use the union method:
https://www.tutorialspoint.com/cprogramming/c_unions.htm

timer.lt is the 16 bit value
timer.bt[0] is the lower 8 bit value of timer.lt
timer.bt[1] is the upper 8 bit value of timer.lt

For some reason I couldnt get this to work. Said I needed a structure and didnt reconize "bt" and such. I'll experiment more with this later to see if I can get it to work.

But anyway. Ive gotten my code to work fully! I'll post it here for all to see. Some changes though. It seems that after I wait for the ADC to be done, a "return" is needed. Hrm. Why? I also removed the "continue" in that same while statement. It didnt make sense, Im telling the PIC to wait, but continue at the same time. I dont know which action caused my circuit to work (ie removing the continue or adding in a return) but it works!

/* 
 * File:   newmain1.c
 * Author: chris
 *
 * Created on August 19, 2013, 6:11 PM
 */

#include <xc.h>
#include <stdio.h>
#include <stdlib.h>
#include <htc.h>

#define _XTAL_FREQ 8000000
#define RA5 (PORTAbits.RA5)

#pragma config FOSC = INTOSC, CLKOUTEN = OFF, PLLEN = OFF, WDTE = OFF, LVP = OFF, MCLRE = ON


void SetupClock (void) //set up clock
{
    OSCCON = 0b01110010;

}
void IntADC (void) //setup ADC
 {
         //** Initalise Ports FOR ADC **//
         //PORTA = 0x00;
         TRISA = 0b00001111; //RA4,RA5 outputs, RA0, RA1 Inputs
         //** Set Up ADC Parameters **//
         ANSELA = 0b00000111; //RA4 Output, RA5 Output, all others input
         ADRESH = 0x00; //Set the analog high bits to 0
         ADRESL = 0x00;
         ADCON1 = 0b10010000; // Sets ADRESL to contain the first 7 bits of conversion, ADRESH will have the final 3 bits.
 }    // void InitADC(void)


int main (){
    SetupClock();
    IntADC();
    unsigned int ADCresult;

    ADCresult = 0;
   
   
     ADCON0bits.ADON = 1; //Turns on ADC module
     ADCON0bits.CHS = 2; //AN0 as input

     while (1)
     {
       
       ADCON0bits.GO = 1; //Starts Conversion
       __delay_us(35);
        while (ADCON0bits.GO_nDONE); //wait till ADC conversion is over
            {      
             ADCresult = (ADRESH <<8 | ADRESL) ;
            
             if (ADCresult > 0b1000000000)
                RA5 = 1;
               else 
                 RA5 = 0;

             return ADCresult; //THIS is important. Function doesnt seem to work without it. 
             //also removed the continue statement.
            }
     }
}

Also, if future people are wondering why I have only certain channels set up to read, its because Im setting it up for my final application, which is actually just to check if a battery is charged, and then shut off the charger.

ADD: I also just want to add that Nigels code did work. I used the little tid bit where the ADRESH and ADRESL come together, thats when I noticed he used a return function on that while loop.

 

[Pommie]

The reason it wants a return is because you have declared main to return an int. Change it to void main() and it should be happy.

BTW, returning from main is not a good idea as it has nowhere to go except to crash.

Mike.

 

[Overclocked]

The reason it wants a return is because you have declared main to return an int. Change it to void main() and it should be happy.

BTW, returning from main is not a good idea as it has nowhere to go except to crash.

Mike.

yeap! Works perfectly. Why Does C seem more finicky than Basic to get things working? Its a nice advanced language, but ever so picky..

Not C Question: is a Delay really needed once I start the ADC (ie GO = 1). Cant I just keep polling the done bit for completion?I did modify my code to remove the delay and it still worked!

Final Code:

/* 
 * File:   ADC_Ver1
 * Author: chris
 * This is Basic ADC Code for the PIC12F1840 Series. Should be portable to others
 * Created on August 19, 2013, 6:11 PM
 */

#include <xc.h>
#include <stdio.h>
#include <stdlib.h>
#include <htc.h>


#define _XTAL_FREQ 8000000
#define RA5 (PORTAbits.RA5)

#pragma config FOSC = INTOSC, CLKOUTEN = OFF, PLLEN = OFF, WDTE = OFF, LVP = OFF, MCLRE = ON


void SetupClock (void) //set up clock
{
    OSCCON = 0b01110010;

}
void IntADC (void) //setup ADC
 {
         //** Initalise Ports FOR ADC **//
         //PORTA = 0x00;
         TRISA = 0b00001111; //RA4,RA5 outputs, RA0, RA1 Inputs
         //** Set Up ADC Parameters **//
         ANSELA = 0b00000111; //RA4 Output, RA5 Output, all others input
         ADRESH = 0x00; //Set the analog high bits to 0
         ADRESL = 0x00;
         ADCON1 = 0b10010000; // Sets ADRESL to contain the first 7 bits of conversion, ADRESH will have the final 3 bits.
          }    // void InitADC(void)


void main ()
{
    SetupClock();
    IntADC();
    unsigned int ADCresult; //ADCresult for Channel x
    ADCresult = 0;
    ADCON0bits.ADON = 1; //Turns on ADC module
    ADCON0bits.CHS = 2; //AN2 as input   
    ADCON0bits.GO = 1; //Starts Conversion
       __delay_us(35); //small delay to get value
        while (ADCON0bits.GO_nDONE); //wait till ADC conversion is over
            {      
             ADCresult = (ADRESH <<8 | ADRESL) ;
            
             if (ADCresult > 768) 
                RA5 = 1;
               else 
                 RA5 = 0;

           
            }
     
}

Also I didnt mind the **** chat about the port b stuff. It actually helped me since I had no clue how to define a port that way

 

[Pommie]

You can check the GO bit instead of a delay but you do need a delay between selecting the channel / turning ADC on and setting the GO bit to allow the internal capacitor to charge.

Mike.

 

[Overclocked]

Now to move on to harder things. I need to read two analog ports, one after the other. Speed isnt necessary. I know how to successfully read from one port, thats simple. I figured I could just redo what I did for one port. However, the following code doesnt work. One Analog port works (AN0), but now AN2 doesn't.

It seems others have had the same issues. The answer to their problems were to just put in a delay, so I did just that. All of my variables are set up for the final application as you can see, but for right now, Im just testing them.

Also, Im pretty sure I can make some nice function to simplify all of this.

/*
 * File:  Battery Charger Controller
 * Author: chris
 * Checks if the Battery is equal to, or greater than the charge voltage, then turns off a PFET
* and turns on a "done" led. But for now, turns on a LED when values >512
 * Created on August 27, 2013, 6:11 PM
 */

#include <xc.h>
#include <stdio.h>
#include <stdlib.h>
#include <htc.h>


#define _XTAL_FREQ 8000000
#define RA5 (PORTAbits.RA5) //Charge LED
#define RA4 (PORTAbits.RA4) // PFET

#pragma config FOSC = INTOSC, CLKOUTEN = OFF, PLLEN = OFF, WDTE = OFF, LVP = OFF, MCLRE = ON


void SetupClock (void) //set up clock
{
    OSCCON = 0b01110010;

}
void IntADC (void) //setup ADC
 {
         //** Initalise Ports FOR ADC **//
         //PORTA = 0x00;
         TRISA = 0b00001111; //RA4,RA5 outputs, RA0, RA1 Inputs
         //** Set Up ADC Parameters **//
         ANSELA = 0b00000111; //RA4 Output, RA5 Output, all others input
         ADRESH = 0x00; //Set the analog high bits to 0
         ADRESL = 0x00;
         ADCON1 = 0b10010000; // Sets ADRESL to contain the first 7 bits of conversion, ADRESH will have the final 3 bits.
         ADCON0bits.ADON = 1;
}    // void InitADC(void)


void main (){
    
    SetupClock();
    IntADC();
    unsigned int Vcharge; //ADCresult for Channel x
    unsigned int Vcurrent;
    unsigned int Vbattery;
    Vcharge, Vcurrent, Vbattery = 0;
    ADCON0bits.CHS = 2; //AN2 as input
     __delay_ms (10);
     ADCON0bits.GO = 1;
       //__delay_ms (10); //small delay to get value
         while (ADCON0bits.GO_nDONE); //wait till ADC conversion is over
            {
             Vcharge = (ADRESH <<8 | ADRESL) ;

             }
    __delay_ms (100);
    IntADC();
    ADCON0bits.CHS = 0;
    __delay_ms (10);
    ADCON0bits.GO = 1;
   // __delay_ms (10);
    while (ADCON0bits.GO_nDONE);
    { 
      Vcurrent = (ADRESH <<8 | ADRESL);
    }
    //Vbattery = Vcharge - Vcurrent;

    if (Vcharge >512)

    {
        RA4 = 1;

    }
        else
        RA4 = 0;


    if (Vcurrent > 512 )
    {
        RA5 = 1;
    }
    else
        RA5 = 0;
 }