1

Can someone look at my code? I'm trying to get 8 LEDs to rotate as I rotate the potentiometer, but four LEDs stay on the whole time not rotating at all.

#include <xc.h>

/********************************************************

* xx/xx/xx - Put date in place of the x's
*
********************************************************/


/********************************************************
* Function: main
*
* Description:  D0 - D7 Display the results of the ADC
*
* Notes:
*
* RA0 - Input from RP1
*
*
*
* Returns:  This routine contains an infinite loop
*
********************************************************/

/* Configuration Word */

#pragma config PWRTE = OFF      // Power-up Timer Enable bit (PWRT disabled)
#pragma config MCLRE = OFF      // MCLR Pin Function Select bit (MCLR pin function is digital input, MCLR internally tied to VDD)
#pragma config CP = OFF         // Code Protection bit (Program memory code protection is disabled)
#pragma config CPD = OFF        // Data Code Protection bit (Data memory code protection is disabled)
#pragma config BOREN = OFF      // Brown Out Detect (BOR disabled)
#pragma config IESO = OFF       // Internal External Switchover bit (Internal External Switchover mode is disabled)
#pragma config FCMEN = OFF      // Fail-Safe Clock Monitor Enabled bit (Fail-Safe Clock Monitor is disabled)



void PORTA_init(void);
void ADC_Disp(void);
void Delay_LED_On(void);

int ADC_Value = 0;

const char PORTA_Value[8] = {
    0b010000,   // D0
    0b100000,   // D1
    0b010000,   // D2
    0b000100,   // D3
        0b100000,   // D4
        0b000100,   // D5
        0b000100,   // D6
        0b000010};  // D7

const char TRISA_Value[8] = {
    0b001111,   // D0
    0b001111,   // D1
    0b101011,   // D2
    0b101011,   // D3
        0b011011,   // D4
        0b011011,   // D5
        0b111001,   // D6
        0b111001};  // D7

main()
{

    PORTA_init();

    ANSEL = 1;                  //  Just RA0 is an Analog Input
    TRISA0 = 1;                 //  Corresponding TRIS bit is set as input

    ADCON0 = 0b00000001;        //  Turn on the ADC
                                //   Bit 7      - Left Justified Sample
                                //   Bit 6      - Use VDD
                                //   Bit 4:2    - Channel 0
                                //   Bit 1      - Do not Start
                                //   Bit 0      - Turn on ADC

    ADCON1 = 0b00010000;        //  Select the Clock as Fosc/8


    ADC_Disp();
    GO_DONE = 1;            // Start A/D Conversion


    while(1 == 1)               //  Loop Forever
    {

        if (GO_DONE == 0)           // Is A/D Conversion complete?
            {   ADC_Disp();     // Display A/D Conversion Results
                ADC_Value = ADRESH; // Get new A/D value
                GO_DONE = 1;        // Start the next A/D Conversion
            }
        else                // A/D Conversion still in progress
                ADC_Disp();

    } 
}

/******** END OF main ROUTINE ***************************/

/********************************************************
* Function: PORT_init
*
* Description:  Initializes PORTA to a known condition
*
* Notes:    None
*
* Returns:  None
*
********************************************************/
void PORTA_init(void)
{
    PORTA = 0;                  //  All PORTA Pins are low
    CMCON0 = 7;                 //  Turn off Comparators
    ANSEL = 0;                  //  Turn off ADC

    return;

}
/******** END OF PORTA_init ****************************/



/********************************************************
* Function: ADC_Disp
*
* Description:  Displays the value of A/D Conversion on D0 - D7
*
* Notes:
*
* 
*
* Returns:  None
*
********************************************************/
void ADC_Disp(void)
{

    int i;

    for (i = 0; i < 8; i++ )
        {                                   // Loop through Each of the 8 LEDS

        Delay_LED_On();             // Allows time for individual LEDs to light

            if ((ADC_Value & (1 << i)) == 0)
                PORTA = 0;
            else
                PORTA = PORTA_Value[i];
                TRISA = TRISA_Value[i];
        }  //  

    return;

}
/******** END OF ADC_Disp *************************/

/********************************************************
* Function: delay_LED_On
*
* Description:  Causes a delay in program execution
*
* Notes:
*
*
********************************************************/
void Delay_LED_On(void)
{
    int j;

    for (j = 0; j < 60; j++);       //  Display "On" Loop 

    return;

}
/******** END OF Delay_LED_On *************************/
Rishikesh Raje
  • 8,556
  • 2
  • 16
  • 31

2 Answers2

2

For the other students in your class here is code tested using a PICkit1 starter kit that I suspect does a bit more that you have requested:

