From 2d4c6e3481003f2fa21d467618622e320f4b732c Mon Sep 17 00:00:00 2001
From: Emilio Corigliano <emilio.corigliano@skywarder.eu>
Date: Fri, 9 Jun 2023 09:55:31 +0000
Subject: [PATCH] [PWM][GeneralPurposeTimer] Small refactoring

---
 .../drivers/timer/GeneralPurposeTimer.h       | 18 ++--
 src/shared/drivers/timer/PWM.cpp              | 82 +++++++++----------
 src/shared/drivers/timer/PWM.h                | 26 +++---
 3 files changed, 61 insertions(+), 65 deletions(-)

diff --git a/src/shared/drivers/timer/GeneralPurposeTimer.h b/src/shared/drivers/timer/GeneralPurposeTimer.h
index de2256237..3f30bb23e 100644
--- a/src/shared/drivers/timer/GeneralPurposeTimer.h
+++ b/src/shared/drivers/timer/GeneralPurposeTimer.h
@@ -186,9 +186,9 @@ public:
      */
     void disableCaptureCompareComplementaryOutput(TimerUtils::Channel channel);
 
-    bool isCaptureComapreOutputEnabled(TimerUtils::Channel channel);
+    bool isCaptureCompareOutputEnabled(TimerUtils::Channel channel);
 
