What are you talking about? Your 'keychg' variable isn't persistent. You overwrite it each time through the routine.

You are misreading my code. The line:

       retbc    TC8ms              ;process kbd row every 8 ms (total 32 ms debounce time)

is a macro that expands to:

    btfss    TC8ms
    return

TC8ms is a flag that is set every 8ms elsewhere in my 'heart beat' routine, and is active for only for a single main loop. Therefore, the code following is only executed every 8 ms, not each time the routine is called.

Actually, four samples at the same state at 8-msec intervals is a span of 24-msecs so your code requires a minimum steady state 'press' or 'release' of between 24 and 32 msecs.

Of course you are correct. Trying not to confuse the issue.

Nonsense!

Why are you taking this personally? So far, you've called my code 'clumsy', and my analysis 'nonsense'. Correct me where I am wrong, but ad hominems don't seem appropriate.

My routines work just as well, even when using a pair of bare wires. In this case, if the extra code and registers don't improve performance or add features they're simply unnecessary extra code and registers (grin).

In my experience, it does make a huge difference, for reasons I have already explained. We can leave this as a difference of opinions.

 

Here is some sample code for an Arduino, which detects button changes via interrupts, debounces the inputs and detects both a long press and a short press. It is coded for a 1 second long press. Anything shorter is a short press. It has a relatively long 50 ms time period during which changes are ignored, except for resetting the debounce window (e.g., a button state - pressed/released - must be stable for 50 ms.).

As you can see, the hardware interrupt pin used is pin 2. I defined the interrupt to fire on any change of the pins status. It also supports rising and falling edge detection.

I have used this code, and like @MMcLaren, it works with bare wires. (That is how I tested it.) It is wordy, but it makes it easier to understand for me. Boolean variables are used liberally to track the state of the button.

// Button input related values
const byte BUTTON_PIN = 2;
const int  STATE_NORMAL = 0; // no button activity
const int  STATE_SHORT = 1; // short button press
const int  STATE_LONG = 2; // long button press
volatile int resultButton = 0; // global value set by checkButton()

..... main code

void setup() {
...
  // initialize input button pins
  pinMode(BUTTON_PIN, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(BUTTON_PIN), checkButton, CHANGE);
...
}


void checkButton() {
  /*
   * This function implements software debouncing for a two-state button. It responds to a short press and a long press
   * and identifies between the two states. Your sketch can continue processing while the button function is driven by
   * pin changes. It returns the button state in a global variable "resultButton"
   */

  const long LONG_DELTA = 1000; // hold seconds for a long press
  const long DEBOUNCE_DELTA = 50; // debounce time
  static int lastButtonStatus = HIGH; // HIGH indicates the button is NOT pressed
  int buttonStatus;                       // button atate Pressed/LOW; Open/HIGH
  static long longTime = 0, shortTime = 0; // future times to determine is button has been poressed a short or long time
  boolean Released = true, Transition = false; // various button states
  boolean timeoutShort = false, timeoutLong = false; // flags for the state of the presses

  buttonStatus = digitalRead(BUTTON_PIN); // read the button state on the pin "BUTTON_PIN"
  timeoutShort = (millis() > shortTime); // calculate the current time states for the button presses
  timeoutLong = (millis() > longTime);

  if (buttonStatus != lastButtonStatus) { // reset the timeouts if the button state changed
    longTime = millis() + LONG_DELTA;
    shortTime = millis() + DEBOUNCE_DELTA;
  }

  Transition = (buttonStatus != lastButtonStatus);
  Released = (Transition && (buttonStatus == HIGH)); // for input pullup circuit
  lastButtonStatus = buttonStatus; // save the button status

  if ( ! Transition) {
    resultButton =  STATE_NORMAL | resultButton;
    return;
  } // if there has not been a transition, return the normal state
  if (timeoutLong && Released) { // long timeout has occurred and the button was just released
    resultButton = STATE_LONG | resultButton;
  } else if (timeoutShort && Released) { // short timeout has occurred (and not long timeout) and button ws just released
    resultButton = STATE_SHORT | resultButton;
  } else { // else there is no change in status, return the normal state
    resultButton = STATE_NORMAL | resultButton;
  }
  return;
}

 

