What's the wrong with this code ?

General discussion on mikroC PRO for PIC.
Post Reply
Author
Message
zero2004
Posts: 32
Joined: 13 May 2010 13:42

What's the wrong with this code ?

#1 Post by zero2004 » 27 Mar 2012 08:21

Hi every one

I try to make an AC Voltmeter with pic 16f677 and 3 digits seven segment display, the code work fine but there is a small flickers in display, my code is

Code: Select all

sbit G1 at RB4_bit;
sbit G2 at RB5_bit;
sbit G3 at RB6_bit;



unsigned short i, DD1, DD2, DD3, DD4;

long DisplayVolt, ADC_Value;

 unsigned short mask(unsigned short num) {
  switch (num) {
    case 0 : return 0xC0;
    case 1 : return 0xF9;
    case 2 : return 0xA4;
    case 3 : return 0xB0;
    case 4 : return 0x99;
    case 5 : return 0x92;
    case 6 : return 0x82;
    case 7 : return 0xF8;
    case 8 : return 0x80;
    case 9 : return 0x90;
  } 
}

void Display_Data(){
  for (i = 0; i<=30; i++) {

      G1 = 1;
      G2 = 0;
      G3 = 0;
      PORTC = DD1;
      delay_ms(8);
      

      G1 = 0;
      G2 = 1;
      G3 = 0;
      PORTC = DD2;
      delay_ms(8);
      

      G1 = 0;
      G2 = 0;
      G3 = 1;
      PORTC = DD3;
      delay_ms(8);


       G3 = 0;

   }
 }




void main() {
 ADC_Init();
 adcon0 = 0x03;
 adcon1 = 0x40;
  anselh = 0x00;
  ansel = 0x01;
  osccon = 0x70;
  
  TRISA  = 0x0F;
  TRISB  = 0x00;
  TRISC  = 0x00;



 DD1 = mask(0);
 DD2 = mask(0);
 DD3 = mask(0);





  Display_Data();   // Display all zeros


 do {  DG1 = 0;
      DG2 = 0;
      DG3 = 0;

  ADC_Value = adc_read(0);
  ADC_Value = ADC_Value * 352;
  DisplayVolt = ADC_Value / 100000;
  DD1 = mask(DisplayVolt);
  DisplayVolt = (ADC_Value / 10000)%10;
  DD2 = mask(DisplayVolt);
  DisplayVolt = (ADC_Value / 1000)%10;
  DD3 = mask(DisplayVolt);
   Display_Data();
  
 } while(1);


}
Can you help me to find the solution

thanks

supra
Posts: 187
Joined: 08 Apr 2009 11:54
Location: Ontario, Canada

Re: What's the wrong with this code ?

#2 Post by supra » 27 Mar 2012 10:03

Code: Select all

delay_ms(8);
try this:

Code: Select all

delay_ms(1000);
also move ADC_Init() between:

Code: Select all

TRISC  = 0x00;
ADC_Init();
DD1 = mask(0);

zero2004
Posts: 32
Joined: 13 May 2010 13:42

Re: What's the wrong with this code ?

#3 Post by zero2004 » 27 Mar 2012 10:48

supra wrote:

Code: Select all

delay_ms(8);
try this:

Code: Select all

delay_ms(1000);
also move ADC_Init() between:

Code: Select all

TRISC  = 0x00;
ADC_Init();
DD1 = mask(0);

Dear supra

for the delay_ms(1000) this is too much , because we want to multiplex the three digits to show the result, so this time must be small as possible.

The problem is , there is a small flickers in 7 segment display, how we can remove it.

thanks again

supra
Posts: 187
Joined: 08 Apr 2009 11:54
Location: Ontario, Canada

Re: What's the wrong with this code ?

#4 Post by supra » 27 Mar 2012 12:10

I only worked with small DC voltage but not AC voltmeter

jtemples
Posts: 258
Joined: 22 Jan 2012 05:46

Re: What's the wrong with this code ?

#5 Post by jtemples » 27 Mar 2012 12:18

You should try to put the display function into a timer isr.

The divisions / multiplications also take lots of time. I would reorganize them like this:

Code: Select all

ADC_Value = adc_read(0);
  DisplayVolt = ADC_Value * 352; //think about ways to speed this up. * 352 = *256 + *96, both can be done by left shifts. and in the case of *256, you can perform it via union or pointers
  DisplayVolt = (DisplayVolt / 1000); DD3 = mask (DisplayVolt %10);
  DisplayVolt = (DisplayVolt / 10  ); DD2 = mask (DisplayVolt %10);
  DisplayVolt = (DisplayVolt / 10); DD1 = mask (DisplayVolt %10);
  //you can also think of faster ways to perform /1000 and /10, using a combination of left shifts + right shifts
   Display_Data();
I would also perform a boundary check in mask.

