Rotary Encoders with interrupts inside a switch matrix circuit

Other Hardware LEDs and Multiplexing
1

Hello, I am using an Arduino Micro with ATmega 32U4 and have a problem in my project.

I am trying to implement a switch-matrix style system, which will process inputs from 10 rotary encoders. It is using the interrupt approach which works on its own, but fails when the switch-matrix aspect is added.

The A pins of the 10 encoders are all connected together to pin 7.
Similarly, B pins of encoders are connected to pin 3
Both pins 3 and 7 support interrupts on the 32U4, and are normally high with internal pull-up resistors.

Then the common pins of each encoder are each connected to a column of the switch matrix, and these columns are cycled by the program through a decade counter and inverters (active column is low), while keeping track which column is active at the moment.

This way, when the encoder is turned and triggers the ISR, the ISR will look at which column is active and will be able to determine which encoder the input is coming from. Please take a look at the rough schematic I prepared

schematic_
schematic_1027×725 81.1 KB

The encoder I am using is 24 PPR and 24 detents
I know I can use a library such as encoder.h but i really wanted to do it manually and learn.

Here is how I catch the input in the interrupt:
The encoders output between 2 detents when turning clockwise:
A: 1 0 0 1 1
B: 1 1 0 0 1

and the following between 2 detents when turning anti-clockwise :
A: 1 1 0 0 1
B: 1 0 0 1 1

When the A line changes state from 1 to 0 (ISR triggered by a fall) and the B line is still high, its clockwise. Similarly when the B line changes state from 1 to 0 (ISR triggered by a fall) and the A line is still high, its anti-clockwise. So its only looking at the first section of detent cycle but seems to work.

So basically in the main program loop I have a for-loop that loops variable i from 0 to 9. This variable represents the active column and is global, so that it can be accessed by the ISRs to determine which encoder was used.

Inside this loop, I have another for-loop that is very big just to waste time and simulate the system doing something (something other than operating the encoders). After the loop is complete, I send a signal to the decade counter to move onto the next column.

The problem is that when I have an encoder connected to column 0 and use it, the column reported by the ISR is 9 (previous one) almost all the time, rarely i get the correct 0, and even sometimes its 10, which shouldn't be possible since the for-loop for the columns is for(i=0; i<10; i++)

Similarly when the encoder is connected to column 1, the reported column is 0 and so on, they all are -1 of what they should be.

Any idea what I am doing wrong here?

Thanks !

//global variable accessable by ISRs 
int i = 0;

//pin used to send signal to decade counter to move to next column
int drive_line_clk = 2;

//reset pin for the decade counter
int reset_pin = 13;

//encoder outputs 
int encoder_A         = 7; //supports interrupt
int encoder_B         = 3; //supports interrupt

//time in ms needed between interrupts 
int bounce_time = 10;

void setup() 
{

  Serial.begin(9600);

  //declare outputs
  pinMode(drive_line_clk, OUTPUT);
  pinMode(reset_pin,      OUTPUT);
  digitalWrite(reset_pin, LOW);

  //encoder sense lines are normally high inputs
  pinMode(encoder_A,        INPUT_PULLUP);
  pinMode(encoder_B,        INPUT_PULLUP);

  //attach interrupts to the encoders
  attachInterrupt(digitalPinToInterrupt(encoder_A), interrupt_function_A, FALLING);
  attachInterrupt(digitalPinToInterrupt(encoder_B), interrupt_function_B, FALLING);
  
  //reset the count of the decade counter when all startup is done
  delay(10);
  digitalWrite(reset_pin,  HIGH);
  delay(10);
  digitalWrite(reset_pin,  LOW);
}

void loop() 
{
  //global variable i represents the active column 
  for(i=0; i<10; i++) 
  {
    //this loop is simulating the system being busy to test interrupts
    for(long a=0; a<1000000000; a++){}

    //when done move to next column
    clock_key_matrix();
  }
}

void clock_key_matrix()
{
    digitalWrite(drive_line_clk,  HIGH);
    digitalWrite(drive_line_clk,  LOW);
}

void interrupt_function_A() 
{
  //first read what the current column is
  int column = i;
  static unsigned int count = 0;
  
  static unsigned long lastInterruptTime = 0;
  unsigned long interruptTime = millis();
  
  // If interrupts come faster than bounce_time, assume it's a bounce and ignore
  if (interruptTime - lastInterruptTime > bounce_time) 
  { 
    if (digitalRead(encoder_B) == HIGH)
    {     
      count ++;     
      Serial.println( (String)"R "+ count + " " + column);
    }
  }
  
  // Keep track of when we were here last (no more than every 5ms)
  lastInterruptTime = interruptTime;
}