Why are you taking this personally?

Forgive me for "picking nits". It's not personal. Just take your time and be concise.

Cheerful regards, Mike

 

I am having trouble.
I can only change PWMbaseK and Whitepct.
If both are 0. Led is OFF and any value above 2 Led is Full on.
No intensity change.
I was wondering without Timer Int. how can it PWM ??

 

I am having trouble.
I can only change PWMbaseK and Whitepct.
If both are 0. Led is OFF and any value above 2 Led is Full on.
No intensity change.
I was wondering without Timer Int. how can it PWM ??

Post your code?

 

@R!f@@
I combined several old test codesets and got this - it does what you want. Look through it to see what I did (and if you have any improvements).
This is for XC8 but the IO definitions are in one place so you can probably port to MikroC without much problem. If you have MPLABX, it builds fine using the free version of the compiler.

Note the use of ONE timer interrupt to do several different functions with NO delay() stuff.

You can blast and run this on a 12F629. See the IO definition section for the hookup info.
There are doubtless other/maybe better ways to do it but this works pretty well for a quickie.
The zip is the .HEX file you can program your chip with to see how it works.
The scope shots show a couple of settings.
Enjoy!

/*
* File:   PWMplay.c
* Author: JohnInTX
* Created on November 4, 2015, 18:23
* XC8 FREE or PRO, Ver 1.35
* This implements a quadrature encoder, PWM output and a blinking LED (heartbeat)
* just for grins.  All timing-sensitive functions are interrupt driven including
* encoder switch debounce, PWM output and derived timers (to do things like
* flash LEDs).
* PIC16F629
*
* Known issues:
* ENstate not fully initialized on power up
* CONFIG should be visited for final target.
*/

#include <xc.h>

//********************* PIC CONFIG  ***********************************
// CONFIG
#pragma config FOSC = INTRCIO   // Oscillator Selection bits (INTOSC oscillator: I/O function on GP4/OSC2/CLKOUT pin, I/O function on GP5/OSC1/CLKIN)
#pragma config WDTE = OFF       // Watchdog Timer Enable bit (WDT disabled)
#pragma config PWRTE = OFF      // Power-Up Timer Enable bit (PWRT disabled)
#pragma config MCLRE = ON       // GP3/MCLR pin function select (GP3/MCLR pin function is MCLR)
#pragma config BOREN = OFF      // Brown-out Detect Enable bit (BOD disabled)
#pragma config CP = OFF         // Code Protection bit (Program Memory code protection is disabled)
#pragma config CPD = OFF        // Data Code Protection bit (Data memory code protection is disabled)

//********************** PROGRAM CONFIGURATION  *****************************
// Change these as required
#define TMR0period_us  250            // how many usec per TMR0 irq
#define ENdebounce_ms  5              // how many ms to debounce the encoder
#define HeartBeatPeriod_ms 150        // how fast to flash the HB LED

#define PWMbaseN 50                   // how many TMR0 IRQs in one PWM cycle
#define PWMchangePerClick 1           // how much to change duty cycle each encoder click
#define InitialPWMDutyK 12             //PWM on power up (actual counts, not percent)

//********************* IO DEFINITIONS  *******************************
// IO is shadowed i.e. set/clear bits in GPIOimg, NOT on GPIO directly.
// The interrupt will write the port.
// This avoids having to turn off the IRQ when main does IO

// IO PINOUT:
// GP0 Encoder PhaseA input - pulled up with 10K
// GP1 Heartbeat LED output - 1 = LED ON
// GP2 Encoder PhaseB input - pulled up with 10K
// GP3/MCLR - Pulled up with 10K (can disable in CONFIG)
// GP4 Not used in standard program.
// GP5 PWM output: 1 = PWM ON

#define ENphaseAmask  0x01      //Phase A is on GPIO.0
#define HeartBeatmask 0x02      // Heartbeat LED on GPIO.1
#define ENphaseBmask  0x04      //Phase B is on GPIO.2
#define PWMoutmask    0x20      // PWM out is GPIO.5

