This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Problem With 7-segment display. I dont know whether it is a timing issue or its my mistake, please suggest.

Hello all,

I am working on a project in which i've to display messages which changes for every 3 seconds as a part of start up routine.

I am using 8, 7-segment displays which are multiplexed by using a BCD decimal decoder. The multiplexed segments are connected to pins P4.0 to P4.7 and the BCD input pins are connected to pins P3.0 to P3.4. 6644.GT_2500_LED_TEST.rar

I am refreshing the 7-segment displays every 1millisecond i.e., it takes 8miliseconds to complete all the 8 7-segment displays.

The problem i am facing is when i want to change the data which i've to sent to display for every 3 seconds by using a global variable called message of type unsigned char, the message value is not updating, i am unable to find what is reason.

Also i am attaching my source code. Please reply me with your valuable suggestions

 

Thank you,

Ch. Shravan Kumar

2475.GT_2500_LED_TEST.rar

  • Thre are some problems with your code.

    Some of them are conceptional. You call the update message without parameters but then rely on the message variable. It would be more logical to call the update function with the message value as parameter. Also, it makes no sense calling the update funciton if there is nothing to update.So call display_ssd where the message changes (inside the timer ISR) or count up message in the timer, but call display_ssd() when message changes, passing it the current value as parameter. but don't do it in an endless loop without need. It does not make a difference in teh resulting behavior, but it will do once you want to use low power modes (well, not too likely if you use an LED display :) ) It also makes the whole thing easier to analyze the code later, when the memory about what you've intended to do with it has started to blur.

    Another thing is effectivity. Once more it won't change behavior yet, but might have some influence if you start to have mroe complex and tiem critical code. In display_ssd, you use a chain of if/else if checks. This is what the switch statement was made for.  Instead of  if (message==1) else if (message=02) else if... you write switch(message){ case 1: ... break; case 2: ...}. It is smaller and faster because it tells the compiler what you want to do: to switch different cases of the same parameter..
    Then you use different assignments for the different messages. Which is unnecessary large. YOu can as well define different arrays ssd_message_1[]={0x35,...} , then you have a global pointer unsigned char * ssd. Inside display_ssd you simply assign ssd=ssd_message_1; which will be only one simple instruction instead of copying large lists of constants. Short and fast. If you arrange your messages in an array of arrays, you can even do the assignment without using an if/switch at all: ssd_message[][] = { {0x35...},{0x01...},...};. Then you can simply assign ssd=ssd_message[message]; and you're done. Then you can skip the fucntion call completely, as the last assignment does not need a separate function anymore (simply declare the structures as external).

    Anyway, this all does not change the behavio and is not the cause for your problem. It is something different: optimization. The compiler optimizes code. You only ever write message from withing the isr. Yet the ISR is never called from anywhere inside the code.(of course it is called, but by the hardware, which the compiler cannot know). so for the compiler, message is rather a constant that never changes. To tell teh compiler that message might be changed at any time without being touched by the currentprogram flow (ISRs are calle doutside the current program flow and undetectable by the compiler), you need to declare it volatile:  (extern) volatile unsigned char message;
    This tells the compiler tht messaeg needs to be read each time you use it in the code and not less or more, as it can change at any time 'withotu a cause' and reading/writing it may have side-effects not described in the code. This is how all the MSP hardware register definitions work too.

    One final thing: in your ISR, you write teh new multiplex value to P4OUT and then change P3OUT for swithcing the difit. This means that for a short period, the new digit is shown at the old location. It is probably invisible to the bare eye, but it would be cleaner to first clear the multiplex with P3OUT=0; then change the value and then activate the next digit. (if you later use an external 3-to-8 multiplexer to save 5 of the signal lines or add more digits, switch off the segments first, then change the multiplex and then switch the segments on)

     

  • Dear Mr. Michael Gross,

    Thank you for your reply with valuable suggestions. As a beginner for this embedded programming i have to learn a lot from the experienced persons like you.

    I've changed my code according to your suggestions, but  with this also my problem is not solved. The new problem i am facing is timer problem.

    I am using a timer B for updating the data on Port pins for 7-segment display and drivers. My timer B interrupt occurs for every 1millisecond. I am using a counter (one_sec_counter) to count it to 1sec by using the condition (one_sec_counter>1000) but this condition is not satisfying.

    I've trouble shooted it by changing the condtion  till (one_sec_counter>28) the condtion satisfies that too only once even if we put condition  (one_sec_counter>29) the condition is not satisfying. Please give your suggestions regarding this.

    I am attaching my updated code please help me to sort out this issue.

    Thank you,

    Shravan Kumar

    1374.GT_2500_LED_TEST(Updated).rar

     

  • Well, some  things I noticed:

    You use _BIS_SR(GIE) at the end of the ISR. This is counterproductive.

    Since GIE was set when entering the ISR (else it wouldn't have been called), it will be also set when you return. Setting GIE (Whcih is disabled during an ISR) is only necessary if you want to have other ISRs being executed while the ISR is running. There are very few cases where this is required. And never at the end of an ISR which is already exiting. If (for some reason) yo do not finish the ISR before the next identical interrupt occurs, it will cause teh ISR to be entered over and over, whiel it is perhaps never exited. And eventually the stack will overflow and the system chrashes.

    Then you use
    TBCTL = 0x01D0u;
    This isn't wrong. The 'u' suffix is not necessary as you already define a hexadezimal value, which isn't signed.
    Also, it is usually better to directly use the bit definitions:
    TBCTL = TASSEL_1 |  MC_1 | TACLR;
    It is better readable, less error-prone than a number and also portable.

    The comparison of one_sec_counter >1000 is wrong. It should be ==1000. Sonce you start over with 0. >1000 wll apply after 1001 ticks. Also, this code is guaranteed to be executed after every increment of one_sec_counter, so you won't miss the 1000.

    You can skip the initialisation of temp = 0, as the next time temp is accessed, it will be assigned a new value anyway. But normally the compiler code optimization should drop this assignment automatically.

    Instead of doing
        display++;
        if(display == 8)
            display = 0; 
      
    you could as well do
    (++display)&=0x07;
    which will do the very same but will be some ticks faster. Of course it only works because you are counting from 0 to 2n-1

    Unfortunately I don't see what's different for 28 and 1000. I'd rather expect that it won't work if you're using smaller update delays as this means more CPU load (more often) inside the ISR and possibly (well, not really in this case of slow 1ms interrupts) an ISR overflow. especially in conjunction with setting GIE inside the ISR.

    So what's what you're observing in the first and the second case?

     

  • Dear Mr. Micheal Gross,

    Thank you very much for your suggestions.

    i've re-write the code keeping your suggestions in my mind from starting and this time i am successful to get the perfect output.

    Thank you for your suggestions.

    I've small query regarding ADC:

    I've connected an analog input signal to ADC pin, the ADC register is giving the response with +/-1 of actual reading i.e., if i've to get ADC counts of 2093 i am getting it as 2093, also 2094 and 2095, by which I am getting error of 0.8microvolts with ADC reference of 3.3V.

    How can i stabilize this fluctuation. Please suggest me.

     

    Thank you,

    Regards,

    Shravan Kumar Ch

  • Shravan Kumar Chirlanchi said:
    I've connected an analog input signal to ADC pin, the ADC register is giving the response with +/-1 of actual reading i.e., if i've to get ADC counts of 2093 i am getting it as 2093, also 2094 and 2095, by which I am getting error of 0.8microvolts with ADC reference of 3.3V.

    A certain noise is normal. There are two ways to deal with it:

    First, you can use a faster ADC10CLK speed (if possible) to shorten the conversion time, but increase the S&H gating time. This acts as a low-pass for the input signal while the reduced conversion time reduces the voltage-decrease on teh hold capacitor during conversion. But +-1LSB is already quite good. So all you can do is oversampling. If you sample with four times the desired frequency, sum up the results and shift the result by 1, you'll get one additional bit of resolution. Or, if you shift by two, you'll get same resolution but your LSB becomes more stable. You can even do a moving average over the last conversion results, but this decreases the maximum signal frequency.

**Attention** This is a public forum