Sure.

Code works for char SettingTimeOut too u know
u know the rollover part in post #108
I cannot get it done. I can get up to 255. It never decrements.

Perhaps the old 'dangling else'?

{
 Enc_New = (PORTB & 0xC0);       // keep on RB7 and RB6 bits.
 if(Enc_New != Enc_Old)            // If Encoder moved.
{
   if(Enc_New.RB7 == Enc_Old.RB6)   // Check Direction.
     if(TopUp_Time)TopUp_Time++;    // If CW, increment TopUp.
     else; // 'completes this 'if' to avoid 'dangling else'
   else // was CCW
     if(TopUp_Time) TopUp_Time--;// decrement TopUp.

   Enc_Old = Enc_New;              // Save Encoder status.
 }
  Int2Segs(TopUp_Time,dTime,SegDP); // update display.
}

'else' belongs to the immediately preceding 'if'. Sometimes you have to put in a dummy else; to keep them associated.

 

Are u referring to the part I high lighted in RED

No, comments are good too, but

while(1)

doesn't mean anything.
You might as well be using one of the the real conditions in context to hold the loop

while(Heatsink_threshold != 0)

No biggie, but it's more useful later.

 

That's quite a lot of progress for the one thread.
Did you get the beeping/buzzer sorted out?
You can do something cheap and nasty in your display routine.

Your display routine:

// Display routine that drives the multiplexed digits
//
bla
bla
something about colums
bla
bla
something about segments
bla
bla

Cheap way to drive piezo:

// Display routine that drives the multiplexed digits
//
piezo port = 1
bla
bla
something about colums
bla
bla
something about segments
bla
bla
piezo port = 0

Then if you don't like the sound you can move the line that turns
the piezo port off a little higher in the display routine to alter the duty cycle.
The above will constantly beep, so you still need a variable to control it.

// Display routine that drives the multiplexed digits
//
if (beeping != 0) {
piezo port = 1
} // beeping
bla
bla
something about colums
bla
bla
something about segments
bla
bla
piezo port = 0

 

Sure. Perhaps the old 'dangling else'?
...
'else' belongs to the immediately preceding 'if'. Sometimes you have to put in a dummy else; to keep them associated.

I'm interested to see if that is the problem! Personally I've never experienced a "dangling else" problem in MikroC but then I normally put the single statement on the same line after an if(), or use { } curly braces to be sure.

And saw a minor bug in your code;
if(TopUp_Time)TopUp_Time++;
should be;
if(TopUp_Time<255)TopUp_Time++;

 

No buzzer. I used up all the ports so I will do this without beeping.

I did not know 2 "else" can be used.

Will check tonight...

Thanks a bunch

 

I'm interested to see if that is the problem! Personally I've never experienced a "dangling else" problem in MikroC but then I normally put the single statement on the same line after an if(), or use { } curly braces to be sure.

And saw a minor bug in your code;
if(TopUp_Time)TopUp_Time++;
should be;
if(TopUp_Time<255)TopUp_Time++;

I guess I usually use braces to force the association as well.. Dummy else; + no <255 = late at night.
Good catch.

 

I was wondering what you were doing so late at night.
John never post tht late before...

 

No buzzer. I used up all the ports so I will do this without beeping.

Not to throw a monkey wrench into the mix, but, you should post a schematic of your current design. Chances are you can mux' the switches and encoder A and B signal lines onto a single pin using the digit/column driver lines. Sample one switch during each ISR with relatively simple code.

Another idea... Have you thought about using a single three or four digit display and simply selecting what you want to view with <up> and <down> push button switches?

Keep up the good work.

Cheerful regards, Mike

 

I know I could mux the switches but for this I rather stick with what I had in mind.

I am trying to get the code working. The schema is no biggie but I do have a concern with the current shunt amp.
But I will tackle that issue once the code is done.

Here is the new suggestions implemented.. John and RB talked about the extra else. The extra else works too and later I was wondering where to put the curlies {}.

But this code works so beautifully u know

