Rotary encoder (interrupt) + LCD + DS3231 = weird stuff happening!

Hi there fellow Arduino'ers;
I'm building an alarm clock with radio (RDA5807) that disables when you put your both feet on the carpet.
But I'm still in the beginning, so that was just some background info.

I currently have the following code which just displays hour, time and temperature (from DS3231) on the lcd without any problem.

#include <Wire.h>
#include <LiquidCrystal.h>

//lcd
LiquidCrystal lcd(12, 11, 5, 6, 7, 8);

byte tempC[8] = {
  0b01100,
  0b10010,
  0b10010,
  0b01100,
  0b00000,
  0b00000,
  0b00000,
  0b00000
};
byte tempChar[8] = {
  0b00100,
  0b01010,
  0b01010,
  0b01010,
  0b01110,
  0b11111,
  0b11111,
  0b01110
};

//DS3231
#define DS3231_I2C_ADDRESS 0x68
byte time_array[7];

//Rotary encoder

#define B_SIG 4
int pulses=0;

void setup()
{
  Wire.begin();
  Serial.begin(9600);
  //lcd
  lcd.begin(16,2);
  lcd.createChar(0, tempC);
  lcd.createChar(1,tempChar);
  //rotary encoder
  attachInterrupt(1, A_RISE, RISING);
  pinMode(B_SIG,INPUT);
}

void loop()
{

  Serial.println(pulses);
  displayHome();
  delay(50);
}


/*DS3231*/

// Convert normal decimal numbers to binary coded decimal
byte decToBcd(byte val)
{
  return( (val/10*16) + (val%10) );
}
// Convert binary coded decimal to normal decimal numbers
byte bcdToDec(byte val)
{
  return( (val/16*10) + (val%16) );
}
void readTime()
{
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0); // set DS3231 register pointer to 00h
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS, 7);
  // request seven bytes of data from DS3231 starting from register 00h
  for(int i = 1; i < 8; i++)
  {
   time_array[i] = bcdToDec(Wire.read()); 
  }
}
  
  void displayHome()
  {  
  readTime();
  lcd.setCursor(0,0);
  lcd.print(time_array[3], DEC);
  lcd.print(":");
  if (time_array[2]<10)
  {
    lcd.print("0");
  }
  lcd.print(time_array[2], DEC);
  lcd.print(":");
  if (time_array[1]<10)
  {
    lcd.print("0");
  }
  lcd.print(time_array[1], DEC);
  lcd.setCursor(11,0);
  lcd.write(1);
  
  lcd.print(getTemp(),DEC);
  lcd.write(byte(0));
  lcd.print("C");
  lcd.setCursor(0,1);
  switch(time_array[4]){
  case 1:
    lcd.print("Zon");
    break;
  case 2:
    lcd.print("Ma");
    break;
  case 3:
    lcd.print("Di");
    break;
  case 4:
    lcd.print("Woe");
    break;
  case 5:
    lcd.print("Do");
    break;
  case 6:
    lcd.print("Vr");
    break;
  case 7:
    lcd.print("Zat");
    break;
  }
  lcd.print(" ");
  lcd.print(time_array[5], DEC);
  lcd.print("/");
  lcd.print(time_array[6], DEC);
  lcd.print("/");
  lcd.print(time_array[7], DEC);
  }
  
  int getTemp()
{
  int temp;
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0x11);
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS,2);
  delay(15);
  byte MSB = Wire.read();  

  byte LSB = Wire.read()>>6;  
  if ((MSB & 0x80) != 0)
        temp = MSB | ~((1 << 8) - 1);      // if negative get two's complement
   else
        temp = MSB;
    temp = 0.25 * LSB + temp;
    return temp;
}

/*Rotary Encoder*/

void A_RISE(){
 detachInterrupt(1);
 boolean var = digitalRead(B_SIG);
 if(var==0)
 pulses++;//moving forward
 if(var==1)
 pulses--;//moving reverse

 attachInterrupt(1, A_RISE, RISING);
}

I wanted to add my rotary encoder, for starters just by showing the steps it was turned in the Serial monitor.

Result:
when I comment out displayHome() in the loop, everything works fine.

When I leave it in the loop, it prints by default 15 in the Serial monitor. When I turn the rotary encoder, I can see that the right amount of steps is added and then the in the following cyclus the value is 15 again.

I really can't imagine how this can happen. I'm not even using the variable 'pulses' in the displayHome() code....

Thank you for reading this and trying to help :slight_smile:
Bram

Your interrupt routine appears to detach itself.

aarg:
Your interrupt routine appears to detach itself.

Indeed, and I don't have any problems with that when I use the interrupt code on itself. (It attaches again in the end of the ISR)
That was added for debouncing reasons. But now my rotary encoder is hardware debounced, so I just left it out and got the same result... But thanks for the input!

I tried to run your sketch and it printed this

�����������������������������

so I // out readTime();

and it printed this which looks like a floating pin voltage

-13
-7
-5
-3
-4
-11
-18
-22
-25
-22
-14
-12
-10
-14
-20

i think your problem is with this. not sure why but if i change the 8 to 7 it prints what i would expect from a floating pin

  // request seven bytes of data from DS3231 starting from register 00h
  for (int i = 1; i < 8; i++)
  {
    time_array[i] = bcdToDec(Wire.read());
  }

way above my pay grade but i tried

byte time_array[7];

for(int i = 1; i < 8; i++)
{
   time_array[i] = bcdToDec(Wire.read()); 
}

So, you have a 7 element array, which contains elements time_array[0] through time_array[6]. You then write to elements 1 through 8 of that array, which will almost certainly clobber the variable "pulses". Nothing good will come of that....

Regards,
Ray L.

..elements 1 through 7... still a fail...

changed the 8 to a 7 then

added a 10k pulldown on pin 4 and its printing 0

gpop1:
changed the 8 to a 7 then

added a 10k pulldown on pin 4 and its printing 0

Arrays start at 0, not 1.

Thanks. The error was indeed in the array. I made adapted everything so that the array was zero indexed and this resolved all the problems.
Thank you so much. I would never have found this because I was looking for where the hell I set that value to 15.

Code for if someone would find this topic a nd be interested:

#include <Wire.h>
#include <LiquidCrystal.h>

//lcd
LiquidCrystal lcd(12, 11, 5, 6, 7, 8);

byte tempC[8] = {
  0b01100,
  0b10010,
  0b10010,
  0b01100,
  0b00000,
  0b00000,
  0b00000,
  0b00000
};
byte tempChar[8] = {
  0b00100,
  0b01010,
  0b01010,
  0b01010,
  0b01110,
  0b11111,
  0b11111,
  0b01110
};

//DS3231
#define DS3231_I2C_ADDRESS 0x68
byte time_array[7];

//Rotary encoder

#define B_SIG 4
int pulses;

void setup()
{
  Wire.begin();
  Serial.begin(9600);
  //lcd
  lcd.begin(16,2);
  lcd.createChar(0, tempC);
  lcd.createChar(1,tempChar);
  //rotary encoder
  attachInterrupt(1, A_RISE, RISING);
  pinMode(B_SIG,INPUT);
}

void loop()
{

  Serial.println(pulses);

  displayHome();
  delay(50);
}


/*DS3231*/

// Convert normal decimal numbers to binary coded decimal
byte decToBcd(byte val)
{
  return( (val/10*16) + (val%10) );
}
// Convert binary coded decimal to normal decimal numbers
byte bcdToDec(byte val)
{
  return( (val/16*10) + (val%16) );
}
void readTime()
{
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0); // set DS3231 register pointer to 00h
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS, 7);
  // request seven bytes of data from DS3231 starting from register 00h
  for(int i = 0; i < 7; i++)
  {
   time_array[i] = bcdToDec(Wire.read()); 
  }
}
  
  void displayHome()
  {  
  readTime();
  lcd.setCursor(0,0);
  lcd.print(time_array[2], DEC);
  lcd.print(":");
  if (time_array[1]<10)
  {
    lcd.print("0");
  }
  lcd.print(time_array[1], DEC);
  lcd.print(":");
  if (time_array[0]<10)
  {
    lcd.print("0");
  }
  lcd.print(time_array[0], DEC);
  lcd.setCursor(11,0);
  lcd.write(1);
  
  lcd.print(getTemp(),DEC);
  lcd.write(byte(0));
  lcd.print("C");
  lcd.setCursor(0,1);
  switch(time_array[3]){
  case 1:
    lcd.print("Zon");
    break;
  case 2:
    lcd.print("Ma");
    break;
  case 3:
    lcd.print("Di");
    break;
  case 4:
    lcd.print("Woe");
    break;
  case 5:
    lcd.print("Do");
    break;
  case 6:
    lcd.print("Vr");
    break;
  case 7:
    lcd.print("Zat");
    break;
  }
  lcd.print(" ");
  lcd.print(time_array[4], DEC);
  lcd.print("/");
  lcd.print(time_array[5], DEC);
  lcd.print("/");
  lcd.print(time_array[6], DEC);
  }
  
  int getTemp()
{
  int temp;
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0x11);
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS,2);
  delay(15);
  byte MSB = Wire.read();  

  byte LSB = Wire.read()>>6;  
  if ((MSB & 0x80) != 0)
        temp = MSB | ~((1 << 8) - 1);      // if negative get two's complement
   else
        temp = MSB;
    temp = 0.25 * LSB + temp;
    return temp;
}

/*Rotary Encoder*/

void A_RISE(){
 
 boolean var = digitalRead(B_SIG);
 if(var==0)
 pulses++;//moving forward
 if(var==1)
 pulses--;//moving reverse

}