From b1dc338564199969b50297a12434ad766fc52306 Mon Sep 17 00:00:00 2001 From: Javier Balloffet Date: Tue, 6 Feb 2024 12:05:00 -0300 Subject: [PATCH] Modify Encoder class API to allow dependency injection (#213) Signed-off-by: Javier Balloffet --- andino_firmware/src/app.cpp | 16 ++++++++++++---- andino_firmware/src/app.h | 9 ++++++++- andino_firmware/src/encoder.cpp | 14 +++++++------- andino_firmware/src/encoder.h | 25 +++++++++++++------------ 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/andino_firmware/src/app.cpp b/andino_firmware/src/app.cpp index c23d8ebb..86700174 100644 --- a/andino_firmware/src/app.cpp +++ b/andino_firmware/src/app.cpp @@ -71,6 +71,7 @@ #include "digital_out_arduino.h" #include "encoder.h" #include "hw.h" +#include "interrupt_in_arduino.h" #include "motor.h" #include "pid.h" #include "pwm_out_arduino.h" @@ -92,8 +93,15 @@ PwmOutArduino App::right_motor_backward_pwm_out_(Hw::kRightMotorBackwardGpioPin) Motor App::right_motor_(&right_motor_enable_digital_out_, &right_motor_forward_pwm_out_, &right_motor_backward_pwm_out_); -Encoder App::left_encoder_(Hw::kLeftEncoderChannelAGpioPin, Hw::kLeftEncoderChannelBGpioPin); -Encoder App::right_encoder_(Hw::kRightEncoderChannelAGpioPin, Hw::kRightEncoderChannelBGpioPin); +InterruptInArduino App::left_encoder_channel_a_interrupt_in_(Hw::kLeftEncoderChannelAGpioPin); +InterruptInArduino App::left_encoder_channel_b_interrupt_in_(Hw::kLeftEncoderChannelBGpioPin); +Encoder App::left_encoder_(&left_encoder_channel_a_interrupt_in_, + &left_encoder_channel_b_interrupt_in_); + +InterruptInArduino App::right_encoder_channel_a_interrupt_in_(Hw::kRightEncoderChannelAGpioPin); +InterruptInArduino App::right_encoder_channel_b_interrupt_in_(Hw::kRightEncoderChannelBGpioPin); +Encoder App::right_encoder_(&right_encoder_channel_a_interrupt_in_, + &right_encoder_channel_b_interrupt_in_); PID App::left_pid_controller_(Constants::kPidKp, Constants::kPidKd, Constants::kPidKi, Constants::kPidKo, -Constants::kPwmMax, Constants::kPwmMax); @@ -110,8 +118,8 @@ void App::setup() { Serial.begin(Constants::kBaudrate); - left_encoder_.init(); - right_encoder_.init(); + left_encoder_.begin(); + right_encoder_.begin(); left_motor_.begin(); left_motor_.enable(true); diff --git a/andino_firmware/src/app.h b/andino_firmware/src/app.h index dbe07795..062b9872 100644 --- a/andino_firmware/src/app.h +++ b/andino_firmware/src/app.h @@ -31,6 +31,7 @@ #include "digital_out_arduino.h" #include "encoder.h" +#include "interrupt_in_arduino.h" #include "motor.h" #include "pid.h" #include "pwm_out_arduino.h" @@ -96,8 +97,14 @@ class App { static PwmOutArduino right_motor_backward_pwm_out_; static Motor right_motor_; - /// Encoders (one per wheel). + /// Left wheel encoder. + static InterruptInArduino left_encoder_channel_a_interrupt_in_; + static InterruptInArduino left_encoder_channel_b_interrupt_in_; static Encoder left_encoder_; + + /// Right wheel encoder. + static InterruptInArduino right_encoder_channel_a_interrupt_in_; + static InterruptInArduino right_encoder_channel_b_interrupt_in_; static Encoder right_encoder_; /// PID controllers (one per wheel). diff --git a/andino_firmware/src/encoder.cpp b/andino_firmware/src/encoder.cpp index 2866a6e3..8ddd8a87 100644 --- a/andino_firmware/src/encoder.cpp +++ b/andino_firmware/src/encoder.cpp @@ -66,7 +66,7 @@ #include -#include "interrupt_in_arduino.h" +#include "interrupt_in.h" namespace andino { @@ -89,17 +89,17 @@ void Encoder::callback_1() { Encoder* Encoder::instances_[kInstancesMax] = {nullptr, nullptr}; int Encoder::instance_count_ = 0; -void Encoder::init() { +void Encoder::begin() { // The current implementation only supports two instances of this class to be constructed. This // prevents reaching a buffer overflow. if (instance_count_ == kInstancesMax) { return; } - channel_a_interrupt_in_.begin(); - channel_a_interrupt_in_.attach(kCallbacks[instance_count_]); - channel_b_interrupt_in_.begin(); - channel_b_interrupt_in_.attach(kCallbacks[instance_count_]); + channel_a_interrupt_in_->begin(); + channel_a_interrupt_in_->attach(kCallbacks[instance_count_]); + channel_b_interrupt_in_->begin(); + channel_b_interrupt_in_->attach(kCallbacks[instance_count_]); instances_[instance_count_] = this; instance_count_++; @@ -112,7 +112,7 @@ void Encoder::reset() { count_ = 0L; } void Encoder::callback() { // Read the current channels state into the lowest 2 bits of the encoder state. state_ <<= 2; - state_ |= (channel_b_interrupt_in_.read() << 1) | channel_a_interrupt_in_.read(); + state_ |= (channel_b_interrupt_in_->read() << 1) | channel_a_interrupt_in_->read(); // Update the encoder count accordingly. count_ += kTicksDelta[(state_ & 0x0F)]; diff --git a/andino_firmware/src/encoder.h b/andino_firmware/src/encoder.h index f8457953..e3c6b627 100644 --- a/andino_firmware/src/encoder.h +++ b/andino_firmware/src/encoder.h @@ -66,7 +66,7 @@ #include -#include "interrupt_in_arduino.h" +#include "interrupt_in.h" namespace andino { @@ -77,15 +77,16 @@ class Encoder { public: /// @brief Constructs a new Encoder object. /// - /// @param a_gpio_pin Encoder channel A GPIO pin. - /// @param b_gpio_pin Encoder channel B GPIO pin. - Encoder(int a_gpio_pin, int b_gpio_pin) - : channel_a_interrupt_in_(a_gpio_pin), channel_b_interrupt_in_(b_gpio_pin) {} + /// @param channel_a_interrupt_in Digital interrupt input connected to encoder channel A pin. + /// @param channel_b_interrupt_in Digital interrupt input connected to encoder channel B pin. + Encoder(const InterruptIn* channel_a_interrupt_in, const InterruptIn* channel_b_interrupt_in) + : channel_a_interrupt_in_(channel_a_interrupt_in), + channel_b_interrupt_in_(channel_b_interrupt_in) {} /// @brief Initializes the encoder. - void init(); + void begin(); - /// @brief Returns the ticks count value. + /// @brief Gets the ticks count value. /// /// @return Ticks count value. long read(); @@ -130,7 +131,7 @@ class Encoder { /// Static wrapper that redirects to the second instance callback method. static void callback_1(); - /// Channels GPIO interrupt callback. + /// Channels interrupt callback. void callback(); /// Holds references to the constructed Encoder instances. @@ -139,11 +140,11 @@ class Encoder { /// Number of constructed Encoder instances. static int instance_count_; - /// Interrupt input connected to encoder channel A pin. - InterruptInArduino channel_a_interrupt_in_; + /// Digital interrupt input connected to encoder channel A pin. + const InterruptIn* channel_a_interrupt_in_; - /// Interrupt input connected to encoder channel B pin. - InterruptInArduino channel_b_interrupt_in_; + /// Digital interrupt input connected to encoder channel B pin. + const InterruptIn* channel_b_interrupt_in_; /// Encoder state. It contains both the current and previous channels state readings: /// +------+-----+-----+-----+-----+-----+-----+-----+-----+