0
\$\begingroup\$

I want to display 4 different data on a multiplexed seven segment display .I am able to display 2 data for a certain interval ,now i want to expand this to 4.How to do this .

main(void) { 

    InitIO();
    InitTimer2();
    int i;

    while(1) {
        switch(i) {

            case 1:
                input();
                break;

            case 2:
                output();
                break;
        }

        if (0==my_timer) {
            i=(1==i)?2:1;
            my_timer = 30 * 50;  // MUX_FREQUENCY must be < 1092 Hz
        }
    }     
}

//full code

/* 
 * File:   main.c
 * Author: EmbeddedBuzz
 *
 * Created on December 4, 2014, 7:24 PM
 */
#include <htc.h>
__CONFIG(FOSC_XT & WDTE_OFF & PWRTE_OFF & CP_OFF & BOREN_ON);
#include <stdio.h>
#include <stdlib.h>
#define _XTAL_FREQ 4000000
#define K 0.01f
//Display Configuration
unsigned short shifter, portb_index;
unsigned int digit, number;
 unsigned short portb_array[4];
float i,adc,adc1;
float result,result1;
volatile int my_timer=0;

unsigned short mask(unsigned short num)

{
 switch (num)
 {
  case 0 : return 0x3F;
  case 1 : return 0x06;
  case 2 : return 0x5B;
  case 3 : return 0x4F;
  case 4 : return 0x66;
  case 5 : return 0x6D;
  case 6 : return 0x7D;
  case 7 : return 0x07;
  case 8 : return 0x7F;
  case 9 : return 0x6F;
     case 'I':return 0x06;
    }
}

void interrupt ISR()
{
 PORTC = 0;                           // Turn off all 7seg. displays;
 PORTB =portb_array[portb_index];    // Bring appropriate value to PORTB;
 PORTC = shifter<<4;                     // Turn on appropriate 7seg. display;

 //move shifter to next digit;
 shifter <<= 1;
 if(shifter > 8u)
 shifter = 1;

 //increment portb_index;
 portb_index ++ ;
 if (portb_index > 3u)
 portb_index = 0;                     //turn on 1st, turn off 2nd 7 seg.;
  if (my_timer > 0) my_timer--;
 TMR2 = 0;                            //reset TIMER0 value;
   TMR2IF = 0;
     TMR2IE = 1;       // Turn on TMR2 interrupt                      //clear T0IF, Bit T0IF=0, T0IE=1;
 }
void InitTimer2(void)
{
     T2CON = 0x3C;
/*   Prescale 1:16
 *   Timer off for now
 */
     PR2 = 249;
/*   With prescale 1:16 and at 4MHz clock,
 *   this gives 2ms period.
 */
     TMR2IF = 0;
     TMR2IE = 1;       // Turn on TMR2 interrupt
     GIE = 1;          // Turn on global interrupts
     PEIE = 1;         // Turn on peripheral interrupts
     TMR2ON = 1;       // Start TMR2
}
void InitIO(void)
{

 digit = 0;
 portb_index = 0;
 shifter = 1;
   PORTB = 0;
     TRISB = 0;
     PORTC = 0;
     TRISC = 0;
     ADCON1 =0x60;
     ADCON0 = 0xC1;
     PORTA = 0;
     TRISA = 0x00;          // Inputs for ADC

}
void UpdateDisplay(int display)
{
     number=display;
     digit = number % 10u;            //extract ones digit;
     portb_array[0] = mask(digit);    //and store it to PORTB array;
     digit = (number / 10u) % 10u;    //extract tens digit;
     portb_array[1] = mask(digit);    //and store it to PORTB array;
     digit = (number / 100u) % 10u;   //extract hundreds digit;
     portb_array[2] = mask(digit);    //and store it to PORTB array;
     digit = number / 1000u;          //extract thousands digit;
     portb_array[3] = mask(digit);    //and store it to PORTB array;

}
void DisplayINPUT()
{
     portb_array[0] = mask('I');    //and store it to PORTB array;
     //portb_array[1] = mask(I);    //and store it to PORTB array;
  //   portb_array[2] = mask(I);    //and store it to PORTB array;
    // portb_array[3] = mask('I');    //and store it to PORTB array;

}

float filter( float aData )
{
    static float memory;
    memory = memory*(1-K) + aData*K;
    return memory;
}
unsigned int ADC_Read(unsigned char channel)
{
  if(channel > 5) //If Invalid channel selected
    return 0;     //Return 0

  ADCON0 &= 0xC5; //Clearing the Channel Selection Bits
  ADCON0 |= channel<<3; //Setting the required Bits
  __delay_ms(2); //Acquisition time to charge hold capacitor
  GO_nDONE = 1; //Initializes A/D Conversion
  while(GO_nDONE); //Wait for A/D Conversion to complete
  return (ADRES); //Returns Result
}

