From e2096d0d693d860d65ca506857c2686ecf40e226 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 11:10:39 +0100 Subject: [PATCH 01/12] Add methods to check if there's an IRQ handler --- cores/arduino/HardwareTimer.cpp | 19 +++++++++++++++++++ cores/arduino/HardwareTimer.h | 2 ++ 2 files changed, 21 insertions(+) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 081ad5dae4..83cc5b822f 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -701,6 +701,25 @@ void HardwareTimer::detachInterrupt(uint32_t channel) callbacks[channel] = NULL; } +/** + * @brief Checks if there's an interrupt callback attached on Rollover event + * @retval returns true if a timer rollover interrupt has already been set + */ +bool HardwareTimer::hasInterrupt() +{ + return callbacks[0] != NULL; +} + +/** + * @brief Checks if there's an interrupt callback attached on Capture/Compare event + * @param channel: Arduino channel [1..4] + * @retval returns true if a channel compare match interrupt has already been set + */ +bool HardwareTimer::hasInterrupt(uint32_t channel); +{ + return callbacks[channel] != NULL; +} + /** * @brief Generate an update event to force all registers (Autoreload, prescaler, compare) to be taken into account * @note Refresh() can only be called after a 1st call to resume() to be sure timer is initialised. diff --git a/cores/arduino/HardwareTimer.h b/cores/arduino/HardwareTimer.h index ce3e0cc1ac..dca655032a 100644 --- a/cores/arduino/HardwareTimer.h +++ b/cores/arduino/HardwareTimer.h @@ -110,9 +110,11 @@ class HardwareTimer { //Add interrupt to period update void attachInterrupt(void (*handler)(HardwareTimer *)); // Attach interrupt callback which will be called upon update event (timer rollover) void detachInterrupt(); // remove interrupt callback which was attached to update event + bool hasInterrupt(); //returns true if a timer rollover interrupt has already been set //Add interrupt to capture/compare channel void attachInterrupt(uint32_t channel, void (*handler)(HardwareTimer *)); // Attach interrupt callback which will be called upon compare match event of specified channel void detachInterrupt(uint32_t channel); // remove interrupt callback which was attached to compare match event of specified channel + bool hasInterrupt(uint32_t channel); //returns true if an interrupt has already been set on the channel compare match void timerHandleDeinit(); // Timer deinitialization From 8f8357286cefdb33a0e933f577b63e07bb03d38b Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 12:51:57 +0100 Subject: [PATCH 02/12] validate user input --- cores/arduino/HardwareTimer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 83cc5b822f..9a2469ac15 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -715,8 +715,11 @@ bool HardwareTimer::hasInterrupt() * @param channel: Arduino channel [1..4] * @retval returns true if a channel compare match interrupt has already been set */ -bool HardwareTimer::hasInterrupt(uint32_t channel); +bool HardwareTimer::hasInterrupt(uint32_t channel) { + if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { + Error_Handler(); // only channel 1..4 have an interrupt + } return callbacks[channel] != NULL; } From 623e17ebecb3d677c8e9757707e9a499cc46fabe Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 17:33:57 +0100 Subject: [PATCH 03/12] Only activate ISR to HardwareTimer when needed. This lets the user start the timer without a callback and add it after. Moreover, now it is faster after detachInterrupt because the ISR is actually stopped and no context switching is made. This is good in performance critical code. --- cores/arduino/HardwareTimer.cpp | 116 ++++++++++++++++++++------------ 1 file changed, 73 insertions(+), 43 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 9a2469ac15..b3e730d3bf 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -179,57 +179,29 @@ void HardwareTimer::resumeChannel(uint32_t channel) if (IS_TIM_PWM_MODE(_channelOC[channel - 1].OCMode)) { HAL_TIM_PWM_ConfigChannel(&(_timerObj.handle), &_channelOC[channel - 1], timChannel); - if ((channel < (TIMER_CHANNELS + 1)) && (callbacks[channel] != NULL)) { - // Only channel 1..4 can have interruption #if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_PWMN_Start_IT(&(_timerObj.handle), timChannel); - } else -#endif - { - HAL_TIM_PWM_Start_IT(&(_timerObj.handle), timChannel); - } - } else { -#if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_PWMN_Start(&(_timerObj.handle), timChannel); - } else + if (isComplementaryChannel[channel]) { + HAL_TIMEx_PWMN_Start(&(_timerObj.handle), timChannel); + } else #endif - { - HAL_TIM_PWM_Start(&(_timerObj.handle), timChannel); - } + { + HAL_TIM_PWM_Start(&(_timerObj.handle), timChannel); } } else if (IS_TIM_OC_MODE(_channelOC[channel - 1].OCMode)) { HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &_channelOC[channel - 1], timChannel); - if ((channel < (TIMER_CHANNELS + 1)) && (callbacks[channel] != NULL)) { - // Only channel 1..4 can have interruption -#if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_OCN_Start_IT(&(_timerObj.handle), timChannel); - } else -#endif - { - HAL_TIM_OC_Start_IT(&(_timerObj.handle), timChannel); - } - } else { #if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_OCN_Start(&(_timerObj.handle), timChannel); - } else + if (isComplementaryChannel[channel]) { + HAL_TIMEx_OCN_Start(&(_timerObj.handle), timChannel); + } else #endif - { - HAL_TIM_OC_Start(&(_timerObj.handle), timChannel); - } + { + HAL_TIM_OC_Start(&(_timerObj.handle), timChannel); } } else if (_channelIC[channel - 1].ICPolarity != TIMER_NOT_USED) { HAL_TIM_IC_ConfigChannel(&(_timerObj.handle), &_channelIC[channel - 1], timChannel); - if (callbacks[channel] != NULL) { - HAL_TIM_IC_Start_IT(&(_timerObj.handle), timChannel); - } else { - HAL_TIM_IC_Start(&(_timerObj.handle), timChannel); - } + HAL_TIM_IC_Start(&(_timerObj.handle), timChannel); } } @@ -660,7 +632,10 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - callbacks[0] = callback; + if (callbacks[0] == null) //if there's no callback the ISR here is not enabled + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); + + callbacks[0] = callback; //set or replace user callback } /** @@ -669,6 +644,11 @@ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) */ void HardwareTimer::detachInterrupt() { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); //disables the interrupt call to save cpu cycles for useless context switching + //we don't need memory barriers, as specified here + //( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the ) + //since, even if the ISR here could be called again it will not call back in the user-space because we clear the user callback below. + //so actually if the user wants to detach the interrupt and there are pending calls the user doesn't get notified anymore. callbacks[0] = NULL; } @@ -680,11 +660,38 @@ void HardwareTimer::detachInterrupt() */ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareTimer *)) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { + if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { Error_Handler(); // only channel 1..4 have an interrupt } + bool mustActivateISR = callbacks[channel] == null; //no user callback means ISR is disabled + callbacks[channel] = callback; + + if (mustActivateISR) + switch (channel) + { + case 1: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt + break; + } + case 2: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Enable the TIM Capture/Compare 2 interrupt + break; + } + case 3: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Enable the TIM Capture/Compare 3 interrupt + break; + } + case 4: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Enable the TIM Capture/Compare 4 interrupt + break; + } + } } /** @@ -694,10 +701,33 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT */ void HardwareTimer::detachInterrupt(uint32_t channel) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { - Error_Handler(); // only channel 1..4 have an interrupt + switch (channel) + { + case 1: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt + break; + } + case 2: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Disable the TIM Capture/Compare 2 interrupt + break; + } + case 3: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Disable the TIM Capture/Compare 3 interrupt + break; + } + case 4: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Disable the TIM Capture/Compare 4 interrupt + break; + } + default: + Error_Handler(); // only channel 1..4 have an interrupt } + //look at the detachInterrupt() for notes callbacks[channel] = NULL; } From 215d64b063c4e42c50f283f5e3088596d4040544 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 20:53:08 +0100 Subject: [PATCH 04/12] Proper code formatting --- cores/arduino/HardwareTimer.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index b3e730d3bf..dacf20ec82 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -632,9 +632,10 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - if (callbacks[0] == null) //if there's no callback the ISR here is not enabled + if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); - + } + callbacks[0] = callback; //set or replace user callback } @@ -660,7 +661,7 @@ void HardwareTimer::detachInterrupt() */ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareTimer *)) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { + if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { Error_Handler(); // only channel 1..4 have an interrupt } @@ -668,9 +669,8 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT callbacks[channel] = callback; - if (mustActivateISR) - switch (channel) - { + if (mustActivateISR) { + switch (channel) { case 1: { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt @@ -692,6 +692,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT break; } } + } } /** @@ -701,8 +702,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT */ void HardwareTimer::detachInterrupt(uint32_t channel) { - switch (channel) - { + switch (channel) { case 1: { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt From 7a3d117126c1ef6cd4a96406e31feb28fc89826f Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 20:56:51 +0100 Subject: [PATCH 05/12] Case statement formatting to pass travis --- cores/arduino/HardwareTimer.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index dacf20ec82..ce1962cecc 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -672,25 +672,17 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT if (mustActivateISR) { switch (channel) { case 1: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt break; - } case 2: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Enable the TIM Capture/Compare 2 interrupt break; - } case 3: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Enable the TIM Capture/Compare 3 interrupt break; - } case 4: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Enable the TIM Capture/Compare 4 interrupt break; - } } } } @@ -704,25 +696,17 @@ void HardwareTimer::detachInterrupt(uint32_t channel) { switch (channel) { case 1: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt break; - } case 2: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Disable the TIM Capture/Compare 2 interrupt break; - } case 3: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Disable the TIM Capture/Compare 3 interrupt break; - } case 4: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Disable the TIM Capture/Compare 4 interrupt break; - } default: Error_Handler(); // only channel 1..4 have an interrupt } From aa0ddd961ab996acc9d7a0a03db1ccc898cf81a5 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 21:02:24 +0100 Subject: [PATCH 06/12] Spaces on an empty line -.-' --- cores/arduino/HardwareTimer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index ce1962cecc..508730f12d 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -635,7 +635,7 @@ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); } - + callbacks[0] = callback; //set or replace user callback } From 166a9c33b7c7628a40f63356cd6895029578821d Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 21:23:12 +0100 Subject: [PATCH 07/12] Uppercase NULL --- cores/arduino/HardwareTimer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 508730f12d..8bc53c9ced 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -632,7 +632,7 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled + if (callbacks[0] == NULL) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); } @@ -665,7 +665,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT Error_Handler(); // only channel 1..4 have an interrupt } - bool mustActivateISR = callbacks[channel] == null; //no user callback means ISR is disabled + bool mustActivateISR = (callbacks[channel] == NULL); //no user callback means ISR is disabled callbacks[channel] = callback; From 51b7947a8a55e97eef9593a544ed7fd1ea226dce Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 17:33:57 +0100 Subject: [PATCH 08/12] Only activate ISR to HardwareTimer when needed. This lets the user start the timer without a callback and add it after. Moreover, now it is faster after detachInterrupt because the ISR is actually stopped and no context switching is made. This is good in performance critical code. --- cores/arduino/HardwareTimer.cpp | 116 ++++++++++++++++++++------------ 1 file changed, 73 insertions(+), 43 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 9a2469ac15..b3e730d3bf 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -179,57 +179,29 @@ void HardwareTimer::resumeChannel(uint32_t channel) if (IS_TIM_PWM_MODE(_channelOC[channel - 1].OCMode)) { HAL_TIM_PWM_ConfigChannel(&(_timerObj.handle), &_channelOC[channel - 1], timChannel); - if ((channel < (TIMER_CHANNELS + 1)) && (callbacks[channel] != NULL)) { - // Only channel 1..4 can have interruption #if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_PWMN_Start_IT(&(_timerObj.handle), timChannel); - } else -#endif - { - HAL_TIM_PWM_Start_IT(&(_timerObj.handle), timChannel); - } - } else { -#if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_PWMN_Start(&(_timerObj.handle), timChannel); - } else + if (isComplementaryChannel[channel]) { + HAL_TIMEx_PWMN_Start(&(_timerObj.handle), timChannel); + } else #endif - { - HAL_TIM_PWM_Start(&(_timerObj.handle), timChannel); - } + { + HAL_TIM_PWM_Start(&(_timerObj.handle), timChannel); } } else if (IS_TIM_OC_MODE(_channelOC[channel - 1].OCMode)) { HAL_TIM_OC_ConfigChannel(&(_timerObj.handle), &_channelOC[channel - 1], timChannel); - if ((channel < (TIMER_CHANNELS + 1)) && (callbacks[channel] != NULL)) { - // Only channel 1..4 can have interruption -#if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_OCN_Start_IT(&(_timerObj.handle), timChannel); - } else -#endif - { - HAL_TIM_OC_Start_IT(&(_timerObj.handle), timChannel); - } - } else { #if defined(TIM_CCER_CC1NE) - if (isComplementaryChannel[channel]) { - HAL_TIMEx_OCN_Start(&(_timerObj.handle), timChannel); - } else + if (isComplementaryChannel[channel]) { + HAL_TIMEx_OCN_Start(&(_timerObj.handle), timChannel); + } else #endif - { - HAL_TIM_OC_Start(&(_timerObj.handle), timChannel); - } + { + HAL_TIM_OC_Start(&(_timerObj.handle), timChannel); } } else if (_channelIC[channel - 1].ICPolarity != TIMER_NOT_USED) { HAL_TIM_IC_ConfigChannel(&(_timerObj.handle), &_channelIC[channel - 1], timChannel); - if (callbacks[channel] != NULL) { - HAL_TIM_IC_Start_IT(&(_timerObj.handle), timChannel); - } else { - HAL_TIM_IC_Start(&(_timerObj.handle), timChannel); - } + HAL_TIM_IC_Start(&(_timerObj.handle), timChannel); } } @@ -660,7 +632,10 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - callbacks[0] = callback; + if (callbacks[0] == null) //if there's no callback the ISR here is not enabled + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); + + callbacks[0] = callback; //set or replace user callback } /** @@ -669,6 +644,11 @@ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) */ void HardwareTimer::detachInterrupt() { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); //disables the interrupt call to save cpu cycles for useless context switching + //we don't need memory barriers, as specified here + //( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the ) + //since, even if the ISR here could be called again it will not call back in the user-space because we clear the user callback below. + //so actually if the user wants to detach the interrupt and there are pending calls the user doesn't get notified anymore. callbacks[0] = NULL; } @@ -680,11 +660,38 @@ void HardwareTimer::detachInterrupt() */ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareTimer *)) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { + if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { Error_Handler(); // only channel 1..4 have an interrupt } + bool mustActivateISR = callbacks[channel] == null; //no user callback means ISR is disabled + callbacks[channel] = callback; + + if (mustActivateISR) + switch (channel) + { + case 1: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt + break; + } + case 2: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Enable the TIM Capture/Compare 2 interrupt + break; + } + case 3: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Enable the TIM Capture/Compare 3 interrupt + break; + } + case 4: + { + __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Enable the TIM Capture/Compare 4 interrupt + break; + } + } } /** @@ -694,10 +701,33 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT */ void HardwareTimer::detachInterrupt(uint32_t channel) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { - Error_Handler(); // only channel 1..4 have an interrupt + switch (channel) + { + case 1: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt + break; + } + case 2: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Disable the TIM Capture/Compare 2 interrupt + break; + } + case 3: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Disable the TIM Capture/Compare 3 interrupt + break; + } + case 4: + { + __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Disable the TIM Capture/Compare 4 interrupt + break; + } + default: + Error_Handler(); // only channel 1..4 have an interrupt } + //look at the detachInterrupt() for notes callbacks[channel] = NULL; } From 348e10681059dfc985c0786dc4844068a1cd181c Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 20:53:08 +0100 Subject: [PATCH 09/12] Proper code formatting --- cores/arduino/HardwareTimer.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index b3e730d3bf..dacf20ec82 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -632,9 +632,10 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - if (callbacks[0] == null) //if there's no callback the ISR here is not enabled + if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); - + } + callbacks[0] = callback; //set or replace user callback } @@ -660,7 +661,7 @@ void HardwareTimer::detachInterrupt() */ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareTimer *)) { - if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { + if ((channel == 0) || (channel > (TIMER_CHANNELS + 1))) { Error_Handler(); // only channel 1..4 have an interrupt } @@ -668,9 +669,8 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT callbacks[channel] = callback; - if (mustActivateISR) - switch (channel) - { + if (mustActivateISR) { + switch (channel) { case 1: { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt @@ -692,6 +692,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT break; } } + } } /** @@ -701,8 +702,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT */ void HardwareTimer::detachInterrupt(uint32_t channel) { - switch (channel) - { + switch (channel) { case 1: { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt From b9cf8a6e42c2dcf82ddec448ea0aeaaea18633e2 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 20:56:51 +0100 Subject: [PATCH 10/12] Case statement formatting to pass travis --- cores/arduino/HardwareTimer.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index dacf20ec82..ce1962cecc 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -672,25 +672,17 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT if (mustActivateISR) { switch (channel) { case 1: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Enable the TIM Capture/Compare 1 interrupt break; - } case 2: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Enable the TIM Capture/Compare 2 interrupt break; - } case 3: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Enable the TIM Capture/Compare 3 interrupt break; - } case 4: - { __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Enable the TIM Capture/Compare 4 interrupt break; - } } } } @@ -704,25 +696,17 @@ void HardwareTimer::detachInterrupt(uint32_t channel) { switch (channel) { case 1: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC1); // Disable the TIM Capture/Compare 1 interrupt break; - } case 2: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC2); // Disable the TIM Capture/Compare 2 interrupt break; - } case 3: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC3); // Disable the TIM Capture/Compare 3 interrupt break; - } case 4: - { __HAL_TIM_DISABLE_IT(&(_timerObj.handle), TIM_IT_CC4); // Disable the TIM Capture/Compare 4 interrupt break; - } default: Error_Handler(); // only channel 1..4 have an interrupt } From d9e4d44cc853d32d6a910f71350d0989d0f6ec94 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 21:02:24 +0100 Subject: [PATCH 11/12] Spaces on an empty line -.-' --- cores/arduino/HardwareTimer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index ce1962cecc..508730f12d 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -635,7 +635,7 @@ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); } - + callbacks[0] = callback; //set or replace user callback } From c2fde3c024317acfff7657bea7492e391a6c8442 Mon Sep 17 00:00:00 2001 From: Lino Barreca Date: Fri, 8 Nov 2019 21:23:12 +0100 Subject: [PATCH 12/12] Uppercase NULL --- cores/arduino/HardwareTimer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/arduino/HardwareTimer.cpp b/cores/arduino/HardwareTimer.cpp index 508730f12d..8bc53c9ced 100644 --- a/cores/arduino/HardwareTimer.cpp +++ b/cores/arduino/HardwareTimer.cpp @@ -632,7 +632,7 @@ void HardwareTimer::setInterruptPriority(uint32_t preemptPriority, uint32_t subP */ void HardwareTimer::attachInterrupt(void (*callback)(HardwareTimer *)) { - if (callbacks[0] == null) { //if there's no callback the ISR here is not enabled + if (callbacks[0] == NULL) { //if there's no callback the ISR here is not enabled __HAL_TIM_ENABLE_IT(&(_timerObj.handle), TIM_IT_UPDATE); } @@ -665,7 +665,7 @@ void HardwareTimer::attachInterrupt(uint32_t channel, void (*callback)(HardwareT Error_Handler(); // only channel 1..4 have an interrupt } - bool mustActivateISR = callbacks[channel] == null; //no user callback means ISR is disabled + bool mustActivateISR = (callbacks[channel] == NULL); //no user callback means ISR is disabled callbacks[channel] = callback;