core icon indicating copy to clipboard operation
core copied to clipboard

Add WLED Segments Light Capabilities support

Open frenck opened this issue 2 years ago • 47 comments

Proposed change

Add support for segment-based light capabilities to the WLED integration.

Needs: Lots of testing with IRL devices

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] New integration (thank you!)
  • [x] New feature (which adds functionality to an existing integration)
  • [ ] Deprecation (breaking change to happen in the future)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • [ ] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [ ] There is no commented out code in this PR.
  • [ ] I have followed the development checklist
  • [ ] The code has been formatted using Black (black --fast homeassistant tests)
  • [ ] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [ ] The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • [ ] New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • [ ] Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • [ ] No score or internal
  • [ ] 🥈 Silver
  • [ ] 🥇 Gold
  • [ ] 🏆 Platinum

To help with the load of incoming pull requests:

frenck avatar Jul 15 '22 07:07 frenck

I'm happy to provide testing help. Please let me know. I just bought 3 Athom bulbs running WLED not realising that this issue existed and would stop me using them in HA.

sparkydave1981 avatar Jul 30 '22 02:07 sparkydave1981

Just bought a ton of Athom bulbs and they dont work because of then xD

Phara0h avatar Aug 01 '22 18:08 Phara0h

Same here with the Athom bulbs. Hopefully this can be merged soon (if ready). I can help testing whenever I have time.

Yunus1903 avatar Aug 04 '22 21:08 Yunus1903

I've got a ton of bulbs affected by this too, I'm more than happy to test this out

MelvIsntNormal avatar Aug 13 '22 12:08 MelvIsntNormal

The bulbs affected are not the ones that need testing 🫣

frenck avatar Aug 14 '22 19:08 frenck

Looking forward to seeing this merged, so a bunch of my WLED-flashed Electrodragon ESP boards can start working again. Is there anything on the user-facing end that would be helpful in testing this?

augustf avatar Oct 28 '22 18:10 augustf

I've got devices both CCT and RGB. Happy to offer any of them (and my time) up for testing.

boehmap avatar Nov 21 '22 20:11 boehmap

I've got devices both CCT and RGB. Happy to offer any of them (and my time) up for testing.

Not sure how this got marked as off topic, as it relates directly to a listed need....

boehmap avatar Dec 12 '22 04:12 boehmap

I've got devices both CCT and RGB. Happy to offer any of them (and my time) up for testing.

Not sure how this got marked as off topic, as it relates directly to a listed need....

Yeah-my comment offering testing was marked off-topic as well, which is hard to understand as the description explicitly specifies the need as "lots of testing with IRL devices," something both our comments are explicitly offering. That said, it looks like this PR is approaching the top of the incoming queue, so I'm hopeful of movement soon. It's fairly clear to me at least that the current state of nerfed CCT functionality is worse than the slight bugginess of it prior to the change, so this should be an easy win as and when the team can get to it.

augustf avatar Dec 13 '22 19:12 augustf

Was just trying to use the component from this PR as a custom_component and realized that the check for "cct" in config_flow had only been removed for zeroconf, but not for user_input / user conf: https://github.com/home-assistant/core/pull/75248/files#diff-d255ec39863471df4ce0e44fd6e8de8d70abd805e0e91a0bcc2ff3e69bdba090R45

Is that intended or an oversight?

So far, I've not been successful in controlling WLED instances with RGBCCT strips with this change/PR. I can add RGBCCT WLED instances now, yes, but can't control them properly: I have two setups I'm testing with:

  • QuinLED-An-Penta with an RGBCCT strip and WLED v14 (latest master at time of writing) flashed to it:
    • Brightness slider sits on 0%, although the lights are on 80%. Changing the brightness slider changes brightness though
    • I Have three tabs when controlling the device (comparing with my Yeelight bulbs which are also RGBCCT bulbs, they only have two (Color & Temperature)): Color, Temperature & White. "White" just has an effect selector, that's it. Nothing to do there really.
    • Changing the white color temperature does nothing to the LEDs
  • Athom 7W bulb, flashed with latest v13.3
    • Brightness slider also sits on zero although bulb is on 60%, changing brightness doesn't do anything
    • Also has three tabs, similar to what I saw for the An-Penta WLED instance
    • Also changing temperature doesn't do anything
    • Sadly flashing to WLED v14 bricked the bulb. I need to flash it via a usb serial stick and solder some wires to bring it back...

Both had been tested with "White Balance correction" disabled, "Global override for Auto-calculate white" set to "None" and "Calculate CCT from RGB" disabled

Happy to do more testing! Just let me know what to specifically test!

ezcGman avatar Dec 22 '22 14:12 ezcGman

So I've successfully updated the bulb to the latest WLED master (0e236f9), which is also 0.14.0-b1, just released tonight 🥳

Behavior is equal to what I described with the An-Penta above. Here are some screens I took. Let me know if I can test or try out anything else! Would love to help and get that released :)

