Hi,
I have two photo interrupters that I use for wheel encoders that change logic state from low to high or vice versa for every 24th of a rotation. When I run my code which counts those interrupts, it doesn't count every pulse. If I rotate it really slowly I'll get pretty close to what it is supposed to be(+/- 1 pulse). But when I rotate the wheel at 10rot/min for 1/2 inches and manually count the number of pulses it should count(12) I get varied numbers (~8). Any Ideas? All help is appreciated!
Thanks,
luketheduke
Without seeing your code how can anyone have any idea what the problem is? Please use code tags
so the code looks like this.
A wild guess is that your interrupt code takes too long. Ideally it should complete in a number of microseconds.
...R
luketheduke:
When I run my code which counts those interrupts, it doesn't count every pulse.
If I'd make a guess: it might be the code.
Perhaps it's an error in line 42!
Or maybe another line of the sketch.
So check ALL LINES of your code thoroughly!
I am using this with ROS so there is a program running on the Arduino Mega and a python program on the ocmputer. I am keeping track of it all on github at this repository: GitHub - kk6axq/ros_arduino_bridge: ROS + Arduino = Robot. All of the arduino code is here: ros_arduino_bridge/ros_arduino_firmware/src/libraries/MegaRobogaiaPololu at hydro-devel · kk6axq/ros_arduino_bridge · GitHub. All of the enocders are handled in the .ino file, specifically at lines: 48, 49, 54, 55, 56, 57, 122 - 139, 145, 146, and 149 - 165. Could someone look at that and tell me if there is something wrong?
Thanks,
luketheduke
luketheduke:
All of the enocders are handled in the .ino file, specifically at lines: 48, 49, 54, 55, 56, 57, 122 - 139, 145, 146, and 149 - 165. Could someone look at that and tell me if there is something wrong?
If that code is for a MEGA board and not for a DUE, then the interrupts are badly attached:
attachInterrupt(encoderRightPin, encoderRightISR, CHANGE);
attachInterrupt(encoderLeftPin, encoderLeftISR, CHANGE);
For all 8-bit Atmegas you have to attach the INTERRUPT NUMBER and NOT THE PIN NUMBER.
See: http://arduino.cc/en/pmwiki.php?n=Reference/AttachInterrupt
With the MEGA you have interrupt 0 with pin-2 and interrupt 1 with pin-3, so you should attach the interrupts like that:
attachInterrupt(0, encoderRightISR, CHANGE); // encoderRightPin 2, interrupt 0
attachInterrupt(1, encoderLeftISR, CHANGE); // encoderLeftPin 3, interrupt 1
I realize that I named them wrong but the encoders are connected to sca and sdl on the mega, which I am not using for i2c and they are also interrupt pins 2 and 3. Is there anything else that could be causing the problem?
Thanks,
Luketheduke
Please make it as easy as possible for people to help you - post your code here, and make sure it is the code you are actually trying to work with.
...R
luketheduke:
Could someone look at that and tell me if there is something wrong?
Use the Force, Luke.
Robin2:
Please make it as easy as possible for people to help you - post your code here, and make sure it is the code you are actually trying to work with....R
Little problem... The code is contained in 5 different supporting files in the github repository. But here is the first half of the main code that handles the encoders:
/*********************************************************************
* ROSArduinoBridge
A set of simple serial commands to control a differential drive
robot and receive back sensor and odometry data. Default
configuration assumes use of an Arduino Mega + Pololu motor
controller shield + Robogaia Mega Encoder shield. Edit the
readEncoder() and setMotorSpeed() wrapper functions if using
different motor controller or encoder method.
Created for the Pi Robot Project: http://www.pirobot.org
Inspired and modeled after the ArbotiX driver by Michael Ferguson
Software License Agreement (BSD License)
Copyright (c) 2012, Patrick Goebel.
All rights reserved.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:
* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above
copyright notice, this list of conditions and the following
disclaimer in the documentation and/or other materials provided
with the distribution.
THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*********************************************************************/
//my code ******************************************
#include<Servo.h>
#define FORWARDS true
#define BACKWARDS false
#define encoderRightPin 2
#define encoderLeftPin 3
#define sabertoothRightPin 5
#define sabertoothLeftPin 6
#define MINSABERTOOTH 70
#define MAXSABERTOOTH 110
volatile long encoderLeft = 0;
volatile long encoderRight = 0;
volatile boolean directionLeft = FORWARDS;//true = forward false = backwards
volatile boolean directionRight = FORWARDS;//true = forward false = backwards
Servo sabertoothRightChannel;
Servo sabertoothLeftChannel;
// end of my code **************************************************
#define USE_BASE // Enable the base controller code
//#undef USE_BASE // Disable the base controller code
//#define USE_SERVOS // Enable use of PWM servos as defined in servos.h
#undef USE_SERVOS // Disable use of PWM servos
/* Serial port baud rate */
#define BAUDRATE 57600
/* Maximum PWM signal */
#define MAX_PWM 255
#if defined(ARDUINO) && ARDUINO >= 100
#include "Arduino.h"
#else
#include "WProgram.h"
#endif
/* Include definition of serial commands */
#include "commands.h"
/* Sensor functions */
#include "sensors.h"
/* Include servo support if required */
#ifdef USE_SERVOS
#include <Servo.h>
#include "servos.h"
#endif
#ifdef USE_BASE
/* The Pololu motor driver shield */
//#include "DualVNH5019MotorShield.h"
/* The Robogaia Mega Encoder shield */
//#include "MegaEncoderCounter.h"
/* Create the motor driver object */
//DualVNH5019MotorShield drive;
/* Create the encoder shield object */
//MegaEncoderCounter encoders(4); // Initializes the Mega Encoder Counter in the 4X Count mode
/* PID parameters and functions */
#include "diff_controller.h"
/* Run the PID loop at 30 times per second */
#define PID_RATE 30 // Hz
/* Convert the rate into an interval */
const int PID_INTERVAL = 1000 / PID_RATE;
/* Track the next time we make a PID calculation */
unsigned long nextPID = PID_INTERVAL;
/* Stop the robot if it hasn't received a movement command
in this number of milliseconds */
#define AUTO_STOP_INTERVAL 2000
long lastMotorCommand = AUTO_STOP_INTERVAL;
/* Wrap the encoder reading function */
long readEncoder(int i) {
if (i == LEFT) return encoderLeft;
else return encoderRight;
}
/* Wrap the encoder reset function */
void resetEncoder(int i) {
if (i == LEFT) encoderLeft = 0;
else encoderRight = 0;
}
/* Wrap the encoder reset function */
void resetEncoders() {
resetEncoder(LEFT);
resetEncoder(RIGHT);
}
/* Wrap the motor driver initialization */
void initMotorController() {
sabertoothRightChannel.attach(sabertoothRightPin, 1000, 2000);
sabertoothLeftChannel.attach(sabertoothLeftPin, 1000, 2000);
sabertoothRightChannel.write(90);
sabertoothLeftChannel.write(90);
attachInterrupt(encoderRightPin, encoderRightISR, CHANGE);
attachInterrupt(encoderLeftPin, encoderLeftISR, CHANGE);
}
//ISRs for the encoders
void encoderRightISR(){
if(directionRight == BACKWARDS){
encoderRight--;
}else{
encoderRight++;
}
}
void encoderLeftISR(){
if(directionLeft == BACKWARDS){
encoderLeft--;
}else{
encoderLeft++;
}
}
/* Wrap the drive motor set speed function */
void setMotorSpeed(int i, int spd) {
if (i == LEFT){
if(spd < 90){
directionLeft == BACKWARDS;
}else{
directionLeft == FORWARDS;
}
sabertoothLeftChannel.write(constrain(spd, MINSABERTOOTH, MAXSABERTOOTH));
}else{
if(spd < 90){
directionRight == BACKWARDS;
}else{
directionRight == FORWARDS;
}
sabertoothRightChannel.write(constrain(spd, MINSABERTOOTH, MAXSABERTOOTH));
}
}
// A convenience function for setting both motor speeds
void setMotorSpeeds(int leftSpeed, int rightSpeed) {
setMotorSpeed(LEFT, leftSpeed);
setMotorSpeed(RIGHT, rightSpeed);
}
#endif
/* Variable initialization */
// A pair of varibles to help parse serial commands (thanks Fergs)
int arg = 0;
int index = 0;
// Variable to hold an input character
char chr;
// Variable to hold the current single-character command
char cmd;
// Character arrays to hold the first and second arguments
char argv1[16];
char argv2[16];
// The arguments converted to integers
long arg1;
long arg2;
/* Clear the current command parameters */
void resetCommand() {
cmd = NULL;
memset(argv1, 0, sizeof(argv1));
memset(argv2, 0, sizeof(argv2));
arg1 = 0;
arg2 = 0;
arg = 0;
index = 0;
}
Second half:
/* Run a command. Commands are defined in commands.h */
int runCommand() {
int i = 0;
char *p = argv1;
char *str;
int pid_args[4];
arg1 = atoi(argv1);
arg2 = atoi(argv2);
switch(cmd) {
case GET_BAUDRATE:
Serial.println(BAUDRATE);
break;
case ANALOG_READ:
Serial.println(analogRead(arg1));
break;
case DIGITAL_READ:
Serial.println(digitalRead(arg1));
break;
case ANALOG_WRITE:
analogWrite(arg1, arg2);
Serial.println("OK");
break;
case DIGITAL_WRITE:
if (arg2 == 0) digitalWrite(arg1, LOW);
else if (arg2 == 1) digitalWrite(arg1, HIGH);
Serial.println("OK");
break;
case PIN_MODE:
if (arg2 == 0) pinMode(arg1, INPUT);
else if (arg2 == 1) pinMode(arg1, OUTPUT);
Serial.println("OK");
break;
case PING:
Serial.println(Ping(arg1));
break;
#ifdef USE_SERVOS
case SERVO_WRITE:
servos[arg1].write(arg2);
Serial.println("OK");
break;
case SERVO_READ:
Serial.println(servos[arg1].read());
break;
#endif
#ifdef USE_BASE
case READ_ENCODERS:
Serial.print(readEncoder(LEFT));
Serial.print(" ");
Serial.println(readEncoder(RIGHT));
break;
case RESET_ENCODERS:
resetEncoders();
Serial.println("OK");
break;
case MOTOR_SPEEDS:
/* Reset the auto stop timer */
lastMotorCommand = millis();
if (arg1 == 0 && arg2 == 0) {
setMotorSpeeds(0, 0);
moving = 0;
}
else moving = 1;
leftPID.TargetTicksPerFrame = arg1;
rightPID.TargetTicksPerFrame = arg2;
Serial.println("OK");
break;
case UPDATE_PID:
while ((str = strtok_r(p, ":", &p)) != '\0') {
pid_args[i] = atoi(str);
i++;
}
Kp = pid_args[0];
Kd = pid_args[1];
Ki = pid_args[2];
Ko = pid_args[3];
Serial.println("OK");
break;
#endif
default:
Serial.println("Invalid Command");
break;
}
}
/* Setup function--runs once at startup. */
void setup() {
Serial.begin(BAUDRATE);
// Initialize the motor controller if used */
#ifdef USE_BASE
initMotorController();
#endif
/* Attach servos if used */
#ifdef USE_SERVOS
int i;
for (i = 0; i < N_SERVOS; i++) {
servos[i].attach(servoPins[i]);
}
#endif
}
/* Enter the main loop. Read and parse input from the serial port
and run any valid commands. Run a PID calculation at the target
interval and check for auto-stop conditions.
*/
void loop() {
while (Serial.available() > 0) {
// Read the next character
chr = Serial.read();
// Terminate a command with a CR
if (chr == 13) {
if (arg == 1) argv1[index] = NULL;
else if (arg == 2) argv2[index] = NULL;
runCommand();
resetCommand();
}
// Use spaces to delimit parts of the command
else if (chr == ' ') {
// Step through the arguments
if (arg == 0) arg = 1;
else if (arg == 1) {
argv1[index] = NULL;
arg = 2;
index = 0;
}
continue;
}
else {
if (arg == 0) {
// The first arg is the single-letter command
cmd = chr;
}
else if (arg == 1) {
// Subsequent arguments can be more than one character
argv1[index] = chr;
index++;
}
else if (arg == 2) {
argv2[index] = chr;
index++;
}
}
}
// If we are using base control, run a PID calculation at the appropriate intervals
#ifdef USE_BASE
if (millis() > nextPID) {
updatePID();
nextPID += PID_INTERVAL;
}
// Check to see if we have exceeded the auto-stop interval
if ((millis() - lastMotorCommand) > AUTO_STOP_INTERVAL) {;
setMotorSpeeds(0, 0);
moving = 0;
}
#endif
}
luketheduke:
Little problem... The code is contained in 5 different supporting files in the github repository.
Post them as attachments.
Also, it is awkward when code is split between two posts. I am always concerned there may be a line missing, or repeated.
...R
Here is all of the code in a zip file.
Arduino Code.zip (5.96 KB)
luketheduke:
Here is all of the code in a zip file.
You seem determined to make it difficult for people to help you.
luketheduke:
I have two photo interrupters that I use for wheel encoders that change logic state from low to high or vice versa for every 24th of a rotation. When I run my code which counts those interrupts, it doesn't count every pulse. If I rotate it really slowly I'll get pretty close to what it is supposed to be(+/- 1 pulse). But when I rotate the wheel at 10rot/min for 1/2 inches and manually count the number of pulses
How are you doing that?
Is there some code in your program to show the count values?
The ISRs seem to be OK.
The function that reads the values from encoderLeft and -Right does not disable interrupts while it reads the value. It is 4 bytes so you can't otherwise guarantee that all 4 remain unchanged while reading them. Similarly for the encoder reset function.
It might be an idea to have the ISR set a flag when it changes the value. That way you don't have to read the value unnecessarily.
...R
If I send an 'e' down the serial monitor while the program is running it sends back the encoder values. I can see "manually" that if I rotate the wheel 1/4 rotation and there are 24 pulses per rotation that it should read 6. It does if I do it slow(most of the time, sometime it is +/-1 pulse). When I rotate it fast is when it gets off.
Robin2:
You seem determined to make it difficult for people to help you.
No I am not, it is just that the code is around 1mb so I wasn't sure if it would upload.
luketheduke:
If I send an 'e' down the serial monitor while the program is running it sends back the encoder values. I can see "manually" that if I rotate the wheel 1/4 rotation and there are 24 pulses per rotation that it should read 6. It does if I do it slow(most of the time, sometime it is +/-1 pulse). When I rotate it fast is when it gets off.
Have you considered what I said about disabling interrupts during a read ?
...R
I am clueless as how to implement that.
Very simple.
Instead of
long readEncoder(int i) {
if (i == LEFT) return encoderLeft;
else return encoderRight;
}
do
long readEncoder(int i) {
long encVal;
if (i == LEFT) {
noInterrupts();
encVal =encoderLeft;
interrupts();
}
else {
noInterrupts();
encVal = encoderRight;
interrupts();
}
return encVal;
}
...R
A hardware question: What sensor are you using to detect the wheel rotation? Have you checked with a CRO that as you speed the wheel up, you are in fact getting a full voltage transition on your Arduino PIN? If you don't get that transition, no matter how good your software is, the Arduino won't count it.
arduinodlb:
A hardware question: What sensor are you using to detect the wheel rotation?
2 Photo Interrupters scrounged from a printer.
arduinodlb:
Have you checked with a CRO that as you speed the wheel up, you are in fact getting a full voltage transition on your Arduino PIN? If you don't get that transition, no matter how good your software is, the Arduino won't count it.
No I haven't but when I have the Arduino run Serial.println(digitalRead(encoderPin)); I don't see any wavering signals when I spin it fast, eg it is 11111111110000000000 without any stray 1s or 0s such as 00000010010101011101110111111.
Hope that helps.