Repository navigation
Keep the PWM and read() true across a period change - #1
Open
theAndreas wants to merge 1 commit into
Open
theAndreas wants to merge 1 commit into
theAndreas wants to merge 1 commit into
Conversation
read() divided before it shifted, so the truncation was scaled up by the prescaler: at 1024 it moved in steps of 1024 us where a tick is 32 us. Shifted first, it is good to one tick, and 2 * 0xFFFF << 10 still fits. setPeriod() wrote PER outright. In dual-slope mode a TOP that lands below the counter mid-period is not met until the counter has gone round 0xFFFF, one period of up to several seconds where a shorter one was asked for. It now goes through PERBUF, which is taken over at BOTTOM. A duty is a fraction of the period, but it was turned into an absolute compare value once, against the PER of the moment, so init(1000), 50 % and then setPeriod(2000) ran at 25 %. The duty is now kept per channel and the CCxBUF are recomputed alongside PERBUF, arriving at the same BOTTOM. Top is kept as well instead of being read back, because a value still in PERBUF is not in PER yet. start() loads PER and CCx directly while the timer has no clock; otherwise the first period ran on CCx = 0 from init()'s RESET, since the buffers only arrive at the first BOTTOM. Not a forced UPDATE before the RESTART instead: both go to CTRLGSET, and an UPDATE still pending there ORed with RESTART is RESET. The bounds check made setPeriod()'s "set to maximum" branch unreachable; an out of range period is now refused outright, and 0 us with it. attachInterrupt() clears an overflow flag left from while the interrupt was off, which otherwise called the new callback at once. stop() leaves the timer alone before init(), when it may still be the core's. TIMER_USE_TCC4/TCC5 can be given with -D. Selecting a timer that is switched off is now a static_assert rather than an undefined reference at the link, and F_CPU has to be a multiple of 2 MHz, asserted as well. Built with avr-gcc 14.3 for atxmega32e5 at -Wall -Wextra, with TCC5 alone and with TCC4 switched on; both assertions were made to fail on purpose. The same fixes went into the TCA0 port in AndreasBur/Wordclock#79. Not run on hardware.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes what a review turned up, all of it carried into the TCA0 port in
AndreasBur/Wordclock#79 as well, where the same
fixes are in the third commit.
The faults
read()divided before it shifted, so the prescaler scaled the truncation up. At 1024 it movedin steps of 1024 us where a tick is 32 us: a counter of 31 read as 0 us instead of 992. Shifted
first, it is good to one tick, and
2 * 0xFFFF << 10still fits in 32 bit.setPeriod()wrotePERoutright. In dual-slope mode a TOP below the counter mid-period isnot met until the counter has gone round
0xFFFF, one period of up to four seconds in the wrongplace. It now goes through
PERBUF, taken over at BOTTOM.init(1000), 50 % and thensetPeriod(2000)ran at 25 %. The duty is now kept per channel andthe
CCxBUFare recomputed withPERBUF, so both arrive at the same BOTTOM.start()ran onCCx = 0, the valueinit()'s RESET leaves, becausethe buffers arrive only at the first BOTTOM.
start()now loadsPERandCCxdirectly while thetimer has no clock. A forced
UPDATEbefore theRESTARTwould not do: both commands go toCTRLGSET, and anUPDATEstill pending there, ORed withRESTART, reads asRESET.Smaller fixes:
setPeriod()'s "set to maximum" branch could not be reached. An out of range period, and 0 us,is now refused outright.
attachInterrupt()clears an overflow flag left over from while the interrupt was off.Otherwise the new callback ran at once.
stop()does nothing beforeinit(), when the timer may still belong to the core.Configuration
TIMER_USE_TCC4andTIMER_USE_TCC5can now be set with-D. Selecting a timer that is switched off is astatic_assertinstead of an undefined reference at the link.F_CPUhas to be a multiple of2 MHz, also asserted, because
setPeriod()andread()work in whole cycles per microsecond.What was checked
atxmega32e5at-Wall -Wextra, with TCC5 alone and with TCC4switched on.
Timer<TimerC4>with TCC4 off, andF_CPUat 12.5 MHz.read()error was worked through for prescalers 64 and 1024.(1212 -> 1404 bytes,
Timer.cpplinked at-Os -fltoagainst an emptymain()). The RAM isTopand one duty per channel.Not run on hardware.