wled-cct-0 Brightness slider in HA sits on 0, but actually is on 50%. Changing the brightness in HA though works and actually changes the brightness.

wled-cct-1 Temperature slider sits on full warm, but actually is on middle/middle. Changing it does nothing to the LEDs.

image Screen of the LED config

And the light.wled entity in Dev Tools:

min_color_temp_kelvin: 2000
max_color_temp_kelvin: 6535
min_mireds: 153
max_mireds: 500
effect_list:
[...]
supported_color_modes:
  - color_temp
  - rgb
  - white
icon: mdi:led-strip-variant
friendly_name: WLED
supported_features: 36
color_mode: unknown
effect: Solid

Also attaching all API request responses python-wled does: state.json.txt info.json.txt json.json.txt presets.json.txt si.json.txt

ezcGman avatar Dec 23 '22 13:12 ezcGman

as just trying to use the component from this PR as a custom_component and realized that the check for "cct" in config_flow had only been removed for zeroconf, but not for user_input / user conf:

@ezcGman Yup, oversight, removed it.

frenck avatar Dec 23 '22 14:12 frenck

@ezcGman Thanks for posting your states. Oddly enough, your device communicates 15 as its light capability; however, according to the documentation of WLED, that should not be possible.

The following bits are documented:

image

However, your lc is 15, which indicates there is another bit into play. I think this has to do with the auto-calculation of the white from RGB setting

frenck avatar Dec 23 '22 15:12 frenck

image That's what is currently configured for that bulb, if that helps

ezcGman avatar Dec 23 '22 15:12 ezcGman

~Ah and the bulb is compiled with WLED_USE_IC_CCT which is used here: https://github.com/Aircoookie/WLED/blob/61b5e5ad7e5981c0552c25d18be6f52f5f0dfc31/wled00/bus_manager.h#L409~

It is compiled with WLED_MAX_CCT_BLEND=0, used here: https://github.com/Aircoookie/WLED/blob/61b5e5ad7e5981c0552c25d18be6f52f5f0dfc31/wled00/bus_manager.h#L198

Noting: The WLED build for the An-Penta I mentioned above (which has the same issues in controlling) doesn't have that flag compiled into it, so I don't think that makes a difference

ezcGman avatar Dec 23 '22 15:12 ezcGman

Heyhey,

while it's still christmas time, so no rush on this: Just wanted to check if there is an issue on WLED GitHub for this one can follow, or if you have a conversation with them, or if somebody else would need to check with WLED on this!

Thanks!

ezcGman avatar Dec 26 '22 19:12 ezcGman

This is not upstream depended or issue.

frenck avatar Dec 26 '22 20:12 frenck

And what exactly does that mean? :)

Or rephrasing: How is that an answer to my question?

ezcGman avatar Dec 26 '22 21:12 ezcGman

Just wanted to check if there is an issue on WLED GitHub for this one can follow

There is none, this is the Home Assistant project, not the WLED project.

This PR is adopting or upstream changes, this is not an upstream issue.

frenck avatar Dec 26 '22 21:12 frenck

Ok, I'm really interested to finally get WLED CCT back working in HA. I would really like to understand what the issue here is.

What should WLED report to make HA understand that it is a CCT (or even RGBCCT) light? Can u link me to any HA docs?

If you're not raising an issue or telling the WLED people what to fix, would be at least appreciated if you could tell what the issue here is, so I can at least start to understand and maybe fix what's wrong.

Thanks!

ezcGman avatar Dec 26 '22 22:12 ezcGman

Ok, I'm really interested to finally get WLED CCT back working in HA.

That is what this pr is aimed to do, as per Pr description (as a side effect).

I would really like to understand what the issue here is.

What issue? This is an PR, not an GitHub issue.

What should WLED report to make HA understand that it is a CCT (or even RGBCCT) light? Can u link me to any HA docs?

The developer docs of Home Assistant can be found in developers.home-assistant.io

If you're not raising an issue or telling the WLED people what to fix, would be at least appreciated if you could tell what the issue here is, so I can at least start to understand and maybe fix what's wrong.

I am not following this one. I am not sure if there is a WLED issue needed for this PR? Not do I understand what they need to fix for this PR? What fix?

I think you are misunderstanding this PR and what it is doing overal.

This PR is switching to segments based light capabilities exposed by newer WLED versions, which also exposes CCT (assumingely correctly) on the WLED device.

I feel like your comments are no longer focusing on this PR to be honest?

frenck avatar Dec 26 '22 22:12 frenck

OK, I try to phrase it differently.

Even if your PR may be about something not directly related to CCT lights, you at least change something that makes it possible again to add WLED CCT lights in HA. This is literally what the removal of that check for cct lights does.

I've tested this PR here and added a few WLED CCT lights to HA and reported that something is not properly working. That is the issue I was talking about (thanks, I know the difference between a PR and an issue).

I was asking you for guidance if you have any hints on what could go wrong and what HA expects HA to do to fix this.

I'm not asking you to fix it.

I'm not saying this PR should fix it.