#define TOGGLE_HEARTBEAT (GPIOimg ^= HeartBeatmask)

#define ENCODER_MASK (ENphaseAmask | ENphaseBmask) // bits with encoder inputs
#define TRISinit 0b11000101     // Encoder is input, others out
#define GPIOinit 0b11000101     // Inputs 1, outputs 0
#define CMCONinit 0x7           // Comparators OFF
#define OPTIONinit 0b10001000   // No pullups, TMR0 runs at Tcyc

// *************** CALCULATED CONSTANTS  *******************************

#define TMR0reload 256-TMR0period_us;    // what to reload TMR0 to
#define msec5PSset 5000/TMR0period_us  // count this many IRQs for 10msec
#define ENdebTIKS   ((ENdebounce_ms *1000) / TMR0period_us)
#define HeartBeatTimerSET (HeartBeatPeriod_ms /(5*2)) // flash heartbeat period
#define PWMdutyKinit ((InitialPWMpct * PWMbaseK)/100);

//********************** VARIABLES  **********************************
// Encoder
unsigned char curENCODER,newENCODER,lastENCODER,ENdebK,ENstate,GPIOimg;
bit NEW_ENCODER_STATE;

// PWM
unsigned char PWMbaseK,PWMdutyK,PWMduty;

// Derived timers and heartbeat
unsigned char msec5PreScaler,HeartBeatTimer_5ms;

//********************** INTERRUPT SERVICE  **************************
// Interrupts when TMR0 rolls over. Reloads it to trim period then
// processes items below.
// Timer 1 is unused.
// Note that IO is done here.  No direct outputs to GPIO in main is allowed
// unless interrupts are disabled - not super good when running a software
// PWM.  See Heartbeat in main to see how its done.

void interrupt isr(void)
{
    TMR0 = TMR0reload;      // reload TMR0 to trim period
    T0IF = 0;               // clear IRQ flag

    //------------------ SERVICE PWM  --------------------------------
    // PWMbaseK counts PWMbaseN interrupts to make the PWM period
    // The PWM output will be high for the fraction of this period
    // determined by PWMdutyK.
    GPIOimg &= ~PWMoutmask;     // assume it will be 0
    if(PWMbaseK){
        PWMbaseK--;
        if(PWMdutyK){
           PWMdutyK--;
           GPIOimg |= PWMoutmask;
        }
    }
    else{
        PWMbaseK = PWMbaseN;
        PWMdutyK = PWMduty;
        //GPIOimg |= PWMoutmask;
        }
    GPIO = GPIOimg;         // update port

    //------------------- SERVICE ENCODER  ---------------------------
    // Reads switch and compares current value to last valid one.
    // Iff its different, it attempts to debounce it by counting
    // each time it enters the interrupt.  When the switch is stable
    // for ENdebTIKS, the previous and current value are combined into
    // one 4bit value which indicates which way the switch turned.
    // See the encoder processor in main for more details.
    // Note that the switch inputs can be on any GPIO input.  This sorts that
    // out.

    curENCODER = (GPIO & ENCODER_MASK);
    if(ENdebK == 0){        // 0 means not debouncing any changes
        if(curENCODER != lastENCODER){  // but it has just changed so..
            ENdebK = ENdebTIKS;
            newENCODER = curENCODER;    // save value to compare
        }
        else;   // else its not changed, do nothing
    } // was not debouncing, debK >0 iff it just started a new one
    else{  // it is currently debouncing
        if(curENCODER != newENCODER){  // changed during debounce!
            ENdebK = ENdebTIKS;         // reset debouncer w/new value
            newENCODER = curENCODER;    
        }
        else{   // still same value
            if(--ENdebK == 0){ // dec and test debounce counter iff Z..
                lastENCODER=newENCODER; // update 'last' value for next click
                ENstate = (ENstate << 2);       // shift is to was
                ENstate &= 0b00001100;  // clear out old and unused bits
                                        // then combine new inputs to two bits
                if(newENCODER & ENphaseAmask) ENstate |= 0x01;
                if(newENCODER & ENphaseBmask) ENstate |= 0x02;
                NEW_ENCODER_STATE = 1;
            } // debounce complete-new ENstate
        }// encoder value was same during debounce
    }// its currently debouncing

    //---------------------- SERVICE DERIVED TIMERS  ---------------------------
    // Prescaler counts msec5PSset interrupts to make a 5msec period.
    // From that, as many timers can be created as desired.  Set the timer
    // to the number of IRQ periods to time.  The timer will run to 0 and stop.
    // Poll the timer, when its 0, time is out. Do something, reload and go.
    if(msec5PreScaler == 0){            // iff prescaler ran out..
        msec5PreScaler = msec5PSset;    // reset it and run timer(s)
        if(HeartBeatTimer_5ms) HeartBeatTimer_5ms--;
    }
    else
      msec5PreScaler--;
}// IRQ service

