[solved]sketch using PCINT behaviour change with arduino version

Hi everyone

I need to use a rotary encoder as human interface on one of my projects. Considering the libraries I found, it turned out to be simpler to write my own code.

So as a test system, I made something to just change a value between 0 and 255 with increments of 5 and output the value both on LCD screen and on a PWM output (to control the speed of a pump in a heating system)

My sketch is the following :

#include <Wire.h> 
#include <LiquidCrystal_I2C.h>

int pin_sw = 10;
int pin_a = 11;
int pin_b = 12;
int pin_out = 6;
int state = 0;
int state_sub = 0;
volatile int counter = 0;
volatile boolean flag = false;
volatile boolean flagbtn = false;
int valeur = 0;

LiquidCrystal_I2C lcd(0x20,20,4); 

void encoder_read(void)
{
  uint8_t pin_states = PINB & PCMSK0;
  
  // Push button press.
  if ((pin_states&0x04)==0)
  {
    flagbtn += 1;
  }  
  pin_states = (pin_states>>3)&0x03;
  // state is now 0, 1, 2 or 3.
  if (pin_states!=state)
  {
    // Exor the old & new states to determine the rotation direction.
    int inc = ((pin_states>>1)^state)&0x01;
    if (inc==0) inc = -1;
    state = pin_states;
    
    // Reset on change of direction.
    if ((inc<0 && state_sub>0) || (inc>0 && state_sub<0)) state_sub = 0;
    
    state_sub += inc;
    if (state_sub<=-4 || state_sub>=4)
    {
      state_sub -= (inc<<2);
      counter += inc;
      flag = true;
    }
  }
}

ISR(TIMER2_COMPA_vect)
{
  bitClear(TIMSK2,OCIE2A); // Disable timer2 interrupts.
  encoder_read();
  bitSet(PCIFR,PCIF0); // Clear pending interrupts.
  bitSet(PCICR,PCIE0); // Enable PCINT0-7.
}

ISR(PCINT0_vect)
{
  bitClear(PCICR,PCIE0); // Disable PCINT0-7.
  bitSet(TCCR2A,WGM21); // CTC mode.
  bitSet(TCCR2B,CS20);
  bitSet(TCCR2B,CS22); // Clk div by 128.
  OCR2A = 250; // 2 ms.
  TCNT2 = 0; // Clear counter.
  bitSet(TIFR2,OCF2A);
  bitSet(TIMSK2,OCIE2A);
}

void setup(void)
{
  lcd.init();
  lcd.backlight();
  lcd.print("Essai codeur");
  delay(500);

  pinMode(pin_sw,INPUT_PULLUP);
  pinMode(pin_a,INPUT_PULLUP);
  pinMode(pin_b,INPUT_PULLUP);
  state = 0;
  state_sub = 0;
  counter = 0;
  flag = false;
  PCMSK0 = 0x1c; 
  bitSet(PCIFR,PCIF0); // Clear pending interrupts.
  bitSet(PCICR,PCIE0); // Enable PCINT0-7.
  
  lcd.clear();
  lcd.print("Valeur : ");
}

void loop()
{
  if (flagbtn & 1)
  {
    lcd.blink();
    if (counter)
    {
		valeur += counter*5;
		counter = 0;
		if (valeur>255) valeur=255;
		if (valeur<0) valeur=0;
		lcd.setCursor(9,0);
		lcd.print("    ");
		lcd.setCursor(9,0);
		lcd.print(valeur);
    }
  }
  else 
  {
    counter = 0;
    lcd.noBlink();
  }  
  
  analogWrite(pin_out,valeur);
}

I first used Arduino 1.0.5 to write that thing, and it worked pretty well. If I turn the encoder only, it does nothing. When I press the encoder button, a blinking cirsor is shown on the LCD and I can change the value wen turnig the encoder, then if I press again the button the cursor disapear and turning no more change the value.
That is the intended behaviour. It is a first test for parameter change for a heating system controller I'm making.

Then I upgraded My arduino IDE, and nightmare started... I first installed the current version, 1.8.2 but then LiquidCrystal_I2C I'm using no more works, and displays only the first character of things I want to display.
Doing some tests I found 1.6.0 to be the last version wich produce something working with LiquidCrystal_I2C. But using this version, my encoder code is not working.
With this version I can press the button the first time to edit the value, then I can edit the value... but I can't go back. Further press on the button are ignored, but rotating is still counting so interrups looks like to work.

Can someone explain me why my interrupt code is working with some version and not with another one ?
Thanks

It looks like you've flipped the two interrupts: the timer interrupt is calling the encoder_read() and the PCINT interrupt is resetting the timer.

Yes this is intended : the PCINT is lauching the timer with 2ms delay, and at the end of this 2ms the timer interrupt lauch encoder_read(). The goal of this is to debounce the encoder switches. And it works well with 1.0.5.
I could have done this with delay() as timing is not critical here.

The pretty strange thing is the interrupts are still working for the encoder switches, only the button one works only the first time.

Another strange thing : when compiling the sketch with 1.8.2 I get a 3960 bytes binary file, when compiling with 1.0.5, I get 4916 bytes file. Is the new compiler capable of nearly 1kb optimisation or is something missing in the file ?

I first installed the current version, 1.8.2 but then LiquidCrystal_I2C I'm using no more works, and displays only the first character of things I want to display.
Doing some tests I found 1.6.0 to be the last version wich produce something working with LiquidCrystal_I2C.

This problem is a liquid crystal library issue and should be fixed with a current version of of the library. Forum search "i2c lcd only prints first character" for more information.

With this version I can press the button the first time to edit the value, then I can edit the value... but I can't go back. Further press on the button are ignored,

volatile boolean flagbtn = false;
flagbtn += 1;

This is not the correct way to handle the boolean. There were probably changes in the ide as to how values other than 0 and 1 are treated.

cattledog:
This problem is a liquid crystal library issue and should be fixed with a current version of of the library. Forum search "i2c lcd only prints first character" for more information.

Right ! found explanation and solution here : 5V IIC/I2C/TWI LCD Module Adapter For Arduino from eBay - #3 by bperrybap - Displays - Arduino Forum

volatile boolean flagbtn = false;

flagbtn += 1;




This is not the correct way to handle the boolean. There were probably changes in the ide as to how values other than 0 and 1 are treated.

Good catch ! I want to say thank you VERY much for this !! I tried changing almost everything else but did not notice the problem here... changing the variable to uint8_t solved the problem, that way the first bit changes state each time, as intended.