-    bool isCaptureComapreComplementaryOutputEnabled(
+    bool isCaptureCompareComplementaryOutputEnabled(
         TimerUtils::Channel channel);
 
     void setCaptureComparePolarity(TimerUtils::Channel channel,
@@ -202,10 +202,9 @@ public:
 
     T readCaptureCompareRegister(TimerUtils::Channel channel);
 
-    static void clearTriggerInterruptFlag(TIM_TypeDef *timer);
+    void clearTriggerInterruptFlag();
 
-    static void clearCaptureCompareInterruptFlag(TimerUtils::Channel channel,
-                                                 TIM_TypeDef *timer);
+    void clearCaptureCompareInterruptFlag(TimerUtils::Channel channel);
 };
 
 /**
@@ -500,14 +499,14 @@ inline void GeneralPurposeTimer<T>::disableCaptureCompareComplementaryOutput(
 }
 
 template <typename T>
-inline bool GeneralPurposeTimer<T>::isCaptureComapreOutputEnabled(
+inline bool GeneralPurposeTimer<T>::isCaptureCompareOutputEnabled(
     TimerUtils::Channel channel)
 {
     return timer->CCER & (TIM_CCER_CC1E << (static_cast<int>(channel) * 4));
 }
 
 template <typename T>
-inline bool GeneralPurposeTimer<T>::isCaptureComapreComplementaryOutputEnabled(
+inline bool GeneralPurposeTimer<T>::isCaptureCompareComplementaryOutputEnabled(
     TimerUtils::Channel channel)
 {
     return timer->CCER & (TIM_CCER_CC1NE << (static_cast<int>(channel) * 4));
@@ -570,15 +569,14 @@ inline T GeneralPurposeTimer<T>::readCaptureCompareRegister(
 }
 
 template <typename T>
-inline void GeneralPurposeTimer<T>::clearTriggerInterruptFlag(
-    TIM_TypeDef *timer)
+inline void GeneralPurposeTimer<T>::clearTriggerInterruptFlag()
 {
     timer->SR &= ~TIM_SR_TIF;
 }
 
 template <typename T>
 inline void GeneralPurposeTimer<T>::clearCaptureCompareInterruptFlag(
-    TimerUtils::Channel channel, TIM_TypeDef *timer)
+    TimerUtils::Channel channel)
 {
     timer->SR &= ~(TIM_SR_CC1IF << static_cast<int>(channel));
 }
diff --git a/src/shared/drivers/timer/PWM.cpp b/src/shared/drivers/timer/PWM.cpp
index e8b156b96..27a3ba0fa 100644
--- a/src/shared/drivers/timer/PWM.cpp
+++ b/src/shared/drivers/timer/PWM.cpp
@@ -22,88 +22,86 @@
 
 #include "PWM.h"
 
+using namespace Boardcore::TimerUtils;
+
 namespace Boardcore
 {
 
-PWM::PWM(TIM_TypeDef* const timer, unsigned int pwmFrequency,
-         unsigned int dutyCycleResolution)
-    : timer(timer), pwmFrequency(pwmFrequency),
-      dutyCycleResolution(dutyCycleResolution)
+PWM::PWM(TIM_TypeDef* const pulseTimer, uint16_t pulseFrequency)
+    : pulseTimer(pulseTimer), pulseFrequency(pulseFrequency)
 {
     // Erase the previous timer configuration
-    this->timer.reset();
+    this->pulseTimer.reset();
+
+    configureTimer();
 
-    // Set up and enable the timer
-    setTimerConfiguration();
+    // Keep the timer always enabled
+    this->pulseTimer.enable();
 }
 
-PWM::~PWM() { timer.reset(); }
+PWM::~PWM() { pulseTimer.reset(); }
 
-void PWM::setFrequency(unsigned int pwmFrequency)
+void PWM::setFrequency(uint16_t pulseFrequency)
 {
-    this->pwmFrequency = pwmFrequency;
-    setTimerConfiguration();
+    this->pulseFrequency = pulseFrequency;
+    configureTimer();
 }
 
-void PWM::setDutyCycleResolution(unsigned int dutyCycleResolution)
+void PWM::setDutyCycleResolution(uint16_t dutyCycleResolution)
 {
     this->dutyCycleResolution = dutyCycleResolution;
-    setTimerConfiguration();
+    configureTimer();
 }
 
-void PWM::enableChannel(TimerUtils::Channel channel, Polarity polarity)
+void PWM::enableChannel(Channel channel, Polarity polarity)
 {
-    timer.setOutputCompareMode(channel,
-                               TimerUtils::OutputCompareMode::PWM_MODE_1);
-    timer.setCaptureComparePolarity(
-        channel, static_cast<TimerUtils::OutputComparePolarity>(polarity));
+    pulseTimer.setOutputCompareMode(channel, OutputCompareMode::PWM_MODE_1);
+    pulseTimer.setCaptureComparePolarity(
+        channel, static_cast<OutputComparePolarity>(polarity));
 
     // This will ensure that the duty cycle will change only at the next period
-    timer.enableCaptureComparePreload(channel);
+    pulseTimer.enableCaptureComparePreload(channel);
 
-    timer.enableCaptureCompareOutput(channel);
+    pulseTimer.enableCaptureCompareOutput(channel);
 }
 
-void PWM::disableChannel(TimerUtils::Channel channel)
+void PWM::disableChannel(Channel channel)
 {
-    timer.disableCaptureCompareOutput(channel);
+    pulseTimer.disableCaptureCompareOutput(channel);
 }
 
-bool PWM::isChannelEnabled(TimerUtils::Channel channel)
+bool PWM::isChannelEnabled(Channel channel)
 {
-    return timer.isCaptureComapreOutputEnabled(channel);
+    return pulseTimer.isCaptureCompareOutputEnabled(channel);
 }
 
-void PWM::setDutyCycle(TimerUtils::Channel channel, float dutyCycle)
+void PWM::setDutyCycle(Channel channel, float dutyCycle)
 {
     if (dutyCycle >= 0 && dutyCycle <= 1)
-        timer.setCaptureCompareRegister(
-            channel, static_cast<uint16_t>(
-                         dutyCycle * timer.readAutoReloadRegister() + 0.5));
+    {
+        pulseTimer.setCaptureCompareRegister(
+            channel,
+            static_cast<uint16_t>(
+                dutyCycle * pulseTimer.readAutoReloadRegister() + 0.5));
+    }
 }
 
 float PWM::getDutyCycle(TimerUtils::Channel channel)
 {
-    return static_cast<float>(timer.readCaptureCompareRegister(channel)) /
-           static_cast<float>(timer.readAutoReloadRegister());
+    return static_cast<float>(pulseTimer.readCaptureCompareRegister(channel)) /
+           static_cast<float>(pulseTimer.readAutoReloadRegister());
 }
 
-GP16bitTimer& PWM::getTimer() { return timer; }
+GP16bitTimer& PWM::getTimer() { return pulseTimer; }
 
-void PWM::setTimerConfiguration()
+void PWM::configureTimer()
 {
-    timer.disable();
-    timer.setCounter(0);
-
-    timer.setFrequency(dutyCycleResolution * pwmFrequency);
-
-    timer.setAutoReloadRegister(TimerUtils::getFrequency(timer.getTimer()) /
-                                pwmFrequency);
+    pulseTimer.setFrequency(dutyCycleResolution * pulseFrequency);
+    pulseTimer.setAutoReloadRegister(getFrequency(pulseTimer.getTimer()) /
+                                     pulseFrequency);
 
     // Force the timer to update its configuration
-    timer.generateUpdate();
-
-    timer.enable();
+    pulseTimer.generateUpdate();
 }
 
 }  // namespace Boardcore
diff --git a/src/shared/drivers/timer/PWM.h b/src/shared/drivers/timer/PWM.h
index 085be48f6..84d3d8127 100644
--- a/src/shared/drivers/timer/PWM.h
+++ b/src/shared/drivers/timer/PWM.h
@@ -60,18 +60,17 @@ public:
     /**
      * @brief Sets up and enables the PWM timer.
      *
-     * @param timer Pointer to the timer's peripheral registers.
-     * @param pwmFrequency Frequency of the PWM signal.
-     * @param dutyCycleResolution Duty cycle levels.
+     * @param pulseTimer Timer used for the generation of the PWM signal.
+     * @param pulseFrequency Frequency of the PWM signal.
+
      */
-    explicit PWM(TIM_TypeDef* const timer, unsigned int pwmFrequency = 50,
-                 unsigned int dutyCycleResolution = 20000);
+    explicit PWM(TIM_TypeDef* const pulseTimer, uint16_t pulseFrequency = 50);
 
     ~PWM();
 
-    void setFrequency(unsigned int pwmFrequency);
+    void setFrequency(uint16_t pulseFrequency);
 
-    void setDutyCycleResolution(unsigned int dutyCycleResolution);
+    void setDutyCycleResolution(uint16_t dutyCycleResolution);
 
     void enableChannel(TimerUtils::Channel channel,
                        Polarity polarity = Polarity::NORMAL);
@@ -85,8 +84,8 @@ public:
      *
      * Changes the duty cycle only if the specified value is in the range [0,1].
      *
-     * @param channel Channel to change the duty cycle
-     * @param dutyCycle Relative duty cycle, ranges from 0 to 1
+     * @param channel Channel to change the duty cycle.
+     * @param dutyCycle Relative duty cycle, ranges from 0 to 1.
      */
     void setDutyCycle(TimerUtils::Channel channel, float dutyCycle);
 
@@ -105,11 +104,12 @@ private:
     PWM& operator=(const PWM&) = delete;
     PWM(const PWM& p)          = delete;
 
-    void setTimerConfiguration();
+    void configureTimer();
+
+    GP16bitTimer pulseTimer;
 
-    GP16bitTimer timer;
-    unsigned int pwmFrequency;
-    unsigned int dutyCycleResolution;
+    uint16_t pulseFrequency;
+    uint16_t dutyCycleResolution = 10000;
 };
 
 }  // namespace Boardcore
-- 
GitLab