From 3e313f9e5f964440639e37a2ab8ff241fe7f0ee1 Mon Sep 17 00:00:00 2001
From: Alexandre Bourdiol <alexandre.bourdiol@st.com>
Date: Tue, 1 Dec 2020 09:17:06 +0100
Subject: [PATCH] HardwareTimer: Fix assert failed when using
 TIMER_OUTPUT_COMPARE

When assert is activated there may be assert failed, specially
when using Tone or Servo with TIM6 or TIM7.
"assert_param(IS_TIM_CC1_INSTANCE(htim->Instance));"
This is due to the fact that when using timer instances without
output (like TIM6 and TIM7 specially used for Tone and Servo)
in TIMER_OUTPUT_COMPARE mode, the API setMode() requires a channel,
even if it is not used.
This was made like this to simplify the HardwareTimer driver,
and there is no functional issue, but as there is an assert failed
reported when assert is activated, this should be fixed.

TIMER_OUTPUT_COMPARE becomes obsolete, but kept for compatibility
reason.
When only timing configuration is needed, no need to set mode,
just keep the default TIMER_DISABLED.

Fixes #1244

Signed-off-by: Alexandre Bourdiol <alexandre.bourdiol@st.com>
---
 cores/arduino/HardwareTimer.cpp     | 21 ++++++++++++++-------
 cores/arduino/HardwareTimer.h       |  4 ++--
 cores/arduino/Tone.cpp              |  1 -
 libraries/Servo/src/stm32/Servo.cpp |  1 -
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp
index d26e8342b8..3df3d12f0e 100644
--- a/cores/arduino/HardwareTimer.cpp
+++ b/cores/arduino/HardwareTimer.cpp
@@ -371,7 +371,6 @@ void HardwareTimer::resumeChannel(uint32_t channel)
         }
       }
       break;
-    case TIMER_OUTPUT_COMPARE:
     case TIMER_OUTPUT_COMPARE_ACTIVE:
     case TIMER_OUTPUT_COMPARE_INACTIVE:
     case TIMER_OUTPUT_COMPARE_TOGGLE:
@@ -406,6 +405,7 @@ void HardwareTimer::resumeChannel(uint32_t channel)
       }
       break;
     case TIMER_NOT_USED:
+    case TIMER_OUTPUT_COMPARE:
     default :
       break;
   }
@@ -598,9 +598,6 @@ void HardwareTimer::setMode(uint32_t channel, TimerModes_t mode, PinName pin)
     Error_Handler();
   }
 
-  // Save channel selected mode to object attribute
-  _ChannelMode[channel - 1] = mode;
-
   /* Configure some default values. Maybe overwritten later */
   channelOC.OCMode = TIMER_NOT_USED;
   channelOC.Pulse = __HAL_TIM_GET_COMPARE(&(_timerObj.handle), timChannel);  // keep same value already written in hardware <register
@@ -626,9 +623,16 @@ void HardwareTimer::setMode(uint32_t channel, TimerModes_t mode, PinName pin)
       HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &channelOC, timChannel);
       break;
     case TIMER_OUTPUT_COMPARE:
-      channelOC.OCMode = TIM_OCMODE_TIMING;
-      HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &channelOC, timChannel);
-      break;
+      /* In case of TIMER_OUTPUT_COMPARE, there is no output and thus no pin to
+       * configure, and no channel. So nothing to do. For compatibility reason
+       * restore TIMER_DISABLED if necessary.
+       */
+      if (_ChannelMode[channel - 1] != TIMER_DISABLED) {
+        _ChannelMode[channel - 1] = TIMER_DISABLED;
+        channelOC.OCMode = TIM_OCMODE_TIMING;
+        HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &channelOC, timChannel);
+      }
+      return;
     case TIMER_OUTPUT_COMPARE_ACTIVE:
       channelOC.OCMode = TIM_OCMODE_ACTIVE;
       HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &channelOC, timChannel);
@@ -688,6 +692,9 @@ void HardwareTimer::setMode(uint32_t channel, TimerModes_t mode, PinName pin)
       break;
   }
 
+  // Save channel selected mode to object attribute
+  _ChannelMode[channel - 1] = mode;
+
   if (pin != NC) {
     if ((int)get_pwm_channel(pin) == timChannel) {
       /* Configure PWM GPIO pins */
diff --git a/cores/arduino/HardwareTimer.h b/cores/arduino/HardwareTimer.h
index 1a3ee871f8..2bbbc68b21 100644
--- a/cores/arduino/HardwareTimer.h
+++ b/cores/arduino/HardwareTimer.h
@@ -36,9 +36,9 @@
 #define  TIMER_CHANNELS 4    // channel5 and channel 6 are not considered here has they don't have gpio output and they don't have interrupt
 
 typedef enum {
-  TIMER_DISABLED,
+  TIMER_DISABLED,                         // == TIM_OCMODE_TIMING           no output, useful for only-interrupt
   // Output Compare
-  TIMER_OUTPUT_COMPARE,                   // == TIM_OCMODE_TIMING           no output, useful for only-interrupt
+  TIMER_OUTPUT_COMPARE,                   // == Obsolete, use TIMER_DISABLED instead. Kept for compatibility reason
   TIMER_OUTPUT_COMPARE_ACTIVE,            // == TIM_OCMODE_ACTIVE           pin is set high when counter == channel compare
   TIMER_OUTPUT_COMPARE_INACTIVE,          // == TIM_OCMODE_INACTIVE         pin is set low when counter == channel compare
   TIMER_OUTPUT_COMPARE_TOGGLE,            // == TIM_OCMODE_TOGGLE           pin toggles when counter == channel compare
diff --git a/cores/arduino/Tone.cpp b/cores/arduino/Tone.cpp
index 7c475e50f9..b4ffd635a0 100644
--- a/cores/arduino/Tone.cpp
+++ b/cores/arduino/Tone.cpp
@@ -95,7 +95,6 @@ static void timerTonePinInit(PinName p, uint32_t frequency, uint32_t duration)
 
       pin_function(TimerTone_pinInfo.pin, STM_PIN_DATA(STM_MODE_OUTPUT_PP, GPIO_NOPULL, 0));
 
-      TimerTone->setMode(1, TIMER_OUTPUT_COMPARE, NC);
       TimerTone->setOverflow(timFreq, HERTZ_FORMAT);
       TimerTone->attachInterrupt(tonePeriodElapsedCallback);
       TimerTone->resume();
diff --git a/libraries/Servo/src/stm32/Servo.cpp b/libraries/Servo/src/stm32/Servo.cpp
index 41c77c7584..850ca74f54 100644
--- a/libraries/Servo/src/stm32/Servo.cpp
+++ b/libraries/Servo/src/stm32/Servo.cpp
@@ -80,7 +80,6 @@ static void TimerServoInit()
   // prescaler is computed so that timer tick correspond to 1 microseconde
   uint32_t prescaler = TimerServo.getTimerClkFreq() / 1000000;
 
-  TimerServo.setMode(1, TIMER_OUTPUT_COMPARE, NC);
   TimerServo.setPrescaleFactor(prescaler);
   TimerServo.setOverflow(REFRESH_INTERVAL); // thanks to prescaler Tick = microsec
   TimerServo.attachInterrupt(Servo_PeriodElapsedCallback);