void input()
{

         adc=0;
     for(i=0;i<20;i++)    //adc=adc+result*1.96078;// 1s delay;
     {
     adc=adc+ ADC_Read(0);
     }
             result=19.6078431372549*(adc/20);//Endless loop;
              UpdateDisplay( result);
              __delay_ms(1000);
}
void output()
{
         adc1=0;
     for(i=0;i<20;i++)    //adc=adc+result*1.96078;// 1s delay;
     {
     adc1=adc1+ ADC_Read(1);
     }
             result1=19.6078431372549*(adc1/20);//Endless loop;
              UpdateDisplay( result1);
              __delay_ms(1000);
}
main(void)
{
     InitIO();
     InitTimer2();
     int i=1;
     while(1)
     {
     switch(i)
     {
     case 1:
     input();
     break;
     case 2:
         DisplayINPUT();
     case 3:
     output();
    case 4:
  input();
     break;
     }


if (0==my_timer)
{
    i++;
    if (i>3) {
        i = 0;
    }
    my_timer = 30 * 50;  // MUX_FREQUENCY must be < 1092 Hz
}
}
}
\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

There are lots of bugs and potential bugs in your code. Please in future:

  1. NEVER EVER use global variables unless absolutely no other ways to store and communicate data exist. It's a real pain in the butt to debug corruptions of globally defined and globally accessed data. Confine the scope of variables to the code blocks where they are used. In your case portb_array, portb_index, and my_timer are fine to be global because they are used in out-of-order execution (interrupts), the rest aren't. Some variables are plain useless.

  2. NEVER EVER use magic numbers. 20 or 19.78452 are nice numbers, but will you be able to tell in a couple of years what was the intended meaning? What happens if you change it in one place and forget to change in the others? Defining macros or enums are the ultimate solutions.

  3. For the same reason as in item 2 above, at all costs avoid copy-pasting the code. Create generalized functions for the shared code. Make them 'inline' or even 'static inline' if you want to save on calls/returns. What's the use of separate input() and output() functions if they actually do the same thing, just using different ADCs? Besides, giving relevant names to functions also helps a lot in the long run. In this particular example, output() is a bad name because it doesn't actually output anything. These both functions perform an ADC read. If you intended to indicate which value (input or output) they read, you could better use an input parameter and define macros for it, or better yet an enum (for you can see enums in a debugger).

  4. When using local variables, don't forget to initialize them in the declaration. Otherwise you'll end up with garbage in them. They are allocated on stack and can contain any random value. Well, not so random for an embedded system, but still... Your filter() function is a nice example of such a bug. It's good that this function is totally unused.

  5. For looping through a limited set of numbers, use modulo operator, not if(). That is, instead of

    i++;
    if(i>3)
        i = 0;
    

    use this:

    #define NDIGITS 4
    ...
    i = (i + 1) % NDIGITS;
    

    as this is generally easier to understand and is much more compact.

  6. Use proper types for variables. Using float for a loop iterator is usually not the best idea, performance-wise.

  7. Don't forget to properly terminate your case blocks. A forgotten break; may give you some extra grey hair sometimes.

  8. Use arrays for mapping wherever possible. This is lots faster and way more compact than creating endless switch() blocks. Array-based mappings work best for contiguous input ranges (e.g., 0..9 or 25..178). In your case here they are ideal if you get rid of an absolutely useless mask('I') that can be replaced by mask(1), which you essentially do inside mask() itself.

  9. For goodness sake, USE PROPER INDENTATION. It's a pain reading and understanding a badly indented code. Better yet, adhere to "indent with tabs, align with spaces" method.

Additionally, I must note that I don't understand how the code you posted managed to display two values. That must have been a coincidence.

