starryalley / Anduril2

my Anduril2 fork that provides much more features
GNU General Public License v3.0
46 stars 2 forks source link

Channel switching strobe modes have issues at ramp levels >130 #20

Closed SiteRelEnby closed 1 year ago

SiteRelEnby commented 1 year ago

By default, I can ramp brightness up to 150/150 on the dual channel strobe modes (even with normal ceiling set to 130), at which point the two channels start being mixed all the time as a steady brightness (effectively just 200% turbo).

The effect is actually quite nice for the smooth alternating mode at levels above 130 (it still alternates, but neither channel ever switches fully off) but at level 150 it is just 200% turbo with no channel switching. The best idea might be to pause ramping up at level 130 and blink once, then if the user keeps holding the button after a second or two, continue up to 149 and stop there. The alternative of stopping at the configured ceiling is the other logical way, but I like that one less as I have my normal ceiling at 130/150 on dual channel lights (since my anduril fork has direct shortcuts to 200% and both 100% turbo modes all available at the same time) and still like the effect of the channel ramping modes at 130-149, 150 is just obviously unintentional-appearing. Alternatively, making which it does be user-configurable.

On the other hand, the instant alternating strobe mode works less well with this - if channels are mixed in ramp mode and then that strobe is selected, it will alternate between the mixed channels and the second channel on 100%. The two approaches I can think of to handle this would be to either always set to single channel max at each end, or to invert the ramp (e.g. if the user ramped to a tint value of 64, alternate between that 191 as the opposite point (easy to obtain with tint ^ 0xFF).

May hack on this myself a bit if I have time. Really like your work, have incorporated some of your changes into my fork as well. If there's any features there you like I can probably make a merge request, at the moment I'm trying to remove some interdependencies and conflicts so they could be more easily applied to different forks.

starryalley commented 1 year ago

Thank you for this feedback and proposed solution! I'll see what I can do when I have some time. Thanks for using this fork too. Feel free to take whatever you like to yours as well!

SiteRelEnby commented 1 year ago

Fixed it in my version, backported: https://github.com/starryalley/Anduril2/pull/21

starryalley commented 1 year ago

Awesome, thank you! I will need to find some time to check this!

starryalley commented 1 year ago

Hi @SiteRelEnby I finally got some time to review this. My apologies. I have got 2 busy little girls to deal with every day after work so less time now for Anduril2. Thanks for the excellent fix in this PR. Your code works perfectly.

However on my (limited numbers of) dual channel lights I don't see a benefit of going above RAMP_SMOOTH_CEIL (130) even in smooth alternating mode. Maybe it works better in some LED combinations. I hope you don't mind I won't merge the PR. I will however adopt your proposed fix so the max level user can go is 130, for both alternating modes.

Your fork that has direct shortcuts to 200% and both 100% turbo modes is brilliant. Since I'm getting more dual/multi channel Anduril2 lights, I might copy that over here :P

Thank you again.

SiteRelEnby commented 1 year ago

Yeah, no problem there, I like it with my white/red light although it's less useful on flood/throw. I'm keeping my fix in my version, or maybe make it configurable once I'm finished with LT1S Pro support.