shreevs
Posts: 38
Joined: 16 Jun 2011 16:04

Re: What's the wrong with this code ?

#6 Post by shreevs » 27 Mar 2012 15:29

Dear Zero,
In order to multiplex a display which is flicker free, the frequency (total time) must be more then 100Hz, but less than 200Hz generally. I had done a 3 X 7 Seg display based voltmeter and encountered the same problem. I got a flicker free display by making the interval of 5mSec each (which in your code is 8mSec). If you are getting the reaadings right then I would suggest to try and tweak with that delay which is been 8mSec in your code. I got my thing working with 5msec interval, although I derived the time base by using timer 0 instead of using Delay_ms() routine. But still I think that 5mSec would work fine. All the best.
Regards
Shree

zero2004
Posts: 32
Joined: 13 May 2010 13:42

Re: What's the wrong with this code ?

#7 Post by zero2004 » 27 Mar 2012 18:09

jtemples wrote:You should try to put the display function into a timer isr.

The divisions / multiplications also take lots of time. I would reorganize them like this:

Code: Select all

ADC_Value = adc_read(0);
  DisplayVolt = ADC_Value * 352; //think about ways to speed this up. * 352 = *256 + *96, both can be done by left shifts. and in the case of *256, you can perform it via union or pointers
  DisplayVolt = (DisplayVolt / 1000); DD3 = mask (DisplayVolt %10);
  DisplayVolt = (DisplayVolt / 10  ); DD2 = mask (DisplayVolt %10);
  DisplayVolt = (DisplayVolt / 10); DD1 = mask (DisplayVolt %10);
  //you can also think of faster ways to perform /1000 and /10, using a combination of left shifts + right shifts
   Display_Data();
I would also perform a boundary check in mask.

Dear jtemples
The divisions / multiplications take lots of time In mikroc compiler only or this is a general case in other compiler ?

thanks

supra
Posts: 187
Joined: 08 Apr 2009 11:54
Location: Ontario, Canada

Re: What's the wrong with this code ?

#8 Post by supra » 27 Mar 2012 19:11

@zero2004
I'm sorry that i don't do 7-segments, b/c i'm moving into lcd/glcd etc.

I noticed that u r using 3 digits 7-segment, either common cathode or common anode 7-segments.
I noticed that u r using common anode 7-segments. That fine with u.
U need to declared array for portc......

Code: Select all

unsigned short digt_array[3];
Here is link:
http://www.mikroe.com/forum/viewtopic.php?t=21774
Look in void main procedure event.......do/while block statement

HTH.

regards

Mince-n-Tatties
Posts: 2780
Joined: 25 Dec 2008 15:22
Location: Scotland

Re: What's the wrong with this code ?

#9 Post by Mince-n-Tatties » 27 Mar 2012 23:22

the first change to make is to the ADC capture method.

use ADC_INIT(); to setup the ADC
and then call the ADC by using the function ADC_get_sample(0);

replacing adc_read(0); with ADC_Get_Sample(0); will speed up the adc routine.

read the help file for full details.

The next thing to do is to look into using interrupt to run the 7seg refresh rate. i have built the attached code to show how to update the 7seg using an ISR. The code is documented so reading through it should be enough to grasp the concept. This code should run on Easypic 4, 5, 7

you will note that i have fixed the ADC value as if it had been 10, this would result in the 7seg showing 10*352 = [MSB]3520[LSB]

Code: Select all

//******************************************************************************
//
// PIC16F887
//
// 8MHz
//
// Timer0 used to generate 1ms Interrupt
// prescaler required is 16
// Timer0 start value is 131 or 0x83 hex
//
// 7SEG will be updated ever 20ms based on - while (_7SEG_Update >= 20)
//
// If you want to see the update rate slowly then make this a larger number i.e 50 would = 50ms

long ADC_Value;

unsigned short digit_control = 0;     // 7SEG Lower / Upper selection control
unsigned short _7SEG_Update  = 0;     // 7SEG update rate control
unsigned short Digit1 = 0, Digit2 = 0, Digit3 = 0, Digit4 = 0; // Holders for 7SEG data values

const unsigned char MASK[] = {0x3F,0x06,0x5B,0x4F,0x66,0x6D,0x7D,0x07,0x7F,0x6F};

void Interrupt() {

   if(INTCON.T0IF == 1) // check that timer0 overflow is reason for ISR.
                        // even though there is only one Interrupt source
                        // trigger enabled, it is good programming practice to
                        // test why we have arrived at the ISR.
       {
        TMR0 = 0x83;      // Reset Timer to Start Value
        _7SEG_Update++;  // add 1 to update rate control
        INTCON.T0IF = 0;  // Clear timer0 overflow flag
       } // exit if
}// exit ISR