void Setting() {

    SystemTime_Secs = 0;
    SettingTimeOut = 31;             // Load 30 seconds to timer before loop.
  while(SettingTimeOut) {           // Remains in Loop for 30 seconds.

   if(Sec_Elapsed)                  // maintain multibyte timers
  {
    Sec_Elapsed = 0;                // ack flag
    SystemTime_Secs++;              // bump system timer by one sec
   if (SystemTime_Secs & 0x0001)    // one way to ash the decimal if its running
    Int2Segs(SystemTime_Secs,dVolts,SegDP);// update display
   else
    Int2Segs(SystemTime_Secs,dVolts,0);// update display - DP off
  } // one second elapsed*/
      
      
    Enc_New = (PORTB & 0xC0);       // keep only RB7 and RB6 bits.
   if(Enc_New != Enc_Old)           // If Encoder moved.
  {
   if(Enc_New.RB7 == Enc_Old.RB6)
   {
    if(TopUp_Time<255)TopUp_Time++; 
   }
   else
    if(TopUp_Time)TopUp_Time--; 

    Enc_Old = Enc_New;              // Save Encoder status.
    Int2Segs(TopUp_Time,dTime,SegDP);// update display
    //SettingTimeOut = 0;             // Resets Timeout counter if encoder moved
  }
 }
}

 

John and RB talked about the extra else. The extra else works too and later I was wondering where to put the curlies {}.

Searching on 'dangling else' brings up this page (and lots of others) that explain the issue in terms of designing a language. At the end of the page, at

Java, C and C++ have chosen to resolve the Dangling-Else ambiguity uses the following rule:

is essentially how K&R explain and resolve the problem using braces.

Upon reflection, RB's (and K&R's) braces are better than a dummy else - and actually how I write it when I'm functional.

 

Just a matter of general practice, I always type in the curly braces before filling in the code:

if ( )
   {
   }
else
   {
   }

Never encountered a dangling else.

 

Yeah I do that too.

But MikroC seems to be fine (from my experience) with single lining used on single operations;

if(x > 4) blah = 0;
else      blah = 1;

And I'm sure I've used a double if() like this;

if(Enc_New.RB7 == Enc_Old.RB6) if(TopUp_Time)TopUp_Time++;    // If CW, increment TopUp.
else                           if(TopUp_Time) TopUp_Time--;   // decrement TopUp.

And have no memory of any problems with it, although as a general rule if I put any decision making after an if(), or multiple statements, I do use curly braces.

 

where can I find this K&R.???

 

The C Programming Language 2ed ed. Brian W. Kernighan and Dennis M. Ritchie
This is an old book but is still a great reference. They wrote the language. There are many other books on C but K&R just won't go away. Maybe others on the board have some ideas as well.

The MikroC manual has some pretty good examples and descriptions of the various constructs as well.

 

