Do you think this XC8 C code for PIC16F1503 IS OK?

[Flyback]

This code is to monitor the mains peak voltage, and repeatedly indicate the level to an external data logger via the pic16f1503 DAC.
Our code never works first time, so when we try it tomorrow, do you know what our problems will likely be?
The schematic for this code is also attached.

// PIC16F1503 Configuration Bit Settings
// 'C' source line config statements

// CONFIG1
#pragma config FOSC = INTOSC    // Oscillator Selection Bits (INTOSC oscillator: I/O function on CLKIN pin)
#pragma config WDTE = OFF       // Watchdog Timer Enable (WDT disabled)
#pragma config PWRTE = OFF      // Power-up Timer Enable (PWRT disabled)
#pragma config MCLRE = ON       // MCLR Pin Function Select (MCLR/VPP pin function is MCLR)
#pragma config CP = OFF         // Flash Program Memory Code Protection (Program memory code protection is disabled)
#pragma config BOREN = OFF      // Brown-out Reset Enable (Brown-out Reset disabled)
#pragma config CLKOUTEN = OFF   // Clock Out Enable (CLKOUT function is disabled. I/O or oscillator function on the CLKOUT pin)

// CONFIG2
#pragma config WRT = OFF        // Flash Memory Self-Write Protection (Write protection off)
#pragma config STVREN = ON      // Stack Overflow/Underflow Reset Enable (Stack Overflow or Underflow will cause a Reset)
#pragma config BORV = LO        // Brown-out Reset Voltage Selection (Brown-out Reset Voltage (Vbor), low trip point selected.)
#pragma config LPBOR = OFF      // Low-Power Brown Out Reset (Low-Power BOR is disabled)
#pragma config LVP = OFF        // Low-Voltage Programming Enable (High-voltage on MCLR/VPP must be used for programming)

// #pragma config statements should precede project file includes.
// Use project enums instead of #define for ON and OFF.

#include <xc.h>
#include <stdint.h>
#include <pic16f1503.h>

#define  _XTAL_FREQ 4000000
 
    uint8_t   count;
    uint8_t   count1;
    uint8_t   count2;
    uint8_t   inamp;
    uint8_t   ADCval;
    uint8_t   MAXval;

void doADC(void);
        void doADC(void) {
        RC4 = 1;
        //Initiate ADRESH, ADRESL
        ADRESL = 0x00;
        ADRESH = 0x00;
        ADCON0bits.GO = 1;    //start conversion (GODONE=1)
        while (ADCON0bits.GO) {;}     //Wait for conversion to finish (GODONE CLR)
        ADCval = ADRESH;
        RC4 = 0;
        return;
        }
        
 //Pin useage
        //RA4 (PIN 3)  = AN3 = ADC input
        //RA2 (PIN 11) = AN2 = DACOUT)
        //RC3 = PIN7 = General output
        //RC4 = PIN 6 = GENERAL OUTPUT = ADC indicator (see when ADC is done)
        
void main(void) {
    //Setup registers
    OSCCON = 0x6A;     //Select 4MHz intosc
    TRISA = 0x10;    //ADC input = RA4
    TRISC = 0x00;    //unused so make all outputs.     
    ANSELA = 0x10;   //RA4 = ADC INPUT;RA2 = DAC output
    ANSELC = 0x00;
    INTCON = 0x00; //disable all interrupts
    FVRCON = 0x00;
    ADCON0 = 0x0D;  //Select ADC channel AN3; ADC enabled
    ADCON1 = 0x60;  //Left justified; FOSC/64; VDD=REF
    ADCON0bits.ADON = 1;   //Turn on ADC module
    DACCON0 = 0x90; //DAC enabled, vref=vdd, DACOUT2 enabled
    WPUA = 0x00;    //weak pullups disabled
    PORTA = 0x00;   //zero outputs
    PORTC = 0x00;   //zero outputs
    CM1CON1 = 0x00; //disabled comparator
    CM2CON1 = 0x00; //disabled comparator
    
    __delay_ms(10);
    MAXval = 0;
    

    count = 0;  //Do ten flashes to show your in the "zone".
    while(1){
    LATCbits.LATC3 = 0;
    __delay_ms(10);
    LATCbits.LATC3 = 1;
    __delay_ms(10);
    count = count + 1;
    if (count >= 10) {break;}
    }
    
here:
    //This code picks out the mains peak.
    MAXval = 0;
    count1 = 0;
    while (1) {
    doADC();
    if (ADCval >= MAXval)
            {MAXval = ADCval;}
    count1 = count1 + 1;
    if (count1 >= 100) {break;}
    }
    
    //Now put MAXval representor out on DAC (ie mains peak representor)
    if (MAXval < 0x9C)                        {DACCON1 = 0x00;}    //MAINS <180VAC
    if ((MAXval >= 0x9C) &&  (MAXval < 0xA4)) {DACCON1 = 0x02;}    //MAINS 180-190VAC
    if ((MAXval >= 0xA4) &&  (MAXval < 0xAD)) {DACCON1 = 0x04;}    //MAINS 191-200VAC
    if ((MAXval >= 0xAD) &&  (MAXval < 0xB6)) {DACCON1 = 0x06;}    //MAINS 201-210VAC
    if ((MAXval >= 0xB6) &&  (MAXval < 0xBE)) {DACCON1 = 0x08;}    //MAINS 211-220VAC
    if ((MAXval >= 0xBE) &&  (MAXval < 0xC7)) {DACCON1 = 0x0A;}    //MAINS 221-230VAC
    if ((MAXval >= 0xC7) &&  (MAXval < 0xD0)) {DACCON1 = 0x0C;}    //MAINS 231-240VAC
    if ((MAXval >= 0xD0) &&  (MAXval < 0xD8)) {DACCON1 = 0x0E;}    //MAINS 241-250VAC
    if ((MAXval >= 0xD8) &&  (MAXval < 0xE1)) {DACCON1 = 0x11;}    //MAINS 251-260VAC
    if ((MAXval >= 0xE1) &&  (MAXval < 0xEA)) {DACCON1 = 0x13;}    //MAINS 261-270VAC
    if ((MAXval >= 0xEA) &&  (MAXval < 0xF2)) {DACCON1 = 0x15;}    //MAINS 271-280VAC
    if ((MAXval >= 0xF2) &&  (MAXval < 0xFB)) {DACCON1 = 0x17;}    //MAINS 281-290VAC
    if (MAXval >= 0xFB)                       {DACCON1 = 0x19;}    //MAINS >291VAC
    
    goto here;

        //-----------------------------------------------------------------
        return;
}

 

[Pommie]

The description "doesn't work" isn't helpful. What does it do/not do?

Mike.
BTW, I think this is the first time I've seen goto used in C.

 

[mpgMike]

Why do you have the PIC at all? Why not just send the scaled voltage directly to the data logger? It's not like you're sending values via UART or USB, you're using DAC!

As for the code:
- you don't need "#include <pic16f1503.h>" with XC8, "#include <xc.h>" covers it
- you don't need "return;" in your "void doADC(void)" function as you are not returning anything
- I would do a for() loop instead of while(1) with a break. Not sure how the compiler will deal with that.

Convention would you replace "here:" "goto here;" with "while(1) {" "[code ]" to create your never-ending loop.