void _Update_7SEG_Write (void)
     {
     //PORTA = 0; // turn off both 7SEG

      //delay_us(400);

       switch (digit_control)
           {
           case 1:   PORTD=MASK[Digit1];    // Data value for first 7SEG
                     PORTA = 0;            // Turn OFF all 7SEG
                     Delay_us(50);         // data setup Delay
                     PORTA.B0 = 1;         // turn on first 7SEG
                     break;                // Exit Case
           case 2:   PORTD=MASK[Digit2];    // Data value for second 7SEG
                     PORTA = 0;            // Turn OFF all 7SEG
                     Delay_us(50);         // data setup Delay
                     PORTA.B1 = 1;         // turn on second 7SEG
                     break;                // Exit Case
           case 3:   PORTD=MASK[Digit3];    // Data value for third 7SEG
                     PORTA = 0;            // Turn OFF all 7SEG
                     Delay_us(50);         // data setup Delay
                     PORTA.B2 = 1;         // turn on third 7SEG
                     break;                // Exit Case
           case 4:   PORTD=MASK[Digit4];    // Data value for fourth 7SEG
                     PORTA = 0;            // Turn OFF all 7SEG
                     Delay_us(50);         // data setup Delay
                     PORTA.B3 = 1;         // turn on fourth 7SEG
                     break;                // Exit Case
           }// exit switch
    }// exit Function



void main()
{
    CM1CON0 = 0x00;       // Comparator 1 set off
    CM2CON0 = 0x00;       // Comparator 2 set off
    CM2CON1 = 0x02;       // Comparator 2 Timer1 Control register default set
    ADCON0 = 0x00;        // ADC off
    ADCON1 = 0x80;        // ADC right justified

    ANSEL  = 0;         // ALL set as Digital I/O
    ANSELH = 0;         // ALL set as Digital I/O

    //ADC_Init();  // Initialize ADC module with default settings


    PORTA = 0x00;       // set PORTA all logic zero
    PORTB = 0x00;       // set PORTB all logic zero
    PORTC = 0x00;       // set PORTC all logic zero
    PORTD = 0x00;       // set PORTD all logic zero
    PORTE = 0x00;       // set PORTE all logic zero
    TRISA = 0x00;       // set PORTA all bits as output
    TRISB = 0x00;       // set PORTB as all output
    TRISC = 0x00;       // set PORTC as all output
    TRISD = 0x00;       // set PORTD as all output
    TRISE = 0x04;       // set PORTE as lower 3 bits output, MCLR is external

    OPTION_REG.T0CS     = 0;    // Bit5 Timer0 clock source select bit [0 = FOSC/4 1 = T0CKI]
    OPTION_REG.T0SE     = 0;    // Bit4 Timer0 source edge select bit [0 = Low to High 1 = High to Low]
    OPTION_REG.PSA      = 0;    // Bit3 Prescalar Assignment bit [0 = Timer0 1 = WDT]
    OPTION_REG.PS2      = 0;    // Bit2 Prescalar Rate Select set to 1:16
    OPTION_REG.PS1      = 1;    // Bit1 Prescalar Rate Select
    OPTION_REG.PS0      = 1;    // Bit0 Prescalar Rate Select

    INTCON = 0;          // Clear INTCON

    TMR0 = 0x83;         // Timer0 start value
    INTCON.T0IF = 0;     // Clear Timer0 Overflow interrupt Flag
    INTCON.T0IE = 1;     // enable timer0 Interrupt
    INTCON.GIE  = 1;     // enable Global interrupts

    do
     {

          //ADC_Value = ADC_Get_Sample(0);    // read analog value from ADC module channel 0
                                              // ch0 is your choice of Analog input and will
                                              // be ok for you as your digit control is on PORTB
          ADC_Value = 10;
          ADC_Value = ADC_Value * 352;
          Digit4 = ADC_Value / 1000u ;        // extract thousands digit
          Digit3 =  (ADC_Value / 100u) % 10u; // extract hundreds digit
          Digit2 = (ADC_Value / 10u) % 10u;   // extract tens digit
          Digit1 = ADC_Value % 10u;           // extract ones digit



       if (_7SEG_Update >= 20)  // Run 7SEG update every 15ms, reduce number for fast update rate
              {
               _7SEG_Update = 0;         // Clear the update rate control
               digit_control++;          // controls which 7SEG will be updated

               if (digit_control > 4)   // only using 3 digits from the 4 available
                   {                     // makes sure we are using 1, 2, 3
                    digit_control = 1;   // this is used to drive the _7SEG_Write
                    }                    // case statement

               _Update_7SEG_Write();     // call the 7SEG update routine

               } // 7SEG update loop

     }while(1);
}
4_digit_7seg.jpg
4_digit_7seg.jpg (145.26 KiB) Viewed 4596 times
obviously ypu will need to alter the code for your pic and take from it what you wish.