void interrupt_function_B() 
{
  //first read what the current column is
  int column = i;
  static unsigned int count = 0;
  
  static unsigned long lastInterruptTime = 0;
  unsigned long interruptTime = millis();
  
  // If interrupts come faster than bounce_time, assume it's a bounce and ignore
  if (interruptTime - lastInterruptTime > bounce_time) 
  { 
    if (digitalRead(encoder_A) == HIGH)
    {     
      count ++;     
      Serial.println( (String)"L "+ count + " " + column);
    }
  }
  
  // Keep track of when we were here last (no more than every 5ms)
  lastInterruptTime = interruptTime;
}
2
  1. Using interrupts.
  2. Local variables in interrupt routines.
  3. Printing within interrupt routines.
  4. ...

What are you doing right?

  1. Posting code
  2. Reading the instructions for posting first. :+1:
3

This is inherently flawed. If your decade counter switches to the next encoder and it's in a different position than the previous one, then this will trigger the read erroneously.

I'm working on a similar issue and my solution was to use an ATTiny88. I read 4 encoders on one port constantly and save its value. When there is a change I raise an interrupt pin to the main MCU which then reads the value of the port from the 88.

You could use an ESP32 to read this many encoders fast and report changes to your main MCU.

4

Why is that a bad thing? I thought interrupts are the way to go, especially with rotary encoders.

What can i do in my case to avoid this bad practice?

This is for debugging only

The encoders I am using are 1 full pulse per detent, so the outputs are the same state at every detent. This means that when the set of encoders is not touched, they are all in the same state and when the decade counter switches through them, no interrupt should trigger. Only the one actually used triggers the interrupt. Or at least that's how i understand it.

5

Do you have a datasheet for the encoders?

6

I agree with @er_name_not_found this idea just won't work. Chances are almost every interrupt will be caused by the code scanning through the multiplexer channels. Most movements of the encoders will be missed because the multiplexer is not looking at the correct channel at that instant.

So how to fix it? How would I connect 10 encoders to an Arduino micro? I've never done it, so I can only suggest ideas. Using attiny chips has already been suggested, as has esp32 (although using such a powerful chip only to pass the data to a less powerful "main" chip seems odd!)

What about 3x pcf8574 chips? Micro has 5 external interrupt pins, so 3 of them could be connected to the interrupt pins of the pcf chips, and of course all 3 pcf chips would be connected to SDA & SCL pins and each chip assigned a different address using the A0/1/2 pins on the chip. 4 encoders would be attached to each chip, except one which would only have 2 attached.

The interrupt routines would do nothing more than set a global flag variable to indicate that one or more inputs changed on that chip. The main code in loop() would check these flags and read the data from the appropriate chip, then figure out which encoder had moved and in which direction.

Over to @Paul_B & @er_name_not_found to shoot holes in my idea!

7

Idea #2: use a Arduino Zero-compatible board. These have 20+ I/o pins and all but one can be used as an external interrupt, plus, like atmega32u4, the chip can act as a HID device. An example board would be

8

i needs to be declared volatile.

    for(long a=0; a<1000000000; a++){}

The for statement will be completely optimized away by the compiler, because it does nothing.

Printing in an ISR is possible, but not a good idea at all. If the output buffer if full, Serial.println will wait for space in the buffer before returning, keeping the code in the ISR for an extended amount of time.

I agree there are problems with your approach. Because the encoders are being scanned, any encoder that has a LOW on either of its switches will generate an interrupt every time the scan line reaches that encoder, UNLESS the encoder in the previous scan position also has a LOW, in which case no interrupt would be generated. In the worst case, if all encoders are in a LOW position, an interrupt would never be generated.

9

It depends.

It depends on whether the encoder is moved by hand, or on a fast-moving machine shaft.

And it depends on what else the code has to do.

10

Hi Guys, thanks for everyone's input so far.

The Encoders that I am using are Bourns PEC11R-4225F-S0024
Data sheet: here

Perhaps I should have mentioned it at the start, these are to be turned by hand - so no crazy speeds. Also only 1 encoder will be turned at any one time.

The data sheet states that on every detent, both outputs are off, meaning they are not connected to anything. That would kind of back up my theory that no false interrupts would be triggered as the columns are scanned while the encoders are not used, because they would all report the same state.

Basically pins 3 and 7 (encoder outputs) have internal pull-up resistors so when not in use (i.e. at detent positions) the encoders all read HIGH as the channels are scanned, and only when they are turned, they read LOW at some point and thus trigger the interrupt with the FALLING condition. At this point in time, I need to determine which channel is active to know which encoder is used.

I actually can confirm that this is working, I currently have 6 encoders hooked up to various channels out of the 10 planned, and they all seem to be registering fine, not missing any detents, recognising turning left and right, reporting different channels, not triggering erroneous interrupts.

The problem is that encoder on line 0 reports line 9, encoder on line 1 reports line 0, encoder on line 2 reports line 1 etc... they all seem to be 1 behind

Thanks, I have changed that already, unfortunately nothing improved.

Good point, what else can I do to simulate busy clock cycles? delay() seems to break it.