/*
 * File:   main.c
 * Author: dan1138
 * Target: PIC16F684
 * Compiler: XC8 v2.00
 * 
 * Description:
 * 
 *  Display the upper 8-bits of the ADC conversion 
 *  from analog input from RA0 on 8 charlieplexed LEDs 
 *  connected to outputs RA1,RA2,RA4,RA5.
 *
 *                       PIC16F684
 *             +------------:_:------------+
 *    GND -> 1 : VDD                   VSS : 14 <- 5v0
 *   DRV5 <> 2 : RA5/T1CKI     PGD/AN0/RA0 : 13 <> POT
 *   DRV4 <> 3 : RA4/AN3       PGC/AN1/RA1 : 12 <> DRV1
 *    VPP -> 4 : RA3/VPP           AN2/RA2 : 11 <> DRV2
 *        <> 5 : RC5/CPP1          AN4/RC0 : 10 <> 
 *        <> 6 : RC4/C2OUT         AN5/RC1 : 9  <> 
 *        <> 7 : RC3/AN7           AN6 RC2 : 8  <> 
 *             +---------------------------:
 *                        DIP-14
 * 
 *           150 OHM
 *  DRV4 ----/\/\/\-------+-----------+-------------+-----------+
 *                        :           :             :           :
 *                        :           :             :           :
 *                       ---         ---            :           :
 *                 LED1  / \         \ / LED0       :           :
 *                       ---         ---            :           :
 *                        :           :             :           :
 *           150 OHM      :           :            ---         --- 
 *  DRV5 ----/\/\/\-------+-----------+       LED3 / \         \ / LED2
 *                        :           :            ---         --- 
 *                        :           :             :           :
 *                       ---         ---            :           :
 *                 LED5  / \         \ / LED4       :           :
 *                       ---         ---            :           :
 *                        :           :             :           :
 *           150 OHM      :           :             :           :
 *  DRV2 ----/\/\/\-------+-----------+-------------+-----------+
 *                        :           : 
 *                        :           :
 *                       ---         ---
 *                 LED7  / \         \ / LED6
 *                       ---         --- 
 *                        :           :
 *           150 OHM      :           :
 *  DRV1 ----/\/\/\-------+-----------+
 *  
 * 
 *  POT ----/\/\/\---+
 *            1K     :
 *                   :
 *                   v
 *  GND ----------/\/\/\-------- 5v0
 *                  10K
 * 
 *
 * Notes:
 *  Charlieplexing, see https://en.wikipedia.org/wiki/Charlieplexing
 * 
 * Created on July 13, 2019, 6:09 PM
 */

#pragma config FOSC = INTOSCIO
#pragma config WDTE = OFF
#pragma config PWRTE = OFF
#pragma config MCLRE = OFF
#pragma config CP = OFF
#pragma config CPD = OFF
#pragma config BOREN = OFF
#pragma config IESO = OFF
#pragma config FCMEN = OFF

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

#define _XTAL_FREQ (8000000ul)
/*
 * Global data
 */
volatile unsigned char gLEDs;

void main(void) 
{
    /*
     * Initialize this PIC
     */
    INTCON = 0;
    OSCCON = 0x70;      /* Select 8MHz system oscillator */
    __delay_ms(500);    /* Give ICSP device programming tool a chance to get the PICs attention */

    TRISA = 0xFF;
    TRISC = 0x00;
    ANSEL  = 0;
    OPTION_REG = 0b11000010; /* TIMER0 clock = FOSC/4, prescale 1:8 */
    PORTA = 0;
    PORTC = 0;
    CMCON0 = 7;
    TMR0 = 0;
    TMR0IF = 0;
    TMR0IE = 1;
    gLEDs = 0b00000000;
    GIE = 1;
    /*
     * Initialize ADC on channel 0
     */
    ADCON0 = 0;
    ADCON1 = 0;
    TRISAbits.TRISA0 = 1;       /* Make RA0 an input */
    ANSELbits.ANS0   = 1;       /* Enable AN0 on the RA0 input */
    ADCON1bits.ADCS  = 0b101;   /* Select FOSC/16 as ADC clock source */
    ADCON0bits.CHS   = 0;       /* Select AN0 as ADC input */
    ADCON0bits.ADFM  = 0;       /* Select left justified data */
    ADCON0bits.VCFG  = 0;       /* Select VDD as VREF source */
    ADCON0bits.ADON  = 1;       /* Turn on ADC */
    /*
     * This is the application loop.
     * 
     * Display 8-bit ADC value in charlieplexed LEDs
     */
    while(1)
    {
        ADCON0bits.GO = 1;      /* Start an ADC conversion */
        while(ADCON0bits.GO);   /* Wait for ADC conversion to finish */
        gLEDs = ADRESH;         /* Put ADC value in LED7 to LED0 */
    }
}
/*
 * Interrupt handlers
 */