/******* Setting function selector switch *****************
works only with T,V & I Setting */
void EncoderSwitch() {
    if(En_Sw !=0)                          // If selector Sw is pressed.
    {                                      // (not equal to zero)
      delay_ms(100);                       // Wait for 100ms.
    if(En_Sw != 1)                         // Wait till button is released.
     {
      EncSw_Count++;                       // Increment counter
     }
    }
    if(EncSw_Count == 1)                   // If counter is 1
    {                                      // Switch pressed once.
      Int2Segs(TopUp_Time,dTime,SegDP);    // Update Time display and
      CannedMsg(SEGmsg_Blank,dVolts);      // blank out Volts display,
      CannedMsg(SEGmsg_Blank,dAmps);       // blank out Amps display.
      Set_Time = 1;                        // Set Time flag.
      Set_Volt = 0;                        // Clear Volt flag.
      Set_Amp = 0;                         // Clear Amp flag.
    }
    if(EncSw_Count == 2)                   // If counter is 2
    {                                      // Switch pressed twice.
      Int2Segs(Volts,dVolts,SegDP);        // Update Volts display and
      CannedMsg(SEGmsg_Blank,dTime);       // blank out Time display,
      CannedMsg(SEGmsg_Blank,dAmps);       // blank out Amps display.
      Set_Time = 0;                        // Clear Time flag.
      Set_Volt = 1;                        // Set Volt flag.
      Set_Amp = 0;                         // Clear Amp flag.
    }
    if(EncSw_Count == 3)                   // If counter is 3
    {                                      // Switch pressed thrice.
      Int2Segs(Amps_Terminate,dAmps,SegDP);// Update Volts display and
      CannedMsg(SEGmsg_Blank,dTime);       // blank out Time display,
      CannedMsg(SEGmsg_Blank,dVolts);      // blank out Volts display.
      Set_Time = 0;                        // Clear Time flag.
      Set_Volt = 0;                        // Clear Volt flag.
      Set_Amp = 1;                         // Set Amp flag.
    }
    if(EncSw_Count == 3)                   // If counter is 3
      EncSw_Count = 0;                     // Reset Counter
}
/******* T,V & I Setting ***********************************/
void Settings() {
  while(1) {
      EncoderSwitch();                     // Read Encoder Selector Sw.
// Sets Top up time
    if(Set_Time == 1)                      // If Time flag is set.
      Enc_New = (PORTB & 0xC0);            // Keep the Last too Bits of PortB.
    if(Enc_New != Enc_Old)                 // If Encoder moved,
   {
    if(Enc_New.RB7 == Enc_Old.RB6)         // Look for direction.
    {                                      // If CW,
     if(TopUp_Time<200)TopUp_Time++;       // Increment TopUp Time if less than -
    }                                      // 200 until 200 is reached.
    else                                   // if not,
    if(TopUp_Time)TopUp_Time--;            // decrerase TopUp Time.
      Enc_Old = Enc_New;                   // Save the encoder position.
      Int2Segs(TopUp_Time,dTime,SegDP);    // Update Time display.
   }
// Sets the charging voltage and display
    if(Set_Volt == 1)                      // If Volt flag is set.
      Enc_New = (PORTB & 0xC0);            // Keep the Last too Bits of PortB.
    if(Enc_New != Enc_Old)                 // If Encoder moved,
   {
    if(Enc_New.RB7 == Enc_Old.RB6)         // Look for direction.
    {                                      // If CW,
      Volts = 840;                         // Charge voltage is 8.40V
    }                                      //
    else                                   // if not,
      Volts = 420;                         // Charge voltage is 4.20V.
      Enc_Old = Enc_New;                   // Save the encoder position.
      Int2Segs(Volts,dVolts,SegDP);        // Update Volts display.
   }
// Sets the Charge termination current
    if(Set_Amp == 1)                       // If Amps flag is set.
      Enc_New = (PORTB & 0xC0);            // Keep the Last too Bits of PortB.
    if(Enc_New != Enc_Old)                 // If Encoder moved,
   {
    if(Enc_New.RB7 == Enc_Old.RB6)         // Look for direction.
    {                                      // If CW,
    if(Amps_Terminate<500)Amps_Terminate++;// Increment Terminate current if less than -
    }                                      // 500 until 500 is reached.
    else                                   // if not,
    if(Amps_Terminate)Amps_Terminate--;    // decrease Terminate current.
      Enc_Old = Enc_New;                   // Save the encoder position.
      Int2Segs(Amps_Terminate,dAmps,SegDP);// Update Time display.
   }
// Sets the charging voltage selector relay
    if(Volts == 840)                       // If Volts is 8.4V
      CVs_Rly = 1;                         // Switch on charger voltage selector relay
    else                                   // else
      CVs_Rly = 0;                         // Switch off charger voltage selector relay
 }
}

This is what I came with for the Time, Volts and Amps settings.
I checked and I can cycle through Time, Volts and Amps using the Selector switch. Display is blanked except the setting one
I set the time max 200 and Amps max 500.

The voltage is just 4.20V or 8.40V which means if the relay is ON or OFF. Which is used to change the charger output from 4.2V to 8.4V.

Is this OK ?

 

I notice you are duplicating the same encoder processing within each control section.

