Closed AJMansfield closed 5 years ago
One possible approach is to use compiler macros, and place code relevant to each version in a different branch.
The macros might be something like:
#define CHAN_A true;
#define CHAN_B true;
(For the real code I'd likely want to use longer macro names to avoid conflicts, like TRIAC_DIMMER_CHAN_A
.)
For most of the code the way to apply this is fairly obvious — the majority of statements only concern a single channel or are required regardless of whether only one channel is in use — but for bit manipulation code this does become a bit trickier.
For instance the single line manipulating the TCCR1A
control register in the capture interrupt (TriacDimmer.cpp:120) expands out to a whopping 13 lines doing it naively:
TCCR1A |=
#if CHAN_A
_BV(COM1A0) | _BV(COM1A1)
#else
0
#endif
|
#if CHAN_B
_BV(COM1B0) | _BV(COM1B1)
#else
0
#fi
; //set OC1x on compare match
The solution for these bit manipulation issues is to define additional macros that correspond to the bit values only when the channel is enabled. For instance earlier in the file do:
#if CHAN_A
#define BV_COM1A0 _BV(COM1A0)
#define BV_COM1A1 _BV(COM1A1)
#define BV_OCF1A _BV(OCF1A)
#define BV_OCIE1A _BV(OCIEA)
// ... other similar #defines ...
#else
#define BV_COM1A0 0
#define BV_COM1A1 0
#define BV_OCF1A 0
#define BV_OCIE1A 0
// ... other similar #defines ...
#endif
And similar macros for channel B.
Then the earlier TCCR1A
line and the neighboring lines can be written as:
TCCR1A |=BV_COM1A0 | BV_COM1A1 | BV_COM1B0 | BV_COM1B1; //set OC1x on compare match
TIFR1 = BV_OCF1A | BV_OCF1B; //clear compare match flags
TIMSK1 |= BV_OCIE1A | BV_OCIE1B; //enable input capture and compare match interrupts
Note that, technically the only thing that actually needs to be done to disable a channel is ensure we don't configure the unused output pin at TriacDimmer.cpp:29-30 and not set the COM1x1
and COM1x0
bits in TCCR1A
at TriacDimmer.cpp:120.
Entirely disabling the unused interrupts and code would be a nice perk to be sure (for performance reasons), but it's not actually necessary as long as we just keep OC1x
disconnected and in "normal port operation" mode.
For reference: ATMega328p datasheet section 13.11.1: TCCR1A – Timer/Counter1 Control Register A.
how would the user set the macro? Arduino doesn't have a good way of setting global defines
There may be other advantages to performing the disable logic at runtime rather than disabling the feature entirely at compile time. Aside from allowing the library to only use one of the pins, it's possible to use this ourselves to get slightly better dimming performance.
Using phase cutting for dimming values even for values very close to 0% or 100% is not technically the absolute optimal behavior:
For 100% on, we could driving the output pin as constant high. This isn't great for power efficiency but it does guarantee absolutely no flicker and only requires the ability to disable the pulse.
A slightly better output is to emit a pulse that goes high a few hundred µs before the zero crossing and ends a few hundred µs afterward. While this is easily possible without requiring a different timer mode, it would require much more extensive software changes to support. In particular, it would require entirely rerouting the logic in both the input capture and output compare interrupts to schedule the entirely opposite pulse edge from normal, and at a very different offset than normal.
(Note that implementing this more complicated behavior is a different matter from the single-channel operation considered here.)
how would the user set the macro? Arduino doesn't have a good way of setting global defines
You make a good point, and I'm thinking it may be better to do this at runtime anyway for reasons I outlined in my previous comment.
If disabling it at compile time is desired though there's another approach using template parameters. This would require some structural changes to the code (moving everything into an instantiated class), and it's not clear if it's even possible to control whether the ISR gets defined that way.
There might also be a way to disable a channel at compile time with constexpr
parameters, although that's likely to be more complicated.
OCR1 for unused pin could be set to a value that would not match so interrupt would not trigger
OCR1 for unused pin could be set to a value that would not match so interrupt would not trigger
That would be a valid approach to implement the no-pulse 0% dimming behavior, but that still wouldn't allow application code to use the pin.
If you look at Figure 13-5 from the datasheet you can see how the PORT
and OC1x
registers are muxed together to control the output pin; if application code wants to have control of the pin's output both COM1x0
and COM1x1
need to be 0.
This behavior should now be available in the development branch, including the change in behavior for dimming values near 0.0 and 1.0.
For now, values near 100% just result in "always on" rather than the more power-efficient inverted pulse, but this should do for now.
The threshold where these behaviors take over are configurable with the on_thresh
and off_thresh
parameters to TriacDimmer::begin()
.
@jandrassy Can you confirm that the updated code in dev works correctly on real hardware?
Split from #11.
Credit to @jandrassy for his suggestion:
The library ought to be able to be configured to use only one of the two channels and leave the other free for other uses.