WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Add a mechanism for storing and setting LED strip specific settings

Open washcroft opened this issue 2 years ago • 17 comments

Add a mechanism for storing and setting LED strip specific settings, such as the constant current level of the TM1814 chip:

image

Also moved the storage of the "off refresh" setting to the new settings bits, rather than being "hacked" into the led/bus type value, whilst retaining backwards config compatibility.

washcroft avatar Mar 18 '22 13:03 washcroft

Is there a benefit if using per-strip individual settings? Wouldn't it be good to have this "settings" as global and applied to all TM1814 strips instead to simplify processing?

blazoncek avatar Mar 18 '22 14:03 blazoncek

Is there a benefit if using per-strip individual settings? Wouldn't it be good to have this "settings" as global and applied to all TM1814 strips instead to simplify processing?

No, I have TM1814 strips that use 50mA high power LEDs, so I run those at 38mA constant current (the max the TM1814 can handle). I also have TM1814 strips that use standard 20mA LEDs. A global setting would prevent you from running both at the same time.

washcroft avatar Mar 18 '22 14:03 washcroft

I have had considered this as part of a per-bus auto brightness limiter, a possible future feature, allowing settings a current limit for individual busses in addition to a global one. I believe a per-bus current selection would not make too much sense without that for now.

Why have you decided to implement it as a select dropdown? mA rating should always use a numeric input IMO.

Aircoookie avatar Mar 18 '22 18:03 Aircoookie

I have had considered this as part of a per-bus auto brightness limiter, a possible future feature, allowing settings a current limit for individual busses in addition to a global one. I believe a per-bus current selection would not make too much sense without that for now.

Don't consider this to be a brightness limiter, this is to enable control of specific settings and features of LED chips that have special settings and features - currently I have only implemented setting the constant current value of the TM1814 chip. Constant current does not scale linearly with perceived brightness, this isn't the same as PWMing the output to control brightness.

Why have you decided to implement it as a select dropdown? mA rating should always use a numeric input IMO.

As above, the LED chip doesn't accept any random numeric input for it's constant current setting - it accepts one of 64 set levels (those in the drop down list).

For example, if you wanted to implement something similar for an LED chip that did accept a numeric constant current, then you'd implement that free form numeric input in the UI but use the same settings bytes to store and pass the setting through to the bus.

washcroft avatar Mar 18 '22 20:03 washcroft

@esev you're familiar with the TM1814 chips, this PR might be of interest

washcroft avatar Mar 18 '22 20:03 washcroft

First of all, I apologize for misunderstanding the purpose of your PR. I thought the mA setting refered to the input for the brightness limiter, and not a configurable chip setting (quite unfamiliar with TM1814). I took a look at your changes and such a "type-specific settings" system looks quite useful. It could for example also be used to set a PWM frequency for analog busses, which is not applicable to digital busses.

My point about the drop-down still stands though. What do you think about <input type="number" max=38 min=6.5 step=0.5>? Storing that to 6 bits would be as simple as value = (input - 6.5)*2

Aircoookie avatar Mar 19 '22 11:03 Aircoookie

No problem. The main point of this was to facilitate getting settings all the way through to the bus, previously this was tricky and probably why the "off refresh" setting was passed through by hacking the last bit of the bus type - so this fixes that too.

Yes, that input would work if you prefer it to the select dropdown? I will make some tweaks if you think we can get this merged in?

washcroft avatar Mar 19 '22 16:03 washcroft

Dear @washcroft could you make this PR compatible with 0_14 branch? 0_14 is ~~almost~~ ready for beta testing so PRs for 0.13 (unless mergeable) will most likely not make the cut.

blazoncek avatar Oct 05 '22 19:10 blazoncek

Dear @washcroft could you make this PR compatible with 0_14 branch? 0_14 is ~almost~ ready for beta testing so PRs for 0.13 (unless mergeable) will most likely not make the cut.

@blazoncek Done, and retested for compatibility

washcroft avatar Oct 05 '22 23:10 washcroft

@Aircoookie can you re-target this to main now and reopen?

blazoncek avatar Oct 20 '22 06:10 blazoncek

Hi @washcroft . I have tried to incorporate @Aircoookie comment and update this PR with latest commits from upstream. Please verify if it still conforms to your needs and works correctly.

blazoncek avatar Jan 06 '23 14:01 blazoncek

@blazoncek @Aircoookie as we have "per-bus auto brightness limiter" in 0.15, does this PR still provide additional value to users? Or should we close it as outdated?

softhack007 avatar Jun 25 '24 21:06 softhack007

@softhack007 AFAIK this adds LED chip (AKA individual LED) current limiter, not bus current limiter.

The issue with this PR is that it is (too) old and there were numerous changes to BusManager and UI section and may not be easy to update it as is. Perhaps a new PR is in order to clear things up as there was no interest from OP after my update and I do not intend to do so again.

blazoncek avatar Jun 26 '24 06:06 blazoncek

@softhack007 This isn't related to brightness limiting.

@blazoncek You asked me to bring this PR into compatibility with 0_14, which I did, but then nothing happened with it - so it felt like I'd wasted my time, both on the initial PR and the subsequent update.

If this is still of value, I am happy to bring it up to date again, assuming something is going to be done with it?

washcroft avatar Jun 26 '24 14:06 washcroft

@washcroft there was no response from you since Jan 6, 2023!

As far as I am concerned I have no added value from it, though I spent some time to incorporate features @Aircoookie requested. What that means is, I have no further interest to do anything related to this PR.

If this PR has any value, we are yet to hear it from users (probably minority) of WLED. So far, nobody replied or voted for it.

blazoncek avatar Jun 26 '24 14:06 blazoncek

@blazoncek what I meant is that I don't want to put the time/effort in bringing it up to date if it's not going to get merged, however I am happy to do so if it will get merged?

Appreciate use for this is minimal, not many strips/drivers can be "configured" (the TM1814 can), but this mainly provides a base for settings to exist and be passed through the bus, i.e. future proofing

washcroft avatar Jun 26 '24 14:06 washcroft