It might be a better idea to handle the encoder processing once, and just set a signed variable showing if the encoder has moved + or - from its last position.

Or you could wrap the encoder read in a quick function that returns one of three values; -1, 0, +1.

Your code gets much simpler, like this;

// Sets the Charge termination current
    if(Set_Amp == 1)                       // If Amps flag is set.
    {
      e = read_encoder();
      if(e == 1 && Amps_Terminate<500)  Amps_Terminate++;    // Increment Terminate current.
      if(e == -1 && Amps_Terminate)     Amps_Terminate--;    // decrease Terminate current.
      if(e) Int2Segs(Amps_Terminate,dAmps,SegDP);  // Update display only if changed.
    }

Or John may have some suggestions on other ways to handle it?

 

I notice you are duplicating the same encoder processing within each control section.

It might be a better idea to handle the encoder processing once, and just set a signed variable showing if the encoder has moved + or - from its last position.

Or you could wrap the encoder read in a quick function that returns one of three values; -1, 0, +1.

Your code gets much simpler..
Or John may have some suggestions on other ways to handle it?

Not really. Roman's observations parallel my own -I like to defer all of the nuts and bolts of peripherals, IO, user inputs etc to lower levels that hide details from the upper functions. Then as RB suggests, just ask what the results are +1, -1 etc. Think black-boxes with a simple interface.

I've done lots of encoder switch stuff but all in assembler. RB's C approach looks like it works fine.

Some thoughts: Keep in mind that however you do it, you'll have to poll the switch code often enough to keep it updated. Also, consider what happens if you get slow and the switch has moved 2 clicks instead of 1. (I ignore it, update the current state and go from there).

But for now, if its doing the job, its OK. Like RB says, if you encapsulate the switch code such that its 1)called to update the switch state and 2)posts its results, it will be easier to use AND, if you find that it needs to be more real-time, you can do things like poll it from an interrupt, use different decoding, write it in assembler etc etc WITHOUT having to do expensive surgery on the main code.

Looks good!

 

I need to leave out the "TopUp_Time" and use the "Time" instead as the display show's a reset value when it resumes from over heat protections.

Now it is all good. When program resumes from protection, display shows the set values in Time and Current. etc prior to to protection

I am changing the encoder as suggested.

 

All's good.

I do not have any skips or update issues. The encoder works smoothly and switch selects and cycles without an issue.

Now comes the part I knew I will have no idea on how to do it.

One is the First Time display.
As you know it is a 2 Hr Timer.
Now it is going from 000 to 200.
Actually it should go from 0.00 to 0.59 and from 1.00 to 1.59 to 2.00.
And it should count down not up.

There is the current part too but for now this is what I like to know how to fix.

I am lost here.

 

One is the First Time display.
As you know it is a 2 Hr Timer.
Now it is going from 000 to 200.
Actually it should go from 0.00 to 0.59 and from 1.00 to 1.59 to 2.00.
And it should count down not up.

Counting down is the easy part, just do
if(SystemTime_Secs > 0) SystemTime_Secs--;
in the interrupt routine.

To format the system time (represented in seconds) to h:mm requires a little math similar to that used in Int2Segs where the raw passed integer was broken into individual digits for segment lookup.

You need another routine similar to Int2Segs (maybe Int2Time)that uses different math:

if i is the number of seconds then:

hours = i / 3600 ; //results in a single digit for hours (in your range)
min = (i %3600) /60;  //returns minutes 0-59
minH = min/10; // returns 10's digit of 0-59 minutes
mihL = min%10; // returns 1's digit

and the 3 segment indexes are hours, minH, minL. I didn't check the math all that much but seems right. You could probably simplify the math as well. For example this removes the /60

hours = i / 3600 ; //results in a single digit for hours (in your range)
mintemp = (i %3600)   //returns minutes part 0-59 min expressed in seconds
minH = mintemp/600; // returns 10's digit of 0-59 minutes
mihL = mintemp%600; // returns 1's digit

There are probably other simplifications (such as eliminating mintemp and letting the compiler handle the intermediate results) but you get the idea.