I am a little concerned about safety... what ac voltage level are you applying to your pic ADC
Best Regards

Mince

jtemples
Posts: 258
Joined: 22 Jan 2012 05:46

Re: What's the wrong with this code ?

#10 Post by jtemples » 28 Mar 2012 00:58

some suggestions:

Code: Select all

// 8MHz
//
// Timer0 used to generate 1ms Interrupt
// prescaler required is 16
// Timer0 start value is 131 or 0x83 hex
You may want to think about defining it as a macro so you can easily port the code to a different chip with a different cpu frequency

Code: Select all

unsigned short digit_control = 0;     // 7SEG Lower / Upper selection control
unsigned short _7SEG_Update  = 0;     // 7SEG update rate control
unsigned short Digit1 = 0, Digit2 = 0, Digit3 = 0, Digit4 = 0; // Holders for 7SEG data values
Use "unsigned char" instead.

Code: Select all

void _Update_7SEG_Write (void)
Consider using a "static unsigned char" for digit_control:

Code: Select all

void _Update_7SEG_Write (void)
     {
     static unsigned char digit_control=1;  //digit control
     //PORTA = 0; // turn off both 7SEG

      //delay_us(400);

       switch (digit_control)
           {
           case 1:   
                     PORTA.B3=0;  //turn off the last display
                     //PORTA = 0;            // Turn OFF all 7SEG - 
                     PORTD=MASK[Digit1];    // Data value for first 7SEG
                     //Delay_us(50);         // data setup Delay - no reason to have those delays
                     PORTA.B0 = 1;         // turn on first 7SEG
                     digit_control=2;  //next digit
                     break;                // Exit Case
           case 2:   
                     PORTA.B0=0;  //turn off the last display
                     //PORTA = 0;            // Turn OFF all 7SEG
                     PORTD=MASK[Digit2];    // Data value for second 7SEG
                     //Delay_us(50);         // data setup Delay
                     PORTA.B1 = 1;         // turn on second 7SEG
                     digit_control=3;     //next digit
                     break;                // Exit Case
           case 3:   
                     PORTA.B1=0;  //turn off the last display
                     //PORTA = 0;            // Turn OFF all 7SEG
                     PORTD=MASK[Digit3];    // Data value for third 7SEG
                     //Delay_us(50);         // data setup Delay
                     PORTA.B2 = 1;         // turn on third 7SEG
                     digit_control=4;  //next digit
                     break;                // Exit Case
           case 4:   
                     PORTA.B2=0;  //turn off the last display
                     //PORTA = 0;            // Turn OFF all 7SEG
                     PORTD=MASK[Digit4];    // Data value for fourth 7SEG
                     //Delay_us(50);         // data setup Delay
                     PORTA.B3 = 1;         // turn on fourth 7SEG
                     digit_control=1;  //next digit
                     break;                // Exit Case
           }// exit switch
    }// exit Function
You need to make corresponding changes to the main():

Code: Select all

       if (_7SEG_Update >= 20)  // Run 7SEG update every 15ms, reduce number for fast update rate
              {
               _7SEG_Update = 0;         // Clear the update rate control
/*            //comment out the following lines
               digit_control++;          // controls which 7SEG will be updated

               if (digit_control > 4)   // only using 3 digits from the 4 available
                   {                     // makes sure we are using 1, 2, 3
                    digit_control = 1;   // this is used to drive the _7SEG_Write
                    }                    // case statement
*/

               _Update_7SEG_Write();     // call the 7SEG update routine

               } // 7SEG update loop

     }while(1);
}

Code: Select all

       if (_7SEG_Update >= 20)  // Run 7SEG update every 15ms, reduce number for fast update rate
In general, you want to refresh the display 50hz or faster. That means displaying all four digits every 20ms, or each digit every 5ms. 15ms is a little bit too slow.

zero2004
Posts: 32
Joined: 13 May 2010 13:42

Re: What's the wrong with this code ?

#11 Post by zero2004 » 28 Mar 2012 07:23

Hi every one

first of all thanks for your assistance

This ac voltage meter range from 0 to 300 V
I have use your suggestions , but the same problem still exist and the flickers in 7 segment display due to the multiplication & division process, any suggestions to avoid the multiplication & division ?

hasanen
Posts: 6
Joined: 17 May 2010 20:58

Re: What's the wrong with this code ?

#12 Post by hasanen » 13 Jul 2012 11:34

Hi

If this voltmeter is for reading AC values , then the main problem is in the analog read method , your code:
" ADC_Value = adc_read(0); "
This code for DC value .
Another thing , you must test , is the display code is OK or not , i suggest to put the code below instead of the code above:

ADC_Value = 123;
//ADC_Value = ADC_Value * 352;

And see , if the number 123 is displayed OK then you have not wrong in the display code and so.


Post Reply

Return to “mikroC PRO for PIC General”