Having all that said, I believe the following code should do the trick for you. Just define all the channels you have at appropriate positions within the channel_t enum. You may also want to adjust the DISPLAY_DURATION macro. If I understood your hardware from your code properly, that must now be the delay between switching displayed values:

    /*
     * File:   main.c
     * Author: EmbeddedBuzz, Alexander Amelkin ([email protected])
     *
     * Created on December 4, 2014, 7:24 PM
     * Updated on July 22, 2015
     */
    #include <htc.h>
    __CONFIG(FOSC_XT & WDTE_OFF & PWRTE_OFF & CP_OFF & BOREN_ON);
    #include <stdio.h>
    #include <stdlib.h>
    #define _XTAL_FREQ 4000000
    #define K 0.01f

    #define MAX_DIGITS        4
    #define MS_IN_SEC         1000
    #define DISPLAY_DURATION  (1 * MS_IN_SEC) // Make sure this is longer than at least MAX_DIGITS ISRs
    #define ADC_SCALE         19.6078431372549
    #define ADC_READS         20

    typedef enum {
        CHANNEL_NONE = -1,
        CHANNEL_IN,        // 0
        CHANNEL_OUT,       // 1
        CHANNELS
    } channel_t;

    //Display Configuration
    unsigned short portb_index;
    unsigned short portb_array[MAX_DIGITS];
    volatile int digits_tdigits_to_show;

    unsigned short mask(unsigned short num)
    {
        unsigned short res[10] = {
            0x3F, 0x06, 0x5B, 0x4F, 0x66, // 0..4
            0x6D, 0x7D, 0x07, 0x7F, 0x6F  // 5..9
        }

        return res[num % 10];
    }

    void interrupt ISR()
    {
        PORTC = 0;                          // Turn off all 7seg. displays;
        PORTB = portb_array[portb_index];   // Bring appropriate value to PORTB;
        PORTC = 1 << (portb_index + 4);     // Turn on appropriate 7seg. display;
        // FIXME: What is this 4 above? Define a macro.

        //increment portb_index;
        portb_index = (portb_index + 1) % MAX_DIGITS;

        if (digits_to_show > 0)
            digits_to_show--;

        TMR2 = 0;                           // reset TIMER0 value;
        TMR2IF = 0;
        TMR2IE = 1;                         // Turn on TMR2 interrupt
        //clear T0IF, Bit T0IF=0, T0IE=1;
    }

    void InitTimer2(void)
    {
        T2CON = 0x3C;
        /*   Prescale 1:16
         *   Timer off for now
         */
        PR2 = 249;
        /*   With prescale 1:16 and at 4MHz clock,
         *   this gives 2ms period.
         */
        TMR2IF = 0;
        TMR2IE = 1;       // Turn on TMR2 interrupt
        GIE = 1;          // Turn on global interrupts
        PEIE = 1;         // Turn on peripheral interrupts
        TMR2ON = 1;       // Start TMR2
    }

    void InitIO(void)
    {
        portb_index = 0;
        PORTB = 0;
        TRISB = 0;
        PORTC = 0;
        TRISC = 0;
        ADCON1 =0x60;
        ADCON0 = 0xC1;
        PORTA = 0;
        TRISA = 0x00;          // Inputs for ADC
    }

    void UpdateDisplay(int number)
    {
        int i, divisor, digit;

        for(i = 0, divisor = 10; i < 3; i++, divisor *= 10) {
            digit = number % divisor;
            portb_array[i] = mask(digit);
        }
    }

    unsigned int ADC_Read(channel_t channel)
    {
        if(channel >= CHANNELS) //If Invalid channel selected
            return 0;     //Return 0

        ADCON0 &= 0xC5; //Clearing the Channel Selection Bits
        ADCON0 |= channel<<3; //Setting the required Bits
        __delay_ms(2); //Acquisition time to charge hold capacitor
        GO_nDONE = 1; //Initializes A/D Conversion
        while(GO_nDONE); //Wait for A/D Conversion to complete
        return (ADRES); //Returns Result
    }

    void read_channel(channel_t which)
    {
        int i;
        float adc;
        float result;

        for(adc = 0.0, i = 0; i < ADC_READS; i++)
        {
            adc = adc + ADC_Read(which);
        }
        //adc=adc+result*1.96078;
        result = ADC_SCALE * (adc / ADC_READS);
        UpdateDisplay(result);
    }

    main(void)
    {
        InitIO();
        InitTimer2();
        channel_t channel = CHANNEL_NONE;

        while(1) {
            if (!digits_to_show) {
                channel = (channel + 1) % CHANNELS;
                read_channel(channel);
                digits_to_show = MAX_DIGITS;
                __delay_ms(DISPLAY_DELAY);
            }
        }
    }
\$\endgroup\$
1
  • \$\begingroup\$ thanks a lot for detailed review of mistakes on my code.Time to implement them :-) \$\endgroup\$ Commented Aug 12, 2015 at 19:51
2
\$\begingroup\$

At the moment you are toggling an integer i between 1 and 2:

i=(1==i)?2:1;

All you need to do instead is count up from 1 to 4. Or, better still, since numbers start at 0, not 1, count from 0 to 3.

When you reach 3 you start from 0 again.

In long hand:

if (0==my_timer) {
    i++;
    if (i>3) {
        i = 0;
    }
    my_timer = 30 * 50;  // MUX_FREQUENCY must be < 1092 Hz
}

Or in short hand, which you seem to prefer looking at your code:

if (0==my_timer) {
    i = (i+1) % 4;
    my_timer = 30 * 50;  // MUX_FREQUENCY must be < 1092 Hz
}

Then you have 4 switch case entries, for 0, 1, 2 and 3.

Oh, and one more thing. If you were to turn on warnings you would see something along the lines of:

Warning: variable 'i' may be used uninitialized in this function.

You're creating the variable i but not giving it an initial value. It may start at 0, or it may start at 4892, or 11944, or who knows what. Chances are 0, since it's in main, and nothing else should have used the stack by that point, but who can tell? You should always initialize a variable with a value before modifying that value. In this case:

int i = 0;

would be a good idea. For your original code, initializing it to 1 would be better since that's a valid entry in your switch.

\$\endgroup\$
4
  • \$\begingroup\$ It's not working ,kindly look into detail code.. \$\endgroup\$ Commented Dec 6, 2014 at 15:31
  • \$\begingroup\$ Maybe if you implemented it like I said, it might work. \$\endgroup\$
    – Majenko
    Commented Dec 6, 2014 at 15:32
  • \$\begingroup\$ Implemented exactly like you said,still not. \$\endgroup\$ Commented Dec 6, 2014 at 15:34
  • \$\begingroup\$ I will highlight one phrase from my answer: Then you have 4 switch case entries, for 0, 1, 2 and 3. \$\endgroup\$
    – Majenko
    Commented Dec 6, 2014 at 15:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.