Skip to content
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

Improvements to LED Help texts #31

Closed
wants to merge 1 commit into from

Conversation

surfdado
Copy link
Contributor

I tried to clarify a few things that confused me or that seemed counter-intuitive. Also removed the (redundant) explanation of the different white LED alternatives for secondary colors.

I tried to clarify a few things that confused me or that seemed counter-intuitive.
Also removed the (redundant) explanation of the different white LED alternatives
for secondary colors.
Copy link
Owner

@lukash lukash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for these, appreciated. A few comments...

<ul style="margin-top: 0px; margin-bottom: 0px; margin-left: 0px; margin-right: 0px; -qt-list-indent: 1;"><li style="" style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;">White (full) is full brightness on all four channels, the brightest it can be. It also consumes the most power. On RGB LEDs, this is the same as White (rgb).</li>
<li style="" style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;">White (rgb) is the white of the three RGB channels.</li>
&lt;li style=&quot;&quot; style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;White (single) only works on RGBW and is the single white channel. It's black on RGB-only LEDs. Less bright than White (full), but consumes significantly less power.&lt;/li&gt;&lt;/ul&gt;&lt;/body&gt;&lt;/html&gt;</description>
&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;Front secondary color. Only used by a few of the fancy lighting modes.&lt;/li&gt;&lt;/ul&gt;&lt;/body&gt;&lt;/html&gt;</description>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with "Only used by a few of the fancy lighting modes." a lot of them actually use it. Basically all of them besides "Solid" and the rainbow modes. What about this if the goal is to make succinct:

Front secondary color for most animated modes.

@@ -2696,7 +2691,7 @@ p, li { white-space: pre-wrap; }
&lt;html&gt;&lt;head&gt;&lt;meta name=&quot;qrichtext&quot; content=&quot;1&quot; /&gt;&lt;style type=&quot;text/css&quot;&gt;
p, li { white-space: pre-wrap; }
&lt;/style&gt;&lt;/head&gt;&lt;body style=&quot; font-family:'Roboto'; ; font-weight:400; font-style:normal;&quot;&gt;
&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;Front animation speed.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</description>
&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;Front animation speed. Higher numbers make it slower.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</description>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true. You scared me a bit if this was true it'd be pretty confusing. If you've found a mode for which it makes it slower, it's a bug.

@@ -3266,7 +3246,7 @@ p, li { white-space: pre-wrap; }
&lt;html&gt;&lt;head&gt;&lt;meta name=&quot;qrichtext&quot; content=&quot;1&quot; /&gt;&lt;style type=&quot;text/css&quot;&gt;
p, li { white-space: pre-wrap; }
&lt;/style&gt;&lt;/head&gt;&lt;body style=&quot; font-family:'Roboto'; ; font-weight:400; font-style:normal;&quot;&gt;
&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;Duty threshold at which the duty is shown on the status bar instead of the battery. Note there is a 10% hysteresis, meaning if you duty shows at 20%, it will be shown until it drops below 10% (to avoid constant blinking). Values below 15% set the threshold to 15%.&lt;/p&gt;
&lt;p style=&quot; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;Duty threshold above which the duty is shown on the status bar (yellow or red) instead of the battery. Note there is a 10% hysteresis, meaning if you duty shows at 20%, it will be shown until it drops below 10% (to avoid constant blinking). Values below 15% set the threshold to 15%.&lt;/p&gt;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure red needs to be mentioned. It's only the last few LEDs that are colored red if duty is that high. Red could be used for something else in the future and there could be other bars that'll have their top ends colored red (the description could obviously be tweaked then, just trying to avoid back and forth). What about:

... duty is shown on the status bar (in amber yellow color) instead of ...

@surfdado
Copy link
Contributor Author

I have no way to test LEDs for the next two weeks so I trust that you're right about the speed and I just misinterpreted what I observed. All the changes you propose make sense to me. Feel free to just make them yourself - I don't care if my name is attached to the commit

@lukash
Copy link
Owner

lukash commented Dec 21, 2024

Alright, pushed it here: 633bd26

(this one should have been against main, as these are fixes to current functionality which don't need to be tested)

I also upped the default for the leds.status.duty_threshold to 50% (in another commit).

@lukash lukash closed this Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants