This cleans a few things up and adds automatic flashing of selected LEDs done by the 'multiplexer'. Tested on 16F884.

Check out the interrupt structure.
Have fun

BTW: The errata for the 887, silicon rev A2 says that you have to pull down RB3 for reliable low-voltage programming. Found that out trying to the the 884's to program. Grrr...

 

Consider adding a line to the header in the source file stating which C compiler is being used, please? There are just so many PIC C compilers it's hard to remember which one is being used, even though it was mentioned earlier in the thread.

Thanks, guys. Nice work.

Cheerful regards, Mike

 

I think there's a bug in your interrupt too;

if (CCP1IE_bit)                 // System Tik

should that be;
if (CCP1IE_bit) {               // System Tik

I prefer to make braces {} line up vertically, it is much better for visually checking the code;

That was something I noticed before, but I thought it will work that way.

There are no braces in what John showed previously and I did not bothered to ask cause I guess you guys knows better and that it is working that way.

I learned that an "if" statement should be opened and closed with braces. Am I right ?

By the way. LVP is disabled.

U know John, every time I read "dumb delays" from ur posts, I cannot help but giggle. 
Why do you hate delays so much?

 

Hey John, ur signature is funny.
And it somehow reflects to me.
Lemme explain

"Beating the stupid out of sand since 1974"

Beating the stupid - Stupid ol' me
out of sand - Maldives is nothing but sand
since 1974 - I was born in 1974

What are the odds ?

{ED}
So I checked the file in post#61
Below is the interrupt()

/*********** Interrupt Service Routine ***************/
void interrupt(){
//Used for Display Muxing.
  if (TMR0IE_bit)
    if (TMR0IF_bit){
      TMR0IF_bit = 0;           //Ack IRQ
      TMR0 = TMR0set;           //Reset Timer0
      //Display Multiplex Code goes here
      if(LEDflash_timer==0){    // if LED flash timer ran out
       LEDflash_timer = LED_MUXflash_set;  // reset timer and.. invert LEDs under mask
       DigitsBuf[dLEDs] ^= LEDflash_mask;
      }
      else
        LEDflash_timer--;
        
      PORTD = DigitsBuf[dLEDs];     // 'mulitiplex' the LED display - only discrete LEDs so far
                                    // This is where the 9 digits get selected/decoded
    }
  if (CCP1IE_bit)                //System Tik (dont specify Time here, it may change then this will be confusing)
    if (CCP1IF_bit){            // Interrupt on Timer1 = CCP
      CCP1IF_bit = 0;           //Ack IRQ
                                // Decrement derived timers
      if (SYStik_timerA) SYStik_timerA--;
      
      SecsTimerPS--;       // dec Seconds timer prescaler
      if(SecsTimerPS == 0){  // iff one second passed..
        SecsTimerPS = SecsTimerPSset;  // reload the prescaler
        SecElapsed = 1;                // signal main program
       }
         // more code goes here
    }
}

See the RED high lights. There are no braces {} as before.
Would it work.
Just need to make this clear. A little explanation is needed.

 

Consider adding a line to the header in the source file stating which C compiler is being used, please? There are just so many PIC C compilers it's hard to remember which one is being used, even though it was mentioned earlier in the thread.

Excellent point. Done!

There are no braces in what John showed previously and I did not bothered to ask cause I guess you guys knows better and that it is working that way.
I learned that an "if" statement should be opened and closed with braces. Am I right ?

Sort of. If the IF condition is true, the code will execute the next statement as in one statement. If it happens that you only need one statement to do the job, you don't need braces. If you need more than one, you use braces to enclose multiple statements to make one compound statement. The compiler treats the compound statement in the same way as if it were a single one.

For your question, what follows

if(CCPIE_bit)

is really the one statement that 'if' will execute on a true condition i.e.

if(CCPIF_bit) // this will execute the one statement following on a true condition also

Since what we need to do requires more than one statement, we make one compound statement with braces.

It wouldn't hurt to add braces following the first test but I find it easier to read without it. Seeing braces means to expect a group of statements (to me at least) and no braces means to expect a simple action. When you've read/written lots of code, such minor simplicities are appreciated and reduce errors IMHO.

if (SysTIK_timerA) SysTIK_timerA--;  // is the same as

if(SysTIK_timerA > 0){ 
  SysTIK_timerA--;
}

I find the first one easier to read. I should note that there are various thoughts on how C source should be formatted, whether to put the opening brace at the end of the conditional statement or below it to box in the following statements visually. I prefer the former. The important thing is to be consistent throughout your code. That way, its easier to read - and that makes it easier to find errors.

Beating the stupid - Stupid ol' me
out of sand - Maldives is nothing but sand
since 1974 - I was born in 1974

What are the odds ?

Finite and non-zero I would think. I probably should change it. Its a sideways reference to the fact that the silicon (sand) won't do anything until you program it (its stupid). Sometimes, this task requires much effort (beating on it). 1974 is when I (badly) wrote my first microcontroller code (i4004) - and to acknowledge that I'm an old coot who may get a bit testy at times. But you are the second person to take 'stupid' a little personally. Its not my intent, even in jest. Indeed, I marvel at the wealth of knowledge present at AAC and more so at how generous most are in sharing it.

So... does it work on EasyPIC?

 

Hey guys... In that source file, shouldn't digit "1" use LED segments 'B' and 'C' instead of segments 'A' and 'B'? Here's how the array looks in one of my BoostC programs;

   #define r08 const rom unsigned char

   r08 segment[] = { 0b00111111,    // "0"   -|-|F|E|D|C|B|A
                     0b00000110,    // "1"   -|-|-|-|-|C|B|-
                     0b01011011,    // "2"   -|G|-|E|D|-|B|A
                     0b01001111,    // "3"   -|G|-|-|D|C|B|A
                     0b01100110,    // "4"   -|G|F|-|-|C|B|-
                     0b01101101,    // "5"   -|G|F|-|D|C|-|A
                     0b01111101,    // "6"   -|G|F|E|D|C|-|A
                     0b00000111,    // "7"   -|-|-|-|-|C|B|A
                     0b01111111,    // "8"   -|G|F|E|D|C|B|A
                     0b01101111,    // "9"   -|G|F|-|D|C|B|A
                     0b00000000,    // " "   -|-|-|-|-|-|-|-
                     0b01000000 };  // "-"   -|G|-|-|-|-|-|-

 

Hey guys... In that source file, shouldn't digit "1" be LED segments 'B' and 'C' instead of segments 'A' and 'B'?

Yes! Fixed. And that's one reason I defined the digits as Segx + Seg.. etc. Much easier to find bugs by inspection. You can also move the segments around on the port without re-figuring all the binary. Thanks Mike!

And for the OP, I do avoid dumb delays like the plague. One reason is that they can be code-intensive -on XC8 at least, every different delay time gets its own routine. But the main reason is that they consume 100% of the CPU time while delaying - less interrupts, which throw off the timed delay, a bad thing. While there are techniques to mitigate the problem, they usually fall into some ad-hoc scheme that requires breaking them up to smaller delays and calling that small delay multiple times with other processing in between - but that requires more tuning and as the program progresses and gets revised, becomes a first-class beating. When its done, the solution is so project-specific that it can't be reused on the next project. There is a limit to what you can invest into a project whether its time available, your own schedule or what a client can/wants to pay. If you burn up all of your bandwidth on tuning dumb delays, you don't get to use that on making the project better (or getting home for dinner for a change). When I did this for a living, I frequently consulted on code problems or took over projects that someone couldn't get work satisfactorily. In many cases, the over-use of dumb delays caused problems.. In one case, I showed a client's programmer how to do pretty much what we are doing here. He didn't like the idea and proceeded to use dumb delays everywhere. It was a reasonably complex program and it eventually got to the point where each keypress on the pad took 2-5 seconds to process. I could relate many more examples but you get the idea. Even if you get eventually get all the delays tuned, you are one meeting with marketing away from having to change something and do all the clever tuning again.

That's why I don't like dumb delays.

 

Don't feel bad. I was just being funny about the stupid part. I did not take it personally.

I am at home now. Checking the code on easyPIC.
Will update soon.

 

Man you are good with this timer thing.

I ported ur code to F887. The code in post #61

RD0 or 2 flashes fast, around 100ms I think. And RD1 flashes at 1 second.

 

Man you are good with this timer thing.

And now you are too!

I think a good way to proceed from here is:
Archive the code so that you can come back to it to buzz out hardware problems or to confirm firmware foul ups

Next, fiddle around with it, change some LEDs, timings etc. and get comfortable with it. Be sure you are on board with the routines to turn them on/off/flash instead of directly accessing the segment array. Eliminate any routines you don't think you'll need and write any enhancements that you like.

Then, rough out the actual digit multiplexer. You'll need to maintain a digit number 0-9 to index the array of stored segment patterns and a way to associate the digit number with a single bit to select the digit. The whole thing will run under TMR0's IRQ in place of our simple PORTD=.. Its a good next step since it will tell whether the drive available on the PIC outputs is enough to light the segments/LEDs when divided over 10 digits. You may decide you'll need a segment driver to increase the current or more efficient displays.

I've also been pondering on some of the delay stuff, thinking back on the beeper code that fouled the LED flasher. I moved the flasher into the multiplexer so the main code would not have to maintain the LED flashing (it was more of a demo anyway) but also think it would be reasonable to move all interrupt-derived timing into the interrupt code i.e. increment the System Seconds timer there rather than in main. If all of the time-sensitive stuff could run under IRQ, then you likely could implement most of the main program using some short, non-critical delays. If so, then you could make use of MicroC's extensive library of cool stuff without having to worry about fouling the basic timing etc of the system. You still might not like how a 5 second beep drags down your system but at least the LEDs would keep flashing nicely But we still have the PWM for the beeper....

Sounds like you are on your way!

 

I was also thinking of putting together the code studying the C files.
I kept looking at the ISR and defines and changing the decimal values and seeing that I can flash slower and faster both RD1 and 2.

I will have a go at it and see whether I can really do this.
Just thought this would be as easy as driving LCD's as in my VA meter project.

The Beeper code is not much actually. I do not want beeping all through the code. Just start beeper at power on and some beeping at the charge complete. I was thinking about what you said about PWM beeping. I think this is too much for me now.
I need to get the hang of this ISR Muxing part first

We will think about the beeping after finishing all that I need this project to do.

Thanks JohnInTX. You have been a real Gentlemen in assisting me to get this started. I learned a lot from ur part.

Please keep an eye on this thread. It might take me some time to study the whole thing.

Regards.

PS .. was waiting for u to reply. It's 3.30 AM here.
G'Night.

 

As long as you are looking.. look at this. I did a quickie multiplexer and cleaned up some things.

Thanks for the kind words and be sure to indicate if you are losing sleep waiting for a reply. Sometimes if it will take longer than I have, I'll just wait until later. If I know you are waiting 1) I'm flattered and 2) I can tell you to go to bed.

There are some issues with how I did the mux - not the least of them is that I did not test it.. There are also reasons why I did it that way for starters. What do you think?

It may be bedtime for you but its cocktail time for me, friend so..

Have fun!

 

As long as you are looking.. look at this. I did a quickie multiplexer and cleaned up some things.

Thanks for the kind words and be sure to indicate if you are losing sleep waiting for a reply. Sometimes if it will take longer than I have, I'll just wait until later. If I know you are waiting 1) I'm flattered and 2) I can tell you to go to bed.

There are some issues with how I did the mux - not the least of them is that I did not test it.. There are also reasons why I did it that way for starters. What do you think?

It may be bedtime for you but its cocktail time for me, friend so..

Have fun!

Before anything I ported the code to F887

Connected my test gig( 9 digit 7Seg proto to the PORTS U have stated in the file since the beginning and guess what.

The digit 9 is OFF
Digit 0 is going haywire.

So I checked the file, I had no idea what I was looking for but found an issue.

     switch (digitN){            // turn on new digit (also brute force)
       case 0: dig0sel = 1;
               break;
       case 1: dig1sel = 1;
               break;
       case 2: dig2sel = 1;
               break;
       case 3: dig3sel = 1;
               break;
       case 4: dig4sel = 1;
               break;
       case 5: dig5sel = 1;
               break;
       case 6: dig6sel = 1;
               break;
       case 7: dig7sel = 1;
               break;
       case 8: dig0sel = 1;
               break;
       case 9: dig0sel = 1;
               break;
     }//switch

Changed it to

       case 7: dig7sel = 1;
               break;
       case 8: dig8sel = 1;
               break;
   //    case 9: dig9sel = 1;
   //            break;
     }//switch

No LED digits defined that is dig9sel. So commented it out

and Viola.
Digit 8 came to life and digit 0 is A-OK

The Volt and Amp values are loaded to what ever I changed them in main.
Time ( digit 0 to 2 ) starts from 0 and is incrementing from 000 nd onwards each second perfectly.

Yet I see flicker, did not liked it. So further study.

// Setting is 256 - (3ms / .5usTcyc) / 64Prescaler = 256 - 93.755 counts = 162.25 ~= 162
#define TMR0set 162             // counts 162->255 then rolls over and interrupts

Fiddled with the TMR0 value. 162 is too slow. Messed with it and 200 seems to be flicker free what so ever.

What do you think ?

But still no LED control or flashing. So I think digit 9 is yet to figure out

 

Had a LED unit connection error.
Was using the RE2 as LED control instead or RE1

changed the following area

       case 8: dig8sel = 1;
               break;
       case 9: digLEDsel = 1;
               break;

And now RD0 and RD2 Leds are flashing at 100ms
Cooool !

 

I fiddled with the code and learned how to add more leds and toggle them.
I can now control all the five LED's...Nice.

One thing I forgot to mention is about time display.
and the other two V and I segments.
The dig 3 & dig 6 needs to display the DP at all times. So that it will display 0.00V and 0.00A

The timer is two hours preset-able by the encoder. 0 to 2 hrs. It needs to have a register to save the input time and count down from there when needed ( say after reading a flag bit which sets when the current is 0.1C or something )

And the other thing the DP on the dig 0 need to blink as like a clock dot indicating the timer is running. It should start to blink when the clock starts to count down.

Any pointers on how to do this.

A thought..
What if I connect the DP's on dig 3 & dig 6 permanently to 5V through a resistor.
This way the DP will light every time that Digit is active, right ?

 

A thought..
What if I connect the DP's on dig 3 & dig 6 permanently to 5V through a resistor.
This way the DP will light every time that Digit is active, right ?

Horrors! A hardware solution to a firmware problem? Actually, consider modifying Int2Segs to look at the passed offset (which determines whether the number goes into volts, amps or time. If the offset corresponds to volts or amps, 'OR' SegDP with SegTable[ofs].

EDIT: maybe something like this:

//Conversion routine
// Appends decimal point to Volts and Amps
void Int2Segs(unsigned int i, unsigned short ofs){
    unsigned short x;   // this gets removed by the optimizer
    SegBits tempDP;

    if ((ofs == dVolts) || (ofs == dAmps)) 
      tempDP = SegDP;
    else 
      tempDP = 0;

    x = i/100;
    DigitsBuf[ofs] = SegTable[x] | tempDP;

    x = (i/10)%10;
    DigitsBuf[ofs+1] = SegTable[x];

    x = (i%10);
    DigitsBuf[ofs+2] = SegTable[x];

EDIT2: Here's another, better way to do the DP - pass it in as a parameter..

//Conversion routine
// Appends passed decimal point to first digit
void Int2Segs(unsigned int i, unsigned short ofs, SegBits DPmask){
    unsigned short x;   // this gets removed by the optimizer

    x = i/100;
    DigitsBuf[ofs] = SegTable[x] | DPmask;

    x = (i/10)%10;
    DigitsBuf[ofs+1] = SegTable[x];

    x = (i%10);
    DigitsBuf[ofs+2] = SegTable[x];
}

Call Int2Segs with the desired DP for any field - in this one the time DP flashes at 1 sec intervals driven by the LSbit of the time register. Cheezy but it works:

      Int2Segs(Volts,dVolts,SegDP); // show each time for demo purposes with DP
      Int2Segs(Amps,dAmps,SegDP);

      if(SecElapsed){      // maintain multibyte timers
        SecElapsed = 0;      // ack flag
        SystemTime_Secs++;   // bump system timer by one sec
        if (SystemTime_Secs & 0x0001)  // one way to flash the decimal if its running
           Int2Segs(SystemTime_Secs,dTime,SegDP);// update display
        else
           Int2Segs(SystemTime_Secs,dTime,0);// update display - DP off
        } // one second elapsed

Good catch on the bugs. That's what happens when you block-copy and get sloppy.

Isn't it late there?

 

Ahh, we know you really gave him the bugs so he can get some experience debugging.

 

It was late . Like you said I went to bed to wait later.
I was a bit bzy on Friday so was a bit tired. Checked the code and fiddled with it and thought I would let you know before crashing for the night.

Horrors! A hardware solution to a firmware problem?

Hey it was just a thought. I'd rather let the software do it, tht's why I asked. U know.

Ahh, we know you really gave him the bugs so he can get some experience debugging.

True or not, it was a good lesson.

Will try the DP part tonight. me at work now.

{ed}
I noticed something. Not an issue at this point but for your info.
The Amps display is not showing what I enter.
i.e
If I enter Amps = 050;
I see : 040 in the Amps segments.

Not all entered values are off only some. And I think the Volts entered value is also messing with the Amps display. Although Volts display is OK so far.

Is tht a bug I have to figure out. If so tell me, I will try.

 

Ahh, we know you really gave him the bugs so he can get some experience debugging.

uhhhh... Yes! Exactly.

Quote:
Originally Posted by JohnInTX
Horrors! A hardware solution to a firmware problem?

Hey it was just a thought. I'd rather let the software do it, tht's why I asked. U know.

Just playing with you but its a valid point anyway. Avoid the temptation to run off and change a valid design when you hit a snag until you know for certain that it won't work and why. Why introduce the uncertainties and challenges that a new implementation brings unless its absolutely necessary? And from a practical point, firmware changes are way cheaper to implement than hardware changes.

If I enter Amps = 050;
I see : 040 in the Amps segments.

Ahhh.. did you know that C's default radix was/is octal? The leading '0' of '050' makes it a constant in octal and 050octal == 40decimal. Note the different colors - green for decimal, purple for octal. Don't feel bad - I had a brief WTF moment on that one as well - especially since there were possible issues with the Int2Segs stuff below.

And I think the Volts entered value is also messing with the Amps display. Although Volts display is OK so far.

I noticed that with my hand-rolled digit extraction code as well. While the digits were OK, the compiler occasionally would take the result as a 2 byte value, clobbering the adjacent byte. I looked in the manual for how they did it and it was a bit different. Changed it and verified in MPSIM that it no longer wrote multiple bytes. Hmmm... Even if the digit extraction was faulty, it should only write one byte to the array... Anyway, the last snippets I posted with the DP stuff have the revised DIVs and MODs and seem to work.

Debugging note - I made it easier to debug by temporarily initializing the segment buf to all 0xff and commenting out the segment lookup so that the extracted digits were put in the buffer instead of hard-to-decipher segment patterns. Since the extracted digit can only be 0-9, 0xff provides easy to see boundaries on the digit fields.

Oversight note - I also neglected to review the compiler messages to see if there were any warnings about data type conversions, etc. that might have shed some light on why adjacent fields got clobbered. That might have helped.

This is a good time to suggest that you maintain a software log with descriptions of versions etc but also with a KNOWN ISSUES section. These are known problems or other things that should be checked out (like this one) or have been deferred for various reasons. When you can, revisit each issue and resolve it. Move the description to the RESOLVED ISSUES section (so you can remember where the trouble spots were) with the solution. If the problem resurfaces, you have a starting place for dealing with it. And.. no code is ready for release until the only thing in the KNOWN ISSUES section is *NONE*.

 

Made a mistake. Took me a while trying to figure the issue.

It was from my typo's

This is working just too beautifully u know.
I got to admit, I will be like 80 when I can do what you do