//********************* INCREMENT AND DECREMENT  *****************************
// Called by the encoder switch state decoder.  Increments/Decrements PWMduty
void increment(void)
{
    if ((PWMduty + PWMchangePerClick) >= PWMbaseN)
        PWMduty = PWMbaseN;
    else
        PWMduty += PWMchangePerClick;
}

void decrement(void)
{
   if (PWMduty < PWMchangePerClick)
           PWMduty = 0;
   else
       PWMduty -= PWMchangePerClick;
}

//************************ MAIN  *********************************************
void main(void) {
    INTCON = 0;         // zap all IRQs
    PIE1 = 0;
    //--------------- CONFIGURE THE SYSTEM  -------------------
    OPTION_REG = OPTIONinit;
    CMCON = CMCONinit;

    //---------------- INIT HARDWARE IO  ----------------------
    GPIOimg = GPIOinit; // this must agree with the next line
    GPIO = GPIOinit;    // Init the I/O f/fs first.. then TRIS
    TRISIO = TRISinit;

    //--------------- INIT VARIABLES  --------------------------
    ENdebK = 0;
    NEW_ENCODER_STATE = 0;
    PWMduty = InitialPWMDutyK;       // starting output
    lastENCODER = (GPIO & ENCODER_MASK);  // init the encoder

    //---------------  START TIMER 0 IRQ  -----------------------
    TMR0 = 0;
    T0IF = 0;
    T0IE = 1;
    GIE = 1;

    while(1){
        //--------------------- PROCESS ENCODER  ---------------------
        // Iff NEW state has been posted, process it.
        // ENstate is formatted as 0000llcc where
        //  ll is the last value of the switch
        //  nn is the current value.
        //  The switch runs a gray code (only one bit changes at a time). By
        //  combining the last and current bit inputs, a unique number
        //  0000 to 1111 is created.  This number indicates what happened when
        //  the switch was turned.  For example:
        //  Last value  Current value   State   What happened
        //-------------------------------------------
        //  00          01              1       Rotated CW one click
        //  00          10              2       Rotated CCW one click
        //  and
        //  10          00              8       Rotated CW one click
        //  10          11              11      Rotated CCW one click
        //  also
        //  00          11              3       Illegal, both bits changed,
        //                                      ignored.
        // A total of 4 states indicate CW and 4 indicate CCW.  The other 8
        // possibilities are the 4 possible both bit changes and 4
        // possible no bit changes.  These are ignored.
        // A valid CW or CCW calls increment/decrement which adjusts the PWM.

        if(NEW_ENCODER_STATE){
            NEW_ENCODER_STATE = 0;
            switch (ENstate){
                case 0b00000001:
                case 0b00000111:
                case 0b00001000:
                case 0b00001110:
                    increment();
                    break;
       
                case 0b00000010:
                case 0b00000100:
                case 0b00001011:
                case 0b00001101:
                    decrement();
                    break;
                // ignore others: bad reading         
            }// switch
        }// if NEW_ENCODER_STATE

        //------------------- SERVICE THE HEARTBEAT LED  --------------------
        // Once a system tik is operating, other derived timers can be
        // implemented simply.  This is an example of flashing an LED
        // with no delays or significant impact on the other code.
     if(HeartBeatTimer_5ms==0){
         HeartBeatTimer_5ms = HeartBeatTimerSET;
         TOGGLE_HEARTBEAT;
     }

    }// main while loop
}

 

