Hello everyone.
I'm trying to read the duty cycle of a PWM signal using an Arduino MEGA 2560 and hardware interrupts. I'm well aware of the issues that arise from using the attachInterrupt function with an ISR defined as a class member function, but reading this topic I found the way to get around it. I got my code partially working, but I have two problems: 1. The interrupts work unreliably. After flashing the Arduino with the firmware sometimes the ISR behaves as it wasn't triggered at all, keeping the duty cyle at 0 (as it is initialized). 2. When it is measured, a duty cycle (that is known a priori to be constant) keeps changing randomly and rapidly, even though the same ISR used as a regular function outside the class definition works correctly and reliably.
My question is the following: what is hidden difference that I can't see between my "object-oriented" implementation and the regular "functional" one?
I post below the codes for both implementations.
#define PWM_CHANNEL 2
int pwmDutyCycle_throttle = 0;
void setup() {
Serial.begin(115200); // Serial to PC
attachInterrupt(digitalPinToInterrupt(PWM_CHANNEL), decodePwm_1, CHANGE);
}
void loop(){
Serial.println(pwmDutyCycle_throttle);
}
void decodePwm_1() {
static unsigned long riseTime = micros(); // initialize riseTime
if (digitalRead(PWM_CHANNEL_1) == 1) { // the signal has risen
riseTime = micros(); // save the rise time
}
else { // the signal has fallen
pwmDutyCycle_throttle = micros() - riseTime; // compute the duration of the high pulse
}
}
}
And the code with the ISR defined within the class definition:
#define PWM_CHANNEL 2
class pwm_reader_attachable_t {
uint8_t pin; /**< physical pin on the board */
uint16_t pulse; /**< duration of the high edge (the pwm reading) */
uint16_t pulse_real; /**< a stabilized version of the reading (for fast varying signals) */
uint16_t edge_time; /**< timing of the last high edge */
uint8_t counter; /**< counter (actually used for the encoder) */
uint8_t read; /**< Last reading of the pin, for triggering pulse width evaluation */
public:
void
setup(pin_t pin, void (*ISR_callback)(void), int value),
handleInterrupt(void);
inline pulse_t get_pulse() { return pulse; }
pwm_reader_attachable_t() {
counter = 0;
pulse = 0;
pulse_real = 0;
edge_time = 0;
read = 0;
}
private:
void(*ISR_callback)();
};
void pwm_reader_attachable_t::setup(pin_t pin, void (*ISR_callback)(void), int value)
{
//pinMode(pin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(pin), ISR_callback, value);
interrupts();
}
inline void pwm_reader_attachable_t::handleInterrupt(void)
{
static unsigned long riseTime = micros(); // initialize riseTime
if (digitalRead(pin) == 1) { // the signal has risen
riseTime = micros(); // save the rise time
}
else { // the signal has fallen
pulse = micros() - riseTime; // compute the duration of the high pulse
}
}
pwm_reader_attachable_t* motor;
pulse_t DC=0;
void setup()
{
Serial.begin(115200);
motor = new pwm_reader_attachable_t();
motor->setup(PWM_CHANNEL, []{motor->handleInterrupt();}, CHANGE);
}
void loop()
{
DC = motor->get_pulse_real();
Serial.println(DC);
}
You need do declare your variables used inside the ISR as volatile
Code 1
#define PWM_CHANNEL 2
volatile int pwmDutyCycle_throttle = 0;
void setup() {
Serial.begin(115200); // Serial to PC
attachInterrupt(digitalPinToInterrupt(PWM_CHANNEL), decodePwm_1, CHANGE);
}
void loop() {
int localValue;
noInterrupts();
localValue = pwmDutyCycle_throttle;
interrupts();
Serial.println(pwmDutyCycle_throttle);
}
void decodePwm_1() {
static unsigned long riseTime = micros(); // initialize riseTime
if (digitalRead(PWM_CHANNEL_1) == 1) { // the signal has risen
riseTime = micros(); // save the rise time
}
else { // the signal has fallen
pwmDutyCycle_throttle = micros() - riseTime; // compute the duration of the high pulse
}
}
Code 2 (with some fixes)
#define PWM_CHANNEL 2
typedef byte pin_t;
typedef uint16_t pulse_t;
class pwm_reader_attachable_t {
uint8_t pin; /**< physical pin on the board */
volatile uint16_t pulse; /**< duration of the high edge (the pwm reading) */
uint16_t pulse_real; /**< a stabilized version of the reading (for fast varying signals) */
uint16_t edge_time; /**< timing of the last high edge */
uint8_t counter; /**< counter (actually used for the encoder) */
uint8_t read; /**< Last reading of the pin, for triggering pulse width evaluation */
public:
void setup(pin_t pin, void (*ISR_callback)(void), int value);
void handleInterrupt(void);
inline pulse_t get_pulse() {
pulse_t x;
noInterrupts();
x = pulse;
interrupts();
return x;
};
pwm_reader_attachable_t(void) {
counter = 0;
pulse = 0;
pulse_real = 0;
edge_time = 0;
read = 0;
}
private:
void(*ISR_callback)();
};
void pwm_reader_attachable_t::setup(pin_t pin, void (*ISR_callback)(void), int value)
{
//pinMode(pin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(pin), ISR_callback, value);
interrupts();
}
inline void pwm_reader_attachable_t::handleInterrupt(void)
{
static unsigned long riseTime = micros(); // initialize riseTime
if (digitalRead(pin) == 1) { // the signal has risen
riseTime = micros(); // save the rise time
}
else { // the signal has fallen
pulse = micros() - riseTime; // compute the duration of the high pulse
}
}
pwm_reader_attachable_t* motor;
pulse_t DC = 0;
void setup()
{
Serial.begin(115200);
motor = new pwm_reader_attachable_t();
motor->setup(PWM_CHANNEL, [] {motor->handleInterrupt();}, CHANGE);
//motor->setup(PWM_CHANNEL, motor->handleInterrupt, CHANGE);
}
void loop()
{
DC = motor->get_pulse();
Serial.println(DC);
}
blh64:
You need do declare your variables used inside the ISR as volatile
Thanks for the tip about volatile variables (arrrgh I should read the documentation of the functions I use) !
Still, I get inconsistent values between Code 1 and Code 2: Code 1 works fine, as expected. Code 2 returns random duty cycle values, far from the expected one and rapidly changing over time.