Hi,
i'm attempting to do what I thought was a simple project to read an encoder (Omron 360P/R) 1440 total per rev and display it on a TM1637 Ic2 display. I use two interrupts to get the data from the encoder, adjust the count so it only goes to 360.xx and rolls over. To help test I send the data to the serial monitor which works great!! I get 0.00 to 360.00 or 360.00 to 0.00 depending on direction for a full rotation. Perfect I thought However when I send the data to the TM1637 via I2C it appears to loose lots of pulses, getting maybe 70-90 per revolution. The encoder is turned by hand. Ive tried a Nano and a Teensy 4.0 also different TM1637 display libaries. I'm kinda assuming the I2C is too slow and causing the issue. I also tried only updating the display only after xx loop times, same issue.
My question is am I going about this the wrong way ?, should I be using a different type display ? Im at a loss and to what direction to go.
Thanks for any input
//
// working 360 per rotation up or down with .25 res
// omron encoder 360 p/r give 1/4 degree increments
// 2k pullups on omron pins
// 1-17-24
// teensy 4.0 and ardunio Nano same results
// TM1637, TM1637TinyDisplay, and LEDDisplayDriver libaries same result
//
//#include <Arduino.h>
//#include <TM1637Display.h>
#include "LEDDisplayDriver.h"
//
const byte encoderPinA = 2;//outoutB digital pin2
const byte encoderPinB = 3;//outoutB digital pin3
volatile int count = 0;
int displayloop = 0;
int protectedCount = 0;
int previousCount = 0;
int degreeCount = 0;
int tempCount = 0;
float dial = 0;
int displaycount = 0;
#define readA bitRead(PIND,2)//faster than digitalRead() // encoder
#define readB bitRead(PIND,3)//faster than digitalRead() // encoder
const byte numberOfDigits = 4;
const byte dataPin = 10;
const byte clockPin = 11;
const byte loadPin = 9;
LEDDisplayDriver display(dataPin, clockPin, loadPin, true, numberOfDigits);
//
//
//
void setup() {
Serial.begin (115200);
attachInterrupt(digitalPinToInterrupt(encoderPinA), isrA, CHANGE);
attachInterrupt(digitalPinToInterrupt(encoderPinB), isrB, CHANGE);
pinMode(encoderPinA, INPUT_PULLUP);
pinMode(encoderPinB, INPUT_PULLUP);
display.showNum(0);
}
void loop() {
noInterrupts();
protectedCount = count;
interrupts();
if(protectedCount != previousCount) {
if (protectedCount >= previousCount) {
tempCount = tempCount +1;
if (tempCount == 1440){
tempCount = 0;
}
}
if (protectedCount <= previousCount) {
tempCount = tempCount -1;
if (tempCount == 0){
tempCount = 1440;
}
}
tempCount = abs(tempCount);
dial = (tempCount*.25);
Serial.println(dial);
previousCount = protectedCount;
}
displaycount++;
if (displaycount == 100){
display.showNum(dial); //works with this line commented out but of course no display
displaycount =0;
}
}
void isrA() {
if(readB != readA) {
count ++;
} else {
count --;
}
}
void isrB() {
if (readA == readB) {
count ++;
} else {
count --;
}
}
I didnt understand theTM1637 is not I2C and has its own protocol. I did try using different pins 4,5 then 10,11 with same result.
I'll try not using interrupts instead the arduino encoder libary next and report back. initially i used digitalRead() on the interrupts and missed pulses if i gave the encoder a quick spin by hand.
At this point in the code, couldn't protectedCount be 2, 3, 4... greater than previousCount? But tempCount is still only increased by 1. Won't that make it look as though interrupts are being missed, when perhaps they are not?
Why have count and tempCount as separate counters? Why not use just one count variable?
So count is transfered to protectedCount so the interrupts are not trying to change it while it's being checked.
The thing is that as long as " display.showNum(dial); " is commented out, the output via Serial.println(dial); is correct. You can spin the dial back and forth without any loss. I put a hard stop on the encoder so i can see zero. works perfect. uncomment " display.showNum(dial); " and its way off.
I changed so that i'm using the arduino encoder library and not interrrupts (I think) and I get the exact same results. as long as " display.showNum(dial); " is commented out, the output via Serial.println(dial); is correct.
//
// working 360 per rotation up an down .25 res
// omron encoder 360 p/r give 1/4 degree increments
// 2k pullups on omron pins
// 1-11-24
// teensy 4.0 and ardunio Nano same results
// TM1637, TM1637TinyDisplay, and LEDDisplayDriver libaries same result
//
//#include <Arduino.h>
//#include <TM1637Display.h>
#include <Encoder.h>
#include "LEDDisplayDriver.h"
//
//
Encoder encodinput(20, 21);
volatile int count = 0;
int displayloop = 0;
int protectedCount = 0;
int previousCount = 0;
int degreeCount = 0;
int tempCount = 0;
float dial = 0;
int displaycount = 0;
const byte numberOfDigits = 4;
const byte dataPin = 10;
const byte clockPin = 11;
const byte loadPin = 9;
LEDDisplayDriver display(dataPin, clockPin, loadPin, true, numberOfDigits);
//
//
//
void setup() {
Serial.begin (115200);
display.showNum(3);
Serial.println("Encoder Test:");
}
void loop() {
//noInterrupts();
protectedCount = encodinput.read();
//interrupts();
if(protectedCount != previousCount) {
if (protectedCount >= previousCount) {
tempCount = tempCount +1;
if (tempCount == 1440){
tempCount = 0;
}
}
if (protectedCount <= previousCount) {
tempCount = tempCount -1;
if (tempCount == 0){
tempCount = 1440;
}
}
tempCount = abs(tempCount);
dial = (tempCount*.25);
Serial.println(dial);
previousCount = protectedCount;
}
displaycount++;
if (displaycount == 100){
display.showNum(dial);
displaycount =0;
}
//display.showNum(dial);
}
Try printing protectedCount and tempCount to serial as well as dial. Do they stay in step when you spin the dial with the display.showNum() commented or uncommented? Or does the difference between protectedCount and tempCount increase?
I looked at you original code, and since i am just experimenting with encoders myself, thought i'd share my experience.
I used PinChange interrupts and got it working after a few goes. First of all the encoders that i have don't have any capacitors on them, so added those myself and instead of using the internal pullups i used 10K pullups next to the encoder.
Then as a major difference to your approach i only enable the interrupt for output A and compared state of A & B if A was actually 'falling'
This is the snippet, i connected A & B to pins 3 & 4 ( i do want them on the same port)
volatile int8_t val = 0;
void setup() {
pinMode(3, INPUT);
pinMode(4, INPUT);
Serial.begin(115200);
Serial.println("PCINT test started.");
PCICR |= B100; // enable PCI on PORTD
PCMSK2 |= 1 << 3; // on PD3
}
void loop() {
static int8_t oldVal = 0;
if (oldVal != val) {
Serial.print("New Value = ");
Serial.println(val, DEC);
oldVal = val;
}
}
ISR (PCINT2_vect) {
uint8_t in = PIND; // read the register as soon as the ISR is fired
uint8_t in1 = in & 24; // mask the pins that are relevant
if (in1 == 16) { // if B is HIGH and A is LOW
val++;
}
else if (in1 == 0) { // if both are LOW
val--;
}
}
Got this working with multiple encoders (on the same port, but i guess it could be on a different port as well)
The encoders have buttons and for that i used a slightly different solution with a 50ms debounce
I thought so too! But it almost is... I've used the display before, working well, but based on the OPs question I decided to look closer at the library.
The protocol that is used, looks awfully much like bit-banged I2C, with pins being used in open collector mode: switching between INPUT and OUTPUT modes, i.e. floating (relying on an external pull-up), or driven LOW. The function names in the library even refer to I2C. This makes me wonder whether it'd be possible to use the I2C hardware of the Arduino (ATmega328P) to drive this display. Could be more efficient if there's no other I2C device in use.
But that's not what I was looking at, really, it was just an interesting observation. I was looking at interrupts (they're not used - also not disabled). So interrupts for the encoder should continue as normal. I don't see why OP would be missing interrupts due to this library.
What I was looking for, was the speed at which data is sent. That's pretty slow, there's a 2x 100us delay per bit sent, so 200 us per bit, or 5000 kbps, not counting other code overhead meaning the real rate is even less. Each update of the display is three bytes plus one byte per segment, 7 bytes, 56 bits, 11.2 ms - a bit more due to overhead. This can be done much faster as the display accepts up to 250 kHz.
OP's encoder has 360*4 = 1440 pulses per rotation. Hand rotating fast can easily do a rotation in less than a second, so about 2 kHz pulses. If not using interrupts that is going to be a major problem in combination with these slow screen updates, that's lots of pulses in 11.2 ms.
Taking PaulRBs advice I sent the protectedCount and tempCount to the serial port and found that protectedCount was often increasing/decreasing by more than 1 But I was always only increasing/decreasing tempCount by 1 each change. I just checked how much it changed and added/subtracted that from tempCount and it is working now.
Once I got in left field thinking it was a display issue because it showed up when I sent data to the display. I never looked back !!!
Deva_Rishi, thanks for sharing your code very helpful.
wvmarle, great info on the TM1637, and for confiriming the need for interrupts.
Thanks, everyone else as well. Once I started down the rabbit hole I couldn't force my self back out without the help!! He's the functioning code, needs some clean up but is working.
Cheers !!
//
// working 360 per rotation up an down .25 res
// omron encoder 360 p/r give 1/4 degree increments
// 2k pullups on omron pins
// 1-18-24
// teensy 4.0 and ardunio Nano same results
// TM1637, TM1637TinyDisplay, and LEDDisplayDriver libaries same result
//
//#include <Arduino.h>
//#include <TM1637Display.h>
#include "LEDDisplayDriver.h"
//
const byte encoderPinA = 2;//outoutB digital pin2
const byte encoderPinB = 3;//outoutB digital pin3
volatile int count = 0;
int displayloop = 0;
int protectedCount = 0;
int previousCount = 0;
int degreeCount = 0;
int tempCount = 0;
float dial = 0;
int displaycount = 0;
#define readA bitRead(PIND,2)//faster than digitalRead() // encoder
#define readB bitRead(PIND,3)//faster than digitalRead() // encoder
const byte numberOfDigits = 4;
const byte dataPin = 10;
const byte clockPin = 11;
const byte loadPin = 9;
LEDDisplayDriver display(dataPin, clockPin, loadPin, true, numberOfDigits);
//
//
//
void setup() {
Serial.begin (115200);
attachInterrupt(digitalPinToInterrupt(encoderPinA), isrA, CHANGE);
attachInterrupt(digitalPinToInterrupt(encoderPinB), isrB, CHANGE);
pinMode(encoderPinA, INPUT_PULLUP);
pinMode(encoderPinB, INPUT_PULLUP);
display.showNum(0);
}
void loop() {
noInterrupts();
protectedCount = count;
interrupts();
if(protectedCount != previousCount) {
if (protectedCount >= previousCount) {
tempCount = (tempCount +(protectedCount - previousCount));
if (tempCount >= 1440){
tempCount = 0;
}
}
if (protectedCount <= previousCount) {
tempCount = (tempCount - (previousCount - protectedCount));
if (tempCount <= 0){
tempCount = 1440;
}
}
tempCount = abs(tempCount);
dial = (tempCount*.25);
previousCount = protectedCount;
}
displaycount++;
if (displaycount == 200){
display.showNum(dial);
displaycount =0;
}
}
void isrA() {
if(readB != readA) {
count ++;
} else {
count --;
}
}
void isrB() {
if (readA == readB) {
count ++;
} else {
count --;
}
}
Thanks PaulRB, No , it loses counts. appears to be ok if the encoder is turned very slowly but a couple of direction changes and it loses a few each time.
Only if a second interrupt comes in on the same pin, before it's been processed. Otherwise there should be no issue as interrupts simply get queued and are handled as soon as possible.
Well yes, but that does depend on the total time required to process everything. If both ISR's combined take more time than there is between requests you will start missing counts when the excess interval time is up (eg ISR takes 10 clicks * 2 = 20 clicks, requests come in every 9 clicks or so, after about 10 requests you will start missing counts)
The calling of an ISR takes time as well, and i guess in this case
void isrB() {
if (readA == readB) {
if (++count >= 1440) count = 0;
} else {
if (--count < 0) count = 1399;
}
}
the extra compare is what is slowing down the ISR, as well as the use 16-bit variables of course. On an 8-bit processor 16-bit is significantly slower than 8-bit. So i figure
That is 32-bit isn't it ? that means you can leave interrupts enabled
As test, I commented out one of thr ISRs on PualRBs code and changed roll over numbers to reflect 720 p/r instead of 1440. I is working with no loss of count.
not sure what that tells, less IRQs but also less code being executed