Its moved by hand, only one at a time, no fast speeds. Imagine a set of dials for a control panel on a radio or something, that is the type of application here. The encoders will be inputs that will be used to adjust parameters.

The program will also read states of a few other toggle switches, but that is not time critical.

11

Imagine encoder one is in position 1/4 and encoder two is in position 3/4.
They are not moving, just sitting still.
As soon as your decade counter increments from one to two the values on the interrupt pins will change and the MCU will read the value as new, even though nothing as moved.
This setup will not work.

12

I do agree with you that things would go wrong if one encoder was in position 1/4 and another was in position 3/4 as you scan through the channels. This would cause triggering interrupts in error and what not.

But this won't happen because the encoders I am using have detents, and from one detent to another, they are put through their full pulse cycle and then are put back to the same position as at the beginning.

So if they are left alone (at their detents) they will be all in the same position and all should be good.

clockwise:

        A: 1 0 0 1 1
        B: 1 1 0 0 1
Detent:    ^       ^

anti-clockwise:

         A: 1 1 0 0 1
         B: 1 0 0 1 1
Detent:     ^       ^

You say this setup will not work, but I do have it working for the most part, right in front of me.

The problem I have now is each encoder is reporting the wrong channel, which is consistently the one ahead of what it should be and I am wondering why that is.

13

Those statements seem to be opposite to each other. When you are turning the encoder on channel 4, for example, do you get 3 or 5?

One lower than the channel number I can understand. clock_key_matrix() is executed inside the for loop, so when it switches the channel the interrupt will get triggered immediately, before i can be incremented to match the current channel number.

Forget about using the for loop, instead temporarily disable interrupts, call clock_key_matrix() to advance to the next channel, increment i, then re-enable interrupts.

14 
  for(i=0; i<10; i++) 
  {
    //this loop is simulating the system being busy to test interrupts
    for(long a=0; a<1000000000; a++){}

    //when done move to next column
    clock_key_matrix();
  }

I think @Paul_B is correct to say that the "busy" loop will be completely removed by the compiler. I've seen for myself how "clever" it can appear to be when optimising. I guess the compiler can see that this loop has no side effects, so has no effect on the wider program, so can be removed.

As I said earlier, the interrupts will be caused by the clocking of the encoder, so will happen immediately after clock_key_matrix() is called and before i gets incremented, the opposite to what you intended.

15

I have a encoder library which is able to use mulitplexed encoders here:

It is written for Teensies and XIAO and won't work for your board without changes but it contains some proven hardware examples using HC165, HC4067 and HC4051 to multiplex the encoders. info and schematics here:

https://github.com/luni64/EncoderTool/tree/master/extras

In case you are interested, here the relevant code for the 4067:
https://github.dev/luni64/EncoderTool/blob/master/src/Multiplexed/EncPlex4067.h#L41

16

Sorry my mistake, I should have said it is consistently one channel behind.
When I turn encoder on channel 4, channel 3 is reported.

Thanks guys for the clarification, triggering the interrupt after switching channel but before incrementing i makes sense. It happens at the wrong time.

I modified my code to do this instead:

void loop() 
{  
  //when done move to next column
  clock_key_matrix();
}

void clock_key_matrix()
{
  //turn off interrupts
  noInterrupts(); 

  //adjust channel number to next one
  if( i == 9 )
  {
    i = 0;       
  }
  else
  {
    i++;
  }

  //move to next channel
  digitalWrite(drive_line_clk,  HIGH);
  digitalWrite(drive_line_clk,  LOW);
  
  //turn interrupts back on when done
  interrupts(); 
}

So now it is reporting the correct channel 90% of the time, but sometimes I get one ahead instead.
e.g. encoder 4 is turned and I get lots of reports for channel 4 but an odd one here and there for channel 5.

Now it looks like this time around, i is incremented before the channel actually changes?
Like turning off the interrupts doesn't fully work all the time?

@luni64 thanks for that, I will take a good look at it!

17

Eh? Not me, it was David! :astonished:

18

Probably because you are spending so much time in the code with interrupts disabled that an interrupt is getting queued up after interrupts are disabled, but before you have time to actually change channels. Now that I think about it, instead of disabling/re-enabling interrupts, it would be better to disable all scan lines (channels) while incrementing and advancing the channel, so that no interrupt can occur or get queued up with an incorrect value of i. That would require that the chip you are using to do the channel scanning have an output disable (all outputs high or tri-state) ability.

19

Oops! Sorry @david_2018 @Paul_B

20

Or not use interrupts. (Now I think that was suggested by @Paul_B !)

Using interrupts here makes no sense. They will be triggered by the scanning, not the movement of the encoders, in most cases. They are not, and don't need to be, asynchronous events. I think it would be better to poll pins 3 & 7 with digitalRead() and compare those readings with the previous readings taken when the multiplexer last visited that same encoder.