void __interrupt() ISR_handler(void)
{
    static uint8_t Timer0Ticks = 0;
    static unsigned char State = 8;
    unsigned char OutBits, HighBits;

    if (TMR0IE && TMR0IF) {  /* TIMER0 asserts and interrupt every 1.024 milliseconds */
        TMR0IF=0;
        if (Timer0Ticks == 0) { /* Select another LED every second TIMER0 interrupt */
            Timer0Ticks = 1;    /* to make LEDs a little brighter make this number larger until you don't like the flickering */

            OutBits  =  0b00000000;
            HighBits =  0b00000000;

            switch (--State)
            {
            case 7:
                if (gLEDs & 0x80)
                {
                    HighBits |= (1 << 1); /* Drive LED7, DRV1=H DRV2=L */
                    OutBits = ~((1<<1)|(1<<2));
                }
                break;

            case 6:
                if (gLEDs & 0x40)
                {
                    HighBits |= (1 << 2); /* Drive LED6, DRV1=L DRV2=H */
                    OutBits = ~((1<<1)|(1<<2));
                }
                break;

            case 5:
                if (gLEDs & 0x20)
                {
                    HighBits |= (1 << 2); /* Drive LED5, DRV5=L DRV2=H */
                    OutBits = ~((1<<5)|(1<<2));
                }
                break;

            case 4:
                if (gLEDs & 0x10)
                {
                    HighBits |= (1 << 5); /* Drive LED4, DRV5=H DRV2=L */
                    OutBits = ~((1<<5)|(1<<2));
                }
                break;

            case 3:
                if (gLEDs & 0x08)
                {
                    HighBits |= (1 << 2); /* Drive LED3, DRV4=L DRV2=H */
                    OutBits = ~((1<<4)|(1<<2));
                }
                break;

            case 2:
                if (gLEDs & 0x04)
                {
                    HighBits |= (1 << 4); /* Drive LED2, DRV4=H DRV2=L */
                    OutBits = ~((1<<4)|(1<<2));
                }
                break;
            case 1:
                if (gLEDs & 0x02)
                {
                    HighBits |= (1 << 5); /* Drive LED1, DRV4=L DRV5=H */
                    OutBits = ~((1<<4)|(1<<5));
                }
                break;

            default:
                if (gLEDs & 0x01)
                {
                    HighBits |= (1 << 4); /* Drive LED0, DRV4=H DRV5=L */
                    OutBits = ~((1<<4)|(1<<5));
                }
                State = 8;
            }

            TRISA |= ((1<<5)|(1<<4)|(1<<2)|(1<<1)); /* Turn off all LED output drivers */

            if (OutBits)
            {
                PORTA &= OutBits;      /* Set both LED drivers to low */
                TRISA &= OutBits;      /* Turn on LED output drivers */
                PORTA |= HighBits;     /* Turn on just one of the two LEDs  */
            }
        }
        else
        {
            Timer0Ticks--;
        }
    }
}
Dan1138
  • 1,150
  • 8
  • 11
  • Thanks, Dan! The outputs (pot's pin 2 = 0V, but RA0 = 2.8V) are D4 = on and D7 = on when all the LEDs should be off. The same thing happened to my version of the code. All LEDs are on when the pot's pin 2 = 5V as expected. – NotSoTechnical Jul 14 '19 at 19:27
  • all LEDs (D0 to D7) – NotSoTechnical Jul 14 '19 at 19:28
  • This code works correctly in my PICkit1 so I suspect you may have a wiring problem or a PIC16F684 with some kind of damage to the RA0 pin. Remember that RA0 is also one of the ICSP programming pins. If the ICSP device programmer is attached it may cause this kind of problem. – Dan1138 Jul 14 '19 at 19:42
  • Hi! I got it fixed! I removed a wire that was connected to the seven segments circuit (you know why I have it) from RA0. Thanks again! – NotSoTechnical Jul 14 '19 at 20:08
  • Yes I know but for others see this [link](https://stackoverflow.com/questions/56919370/questions-about-displaying-0x00-to-0xff-with-two-seven-segment-lights) for some background. – Dan1138 Jul 14 '19 at 20:15
1

See my comment below:

void ADC_Disp(void)
{
    int i;
    for (i = 0; i < 8; i++ )
        {                                   
        Delay_LED_On();            

            if ((ADC_Value & (1 << i)) == 0)
                PORTA = 0;
            else
                PORTA = PORTA_Value[i];
                TRISA = TRISA_Value[i]; // <—- this is not under the else-clause. Add curly-braces.
        }
    return;
}

In the for-loop, this code:

if ((ADC_Value & (1 << i)) == 0)
    PORTA = 0;
else
    PORTA = PORTA_Value[i];
    TRISA = TRISA_Value[i];

.... is semantically equivalent to this:

if ((ADC_Value & (1 << i)) == 0) {
    PORTA = 0;
}
else {
    PORTA = PORTA_Value[i];
}
TRISA = TRISA_Value[i];

So this statement: TRISA = TRISA_Value[i]; happens both when the if-statement is true and when it is false.

Morten Jensen
  • 5,818
  • 3
  • 43
  • 55