Post your code?

same code post #31

I will give the code at #46 a try and will post back.

 

May I ask what type of encoder you're using, please? Does it have detents and cycle through all four AB phases between detents? If so, that will affect your code.

Regards, Mike

 

Quadrature encoder.
I believe it puts out a Grey code.
I have done the encoder coding before with the help of John and RB.
Encoder is not a problem for me.
I have never done PWM ever.

 

May I ask again if it has detents, please?

 

JohnInTX, while it's probably not critical in this application, I thought I should mention you're not reloading TMR0 correctly for 250-us interrupt intervals.

    #define TMR0reload 256-TMR0period_us;    // what to reload TMR0 to

    void interrupt isr(void)
    {
      TMR0 = TMR0reload;     // reload TMR0 to trim period
      T0IF = 0;             // clear IRQ flag

I believe you should add the reload value to free-running TMR0, plus one for the increment that occurs during the write, plus two for the TMR0 delay after the write, in order to account for all the TMR0 cycles that occur between the TMR0 overflow and the TMR0 reload. Of course this only works when using a 1:1 TMR0 prescale setting because any write to TMR0 will reset the prescaler, but you've got that covered.

    #define TMR0reload 256-TMR0period_us+3;  // what to reload TMR0 to

    void interrupt isr(void)
    {
      TMR0 += TMR0reload;    // reload TMR0 to trim period
      T0IF = 0;             // clear IRQ flag

 

I am having trouble.
I can only change PWMbaseK and Whitepct.
If both are 0. Led is OFF and any value above 2 Led is Full on.
No intensity change.
I was wondering without Timer Int. how can it PWM ??

PWMbaseK determines the number of counts in one PWM cycle. Its not changed in normal operation. Whitepct determines a count value that is some fraction of that. You'll see in the 'final' code that the base count is chosen and the duty count is between 0 and that value.

I believe you should add the reload value to free-running TMR0,

You're right. You're also correct about the prescaler issue. I originally ran TMR0 with no reload for a 256us period but thought I'd provide a method for speeding things up and to illustrate that you have to add the reload value. Missed the '+='. Oops. There is no TMR2/PR2 in this one and TMR1 is 16bits with all that that entails.

Does it have detents and cycle through all four AB phases between detents?

?? You get one phase change per detent on the ones I am familiar with, no? One click, one bit change.
FWIW: The encoder I used is a cheap mechanical one with no detents.
I generally use IOC with a nice Grayhill optical (no bounce) unit but this worked out well. I mainly wanted to try it and see what potential issues there may be. Something for my bag-o-tricks.

I will give the code at #46 a try and will post back.

First try just blasting a chip with the .HEX I posted. That will confirm the hardware/hookup.

 

Forgive me for jumping ahead but could I make a "feature creep" suggestion, please?

When using "short" and "long" switch presses in the past I found it very helpful to use audible feedback similar to that found on some commercial products. That is, I would generate a single beep on a debounced "new press" and if I held the switch for a "long press" I would generate a double beep at the one second timeout which would let me know that I could release the switch. It was kind of a neat feature and it worked well.

May I suggest using one of the left-over pins for a piezo speaker along with a non-blocking beep task in the ISR to drive it? That 250-us interrupt interval would be perfect for generating beeps for a piezo with a 2000-Hz resonant frequency. In the example below, a 'beep' value of '1' or '3' will produce one or two 32-msec 2000-Hz beeps with 32-msec spacing between multiple beeps.

     /*                                                               *
      *  within the 250-usec interrupt routine                        *
      *                                                               *
      *  beep task generates one or more 32-msec 2000-Hz beeps        *
      *                                                               */
         static unsigned char duration = 32000/250;

         if(beep)                   // if beep task running
         { spkr ^= beep.0;          // toggle spkr if beep.0 is '1'
           if(--duration == 0)      // if 32-msec timeout
           { duration = 32000/250;  // reset 32-ms duration and
             beep--;                // decrement beep counter
           }                        //
         }                          //

You would task the beeps somewhere in your code, whether in the main loop or the ISR, where you detect the debounced new press and the long press conditions.

  /*
   *  this code executed at 25-msec intervals
   */

      if(newpress)                // if "new press"
      { swtmr = 1000/25;          // start 1-second timer
        beep = 1;                 // task a "new press" beep
      }                           //

      if(newrelease)              // if "new release"
      { if(swtmr)                 // if not 1-second timeout
        { flags.norm ^= 1;        // toggle "short" flag and
          swtmr = 0;              // clear 1 second timer
        }                         //
      }                           //

      if(swtmr)                   // if timer running
      { swtmr--;                  // decrement it
        if(swtmr == 0)            // if timed out
        { flags.long ^= 1;        // toggle "long" flag
          beep = 3;               // task a double beep
        }                         //
      }                           //

 

@JohnInTX
Will do John.

@MMcLaren
Beeping is a nice addition. I will see into it after I get the code working.
I will show the encoder I have. A picture is better.

By the way passive beep or active beep. I believe EasyPIC has an active setup

 

?? You get one phase change per detent on the ones I am familiar with, no? One click, one bit change.

Good question. I've used three different mechanical types from pretty decent manufacturers, including the inexpensive Bourn's PEC11 series, and all of them transitioned through all four phases between detent positions. Decoding all four phases between detents when you only want to process a single "up" or "down" action between detents may seem like a coding nightmare but it actually turns out to be pretty simple. Basically, I realized I could run the encoder A and B inputs through the same parallel switch state filter that I use for push buttons and it will produce exactly one "new press" per detent for both the A and B switches (whether they're wired active hi or active lo). Then I simply lock onto either an A "new press" or a B "new press" and check the opposite input to determine direction. Example code runs at ~500-usec intervals but I suspect ~250-usec intervals would work too;

     /*                                                            *
      *  K8LH parallel switch state logic (new press filter)       *
      *                                                            *
      *  swnew  ___---___---____   invert active lo switches       *
      *  swold  ____---___---___   switch state latch              *
      *  swnew  ___-__-__-__-___   changes, press or release       *
      *  swnew  ___-_____-______   filter out 'release' bits       *
      *                                                            */
         swnew = ~gpio;         // invert active lo switches
         swnew ^= swold;        // changes, press or release
         swold ^= swnew;        // update switch state latch
         swnew &= swold;        // filter out 'release' bits
     /*                                                            *
      *  check the rotary encoder                                  *
      *                                                            */
         if(swnew.5)            // if encoder B "new press"
         { if(swold.4)          // if encoder A state = 1
           { if(dcy < 63)       // if duty cycle < upper limit
               dcy++;           // increment duty cycle
           }                    //
           else                 //
           { if(dcy > 0)        // if duty cycle > lower limit
               dcy--;           // decrement duty cycle
           }                    //
         }                      //

 

Here is what I use

 

@MMcLaren Boy, that seems weird to me but I can't argue with your approach. Maybe I've been spoiled (or lucky!) with my Grayhills. But that's kind of why I wanted to contribute here - to find out things like this. It looks like my little no-name encoders are a similar Bourns series, right angle with no detents. Couldn't tell you what the PPR is.
@R!f@@ That should work as well.

 

@JohnInTX
The Hex file you asked to test works.
Led at GP1 is blinking ( no change with encoder).
Led at GP5 dims and brightens with encoder rotation but fast rotation has no effect. I need turn slowly.
So I connected the LED strip circuit to see.
It does work but I can see Strip flickering in the middle brilliance settings. It can off it completely and full on is OK. I can say between 30 to 95% range strip flickers but the LED does not.
At lowest Dim range possible there is no flicker what's so ever. But it is a bit bright than I want in the lowest range

 

Aaah ! Detents ( one that makes clicking sound when rotated ) If this is what you guys were asking.
I had already made a ckt board for the charger we were making while back. That PCB has two encoders with jumpers that I can hook up to Easy PIC.
One with detents and one without. The detent one has difficulty in adjusting brightness with this file but the one without works quite easily.
I need turn the detent one much slower than the one without to adjust the PWM

 

So what's next ?
Want me to rewrite the code in mikrC