-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix: aux channel modes should use low brightness for level 1 (not off) #47
Conversation
3a4492a
to
7b766a7
Compare
Rebased and updated the description to refer to "level 1" as the lowest level, matching the manual. |
Level 0 is off, everything else was converted to 1 in the expressions affected by this pull request. This is the intended behavior. Your change to the single color aux results in either 1 (low) or 3 (undefined). How are aux LEDs turned off after this change? Doesn't it break number readout via aux LEDs? |
Thanks for checking this out @SammysHP.
To clarify the discussion of the problem and the proposed solution, I have opened a corresponding issue: #65. I do not think that the behavior described there is intended (using aux as the active channel and selecting the lowest level resulting in the aux turning off). But my solution might not be the right one. I am still new to this codebase, but I made this change based on trial-and-error on my ts10 and ts10-rgb. So I will try to understand what is actually happening now.
It seems that there are two different definitions of "level 0" in use. In the Anduril UI code, there appears to be an offset of 1. So in ramping.c, when the level is "level 0", that corresponds to "off" and is treated as a special case. Since that uses
Then when a level is 1 or higher, there is an offset of -1 applied before calling the corresponding
So before my change, when the user selects Anduril's level 1, that results in a call of the
Yes, But now that I'm taking a second look, there is a better solution which is less confusing: I can replace this with
I don't think so, but I might be missing something. |
Oh right, I remember there was some refactoring going on recently. Sounds like this might have introduced the bug. |
Thanks for spotting this and fixing it. It looks like I missed that part of the code during a refactor at some point, and it should have been off/low/high all along instead of just off/high. |
thanks @ToyKeeper for accepting! |
In the current implementation of using aux LEDs as channels, level 1 (the lowest level) results in the aux being set to "off" while higher levels result in the aux set to "high".
With this change, level 1 will result in aux "low" while higher levels continue to result in aux "high".
This makes the chan-aux and chan-rgbaux feature slightly more useful. Fixes #65 , also fixes #62.
Testing