I try to offer my help and fix what's going wrong. Or maybe this is already known and tracked somewhere. Or expected to not work and is addresses with another change, either here or on WLED. Or on wled-python which is managed by, well, you.

So again, if you have any information that would enable me to get a lead in helping to do whatever is needed to get WLED CCT lights back to HA: Thanks!

ezcGman avatar Dec 26 '22 23:12 ezcGman

you at least change something that makes it possible again to add WLED CCT lights in HA.

Correct, this was already mentioned and answered above.

I've tested this PR here and added a few WLED CCT lights to HA and reported that something is not properly working.

Yes and thanks for that. I haven't looked into it yet, this PR is draft for a reason.

That is the issue I was talking about

I honestly don't know at this point, it is an issue. As said, the light capability value you mentioned is undocumented/unknown. Besides you are running a custom firmware, however, that is not what this integration is aiming to serve. This integration is for integrating the official WLED firmware.

There are many more forks and custom firmwares out there, that won't work.

was asking you for guidance if you have any hints on what could go wrong and what HA expects HA to do to fix this.

As said before, it don't know if there is an issue to be honest.

So again, if you have any information that would enable me to get a lead in helping to do whatever is needed to get WLED CCT lights back to HA

So again, this PR is adding support for the light capabilities for segments exposed by newer WLED versions, which also exposes CCT (assumingely correctly) on the WLED device.

Thanks for testing, and yes, adding CCT support for would be nice 👍

However, for me that is a side-effect. Using light capabilities adds support for way more than just CCT and affects all possible color modes ANY segment can have (instead of the whole device as previously was the case).

This PR is therefore not about CCT.

frenck avatar Dec 27 '22 06:12 frenck

Rebased to get it on top of the latest dev branch, which has an updated version of the wled package (v0.15.0).

frenck avatar Dec 27 '22 21:12 frenck

Adjusts the WLED light capabilities vs Home Assistant color modes tests, based on testing all different capability modes in WLED (well, minus one bug reported upstream).

The next step is adjusting the code to make the tests pass.

frenck avatar Dec 27 '22 23:12 frenck

Sorry for the late reply, I have been out the whole day!

Besides you are running a custom firmware, however, that is not what this integration is aiming to serve. This integration is for integrating the official WLED firmware.

My firmware isn't really custom ; yes it's custom compiled but no changes to code. Yes, the bulb firmware has the mentioned buildflag set, but the An-Penta hasn't. But it's a fair point, I could test it on the An-Penta with a real bin released by AC to 100% iron out it could be something I do custom.

However, for me that is a side-effect. Using light capabilities adds support for way more than just CCT and affects all possible color modes ANY segment can have (instead of the whole device as previously was the case).

This PR is therefore not about CCT.

Thanks for explaining that in more detail! Understood! I would say at this point I would see where this PR goes / what you consider being part of it (not meant mean in any way; just saying that I can't judge the scope of it) and see what is missing when it's merged. Maybe I can then jump on something to bring CCT support for WLED for HA (if not then already solved by your PR).

Thanks for this btw: https://github.com/Aircoookie/WLED/issues/2979

Let me know if I can test anything, or if I can help with changes on WLED. I'll definitely take a look at that issue there and see if I can address it with a change / fix; I'm way more experienced in WLED's code than HA ;)

Again: If I can test any new commits here or on WLED: Let me know. I have plenty of WLED setup and can create any, if I miss one. From any digital to analog, RGB, CCT, RGBCCT, test with the Athom bulbs... Whatever helps!

ezcGman avatar Dec 27 '22 23:12 ezcGman

After some debugging, my lights do have an lc value of 7 until this line: https://github.com/Aircoookie/WLED/blob/main/wled00/FX_fcn.cpp#L754

The bitwise OR with 8 doesn't really sound right, or it's not documented: https://kno.wled.ge/interfaces/json-api/#light-capabilities, as you said. So it may make sense to just look at the first three bits and ignore the others (they're mentioned as "Reserved (expect any value)", which is nothing one can work with)?

Commenting out that line "fixes" my rgbcct lights to report 7 again

ezcGman avatar Dec 28 '22 01:12 ezcGman

Any updates on this one? just tried wled on my athom bulbs and came across this issue. tried the latest commit as a custom component and I'm seeing the same issues mentioned by @ezcGman.

tungmeister avatar Feb 19 '23 23:02 tungmeister

Any updates on this one? just tried wled on my athom bulbs and came across this issue. tried the latest commit as a custom component and I'm seeing the same issues mentioned by @ezcGman.

For what it's worth, I ended up switching my Athom bulb to ESPHome as a workaround, which works brilliantly. Description of my config is here

intrinseca avatar Feb 20 '23 09:02 intrinseca

Moving conversation from this ticket to here then: https://github.com/Aircoookie/WLED/issues/2984#issuecomment-1461791132

So I guess it's "only" the tests MR that's open that is blocking it? :) Thanks!

ezcGman avatar Mar 09 '23 10:03 ezcGman