From cee400156f350fe5b9df3f90d470fed03b33af7d Mon Sep 17 00:00:00 2001 From: Fabiano Riccardi Date: Sun, 12 Nov 2023 12:54:10 +0100 Subject: [PATCH] Fix timer drifting on AVR platform which introduces flickering when dimmers are more than 8 (#45) * improving precision of gate activation on Atmega (needed to control more that 8 dimmers) * example for avr with 16 dimmers * fix: stop timer when turning off all the thyristors at the end of the semi-period * Revert "example for avr with 16 dimmers" * fix assignment N in DimmableLight class (reverted in previous commit); typo --- src/dimmable_light.h | 2 +- src/hw_timer_avr.cpp | 36 ++++++++++++++++++++++++++---------- src/hw_timer_avr.h | 6 +++++- src/thyristor.cpp | 36 +++++++++++++++++++++++------------- 4 files changed, 55 insertions(+), 25 deletions(-) diff --git a/src/dimmable_light.h b/src/dimmable_light.h index 2612784..ca60076 100644 --- a/src/dimmable_light.h +++ b/src/dimmable_light.h @@ -126,7 +126,7 @@ class DimmableLight { }; private: - static const uint8_t N = 8; + static const uint8_t N = Thyristor::N; static uint8_t nLights; Thyristor thyristor; diff --git a/src/hw_timer_avr.cpp b/src/hw_timer_avr.cpp index 0b5509f..b16d3a8 100644 --- a/src/hw_timer_avr.cpp +++ b/src/hw_timer_avr.cpp @@ -69,8 +69,8 @@ static void (*timer_callback)() = nullptr; ISR(TIMER_COMPA_VECTOR(TIMER_ID)) { - // Stop counter - TCCRxB(TIMER_ID) = 0; + // Disable interrupt of Output Compare A + TIMSKx(TIMER_ID) &= 0b11111101; if (timer_callback != nullptr) { timer_callback(); } } @@ -111,7 +111,8 @@ uint16_t microsecond2Tick(uint16_t micro) { void timerBegin() { // clean control registers TCCRxA and TCC2B registers TCCRxA(TIMER_ID) = 0; - TCCRxB(TIMER_ID) = 0; + // Set CTC mode + TCCRxB(TIMER_ID) = 0x08; // Reset the counter // From the AVR datasheet: "To do a 16-bit write, the high byte must be written @@ -123,17 +124,14 @@ void timerBegin() { TCNTxH(TIMER_ID) = 0; TCNTxL(TIMER_ID) = 0; #endif - - // enable interrupt of Output Compare A - TIMSKx(TIMER_ID) = 1 << OCIExA(TIMER_ID); } void timerSetCallback(void (*f)()) { timer_callback = f; } -bool timerStartAndTrigger(uint16_t tick) { - if (tick <= 1) { return false; } +void timerStartAndTrigger(uint16_t tick) { + timerStop(); #if N_BIT_TIMER == 8 TCNTx(TIMER_ID) = 0; @@ -155,9 +153,27 @@ bool timerStartAndTrigger(uint16_t tick) { TCCRxB(TIMER_ID) = 0x07; #elif N_BIT_TIMER == 16 // 0x02: start counter with prescaler 8 - TCCRxB(TIMER_ID) = 0x02; + TCCRxB(TIMER_ID) |= 0x02; #endif - return true; + + // enable interrupt of Output Compare A + TIMSKx(TIMER_ID) = 1 << OCIExA(TIMER_ID); +} + +void timerSetAlarm(uint16_t tick) { +#if N_BIT_TIMER == 8 + OCRxA(TIMER_ID) = tick; +#elif N_BIT_TIMER == 16 + OCRxAH(TIMER_ID) = tick >> 8; + OCRxAL(TIMER_ID) = tick; +#endif + + // enable interrupt of Output Compare A + TIMSKx(TIMER_ID) = 1 << OCIExA(TIMER_ID); +} + +void timerStop() { + TCCRxB(TIMER_ID) &= 0b11111000; } #endif // END AVR diff --git a/src/hw_timer_avr.h b/src/hw_timer_avr.h index 77048ee..b3e453f 100644 --- a/src/hw_timer_avr.h +++ b/src/hw_timer_avr.h @@ -53,7 +53,11 @@ void timerSetCallback(void (*f)()); * * NOTE: 0 or 1 values are not accepted */ -bool timerStartAndTrigger(uint16_t tick); +void timerStartAndTrigger(uint16_t tick); + +void timerSetAlarm(uint16_t tick); + +void timerStop(); #endif // HW_TIMER_ARDUINO_H diff --git a/src/thyristor.cpp b/src/thyristor.cpp index 83fd2ba..d367aba 100644 --- a/src/thyristor.cpp +++ b/src/thyristor.cpp @@ -144,6 +144,10 @@ void turn_off_gates_int() { for (int i = alwaysOnCounter; i < Thyristor::nThyristors; i++) { digitalWrite(pinDelay[i].pin, LOW); } + +#if defined(ARDUINO_ARCH_AVR) + timerStop(); +#endif } /** @@ -173,8 +177,8 @@ void activate_thyristors() { digitalWrite(pinDelay[thyristorManaged].pin, HIGH); thyristorManaged++; - // This while is dedicated to all those thyristor wih delay == semiPeriodLength-margin; those are - // the ones who shouldn't turn on, hence they can be skipped + // This while is dedicated to all those thyristors with delay == semiPeriodLength-margin; those + // are the ones who shouldn't turn on, hence they can be skipped while (thyristorManaged < Thyristor::nThyristors && pinDelay[thyristorManaged].delay == semiPeriodLength) { thyristorManaged++; } @@ -197,9 +201,7 @@ void activate_thyristors() { #elif defined(ARDUINO_ARCH_ESP32) setAlarm(delayAbsolute); #elif defined(ARDUINO_ARCH_AVR) - if (!timerStartAndTrigger(microsecond2Tick(delayRelative))) { - Serial.println("activate_thyristors() error timer"); - } + timerSetAlarm(microsecond2Tick(delayRelative)); #elif defined(ARDUINO_ARCH_SAMD) timerStart(microsecond2Tick(delayRelative)); #endif @@ -232,9 +234,7 @@ void activate_thyristors() { setAlarm(delayAbsolute); #elif defined(ARDUINO_ARCH_AVR) timerSetCallback(turn_off_gates_int); - if (!timerStartAndTrigger(microsecond2Tick(delayRelative))) { - Serial.println("activate_thyristors() error timer"); - } + timerSetAlarm(microsecond2Tick(delayRelative)); #elif defined(ARDUINO_ARCH_SAMD) timerSetCallback(turn_off_gates_int); timerStart(microsecond2Tick(delayRelative)); @@ -299,6 +299,16 @@ void zero_cross_int() { if (diff < semiPeriodLength - semiPeriodShrinkMargin) { return; } #endif +#endif + +#if defined(ARDUINO_ARCH_AVR) + // Early timer start, only for avr. This is necessary since the instructions executed in this + // ISR take much time (more than 30us with only 4 dimmers). Before the end of this ISR, either + // the timer is stop or the alarm time is properly set. + timerStartAndTrigger(microsecond2Tick(15000)); +#endif + +#if defined(FILTER_INT_PERIOD) || defined(MONITOR_FREQUENCY) #ifdef MONITOR_FREQUENCY // if diff is very very greater than the theoretical value, the electrical signal // can be considered as lost for a while and I must reset my moving average. @@ -424,9 +434,7 @@ void zero_cross_int() { startTimerAndTrigger(delayAbsolute); #elif defined(ARDUINO_ARCH_AVR) timerSetCallback(activate_thyristors); - if (!timerStartAndTrigger(microsecond2Tick(delayAbsolute))) { - Serial.println("zero_cross_int() error timer"); - } + timerSetAlarm(microsecond2Tick(delayAbsolute)); #elif defined(ARDUINO_ARCH_SAMD) timerSetCallback(activate_thyristors); timerStart(microsecond2Tick(delayAbsolute)); @@ -445,8 +453,10 @@ void zero_cross_int() { // and no-autorealod was set (this timer can only down-count). #elif defined(ARDUINO_ARCH_ESP32) stopTimer(); -#elif defined(ARDUINO_ARCH_AVR) || defined(ARDUINO_ARCH_SAMD) - // Given actual HAL, AVR and SAMD counter automatically stops on interrupt +#elif defined(ARDUINO_ARCH_AVR) + timerStop(); +#elif defined(ARDUINO_ARCH_SAMD) + // Given actual HAL, and SAMD counter automatically stops on interrupt #endif } }