Support for Fw1906 #3298
Support for Fw1906 #3298
Conversation
This enable it to other bus types.
…CTT protected member.
wled00/bus_manager.cpp
Outdated
| if (_type == TYPE_FW1906) | ||
| calculateCCT(c, ww, cw); | ||
|
|
||
| PolyBus::setPixelColor(_busPtr, _iType, pix, R(c), G(c), B(c), ww, cw, co); |
There was a problem hiding this comment.
Hi,
setPixelColor() is on the critical path for performance.
Is it strictly necessary to extend the polyBus interface to explicitly require color components (r,g,b,w) instead keeping them in uint32_t and just adding the new byte for cw?
If I understand your code correctly, we will - for all kinds of busses - split RGBW into R(c), G(c), B(c), W(c), and later re-assemble them into uint32_t? This adds around 30-40 extra operations just to go back and forth between color formats. It might seem like a "small price", but when applied to a few thousand LEDs at 100 fps, it accumulates into a noticeable slowdown.
There was a problem hiding this comment.
Hi,
The split is not required. I can change the interface. But in PolyBus::setPixelColor the uint32 is split for 4 uint8 local variables anyway, and this operation is removed. I don't want to guess witch version is faster.
There was a problem hiding this comment.
Thanks for the clarification :-) so before changing, @blazoncek what do you think, which option would be more efficient?
There was a problem hiding this comment.
I would leave API as is and add a static variable to hold CCT information for PolyBus.
|
You can use something similar to: Unfortunately I do not like using unreleased libraries so we'll probably wait until new version of NPB is released as I mentioned elsewhere that last NPB update caused us hedaches we are still recovering from. |
wled00/bus_manager.cpp
Outdated
| if (_type == TYPE_SK6812_RGBW || _type == TYPE_TM1814 || _type == TYPE_WS2812_1CH_X3) c = autoWhiteCalc(c); | ||
| if (_cct >= 1900) c = colorBalanceFromKelvin(_cct, c); //color correction from CCT | ||
| if (_type == TYPE_FW1906 || _type == TYPE_SK6812_RGBW || _type == TYPE_TM1814 || _type == TYPE_WS2812_1CH_X3) c = autoWhiteCalc(c); | ||
| if (_type != TYPE_FW1906 && _cct >= 1900) c = colorBalanceFromKelvin(_cct, c); //color correction from CCT |
There was a problem hiding this comment.
That's inapproptiate as user has an option to use color temperature calculated from RGB values.
There was a problem hiding this comment.
It is also not used for TYPE_ANALOG_5CH.
There was a problem hiding this comment.
Removed in last update.
wled00/bus_manager.cpp
Outdated
| if (_type == TYPE_FW1906) | ||
| calculateCCT(c, ww, cw); | ||
|
|
||
| PolyBus::setPixelColor(_busPtr, _iType, pix, R(c), G(c), B(c), ww, cw, co); |
There was a problem hiding this comment.
I would leave API as is and add a static variable to hold CCT information for PolyBus.
|
I was trying this # notation but it seems not working in lib_deps. Its understandable that this change needs to wait for new release of NeoPixelBus. I push this for review, and for other people to test. |
|
It does whe properly formatted. I'm away from computer so can't provide exact format ATM. |
I've done it in a little different way. Instead of adding new static variables I call a static function from class Bus in PolyBus. |
|
I hate the idea of mixing Bus into PolyBus so I removed the dependency. There is also an issue you did not address and that is reverse CCT for retrieving pixel information from |
|
I assumed that it is not an issue, no reverse CCT is used for other pixel formats in digital and PWM buses. |
Other CCT enabled bus ( |
|
@Robert-github-com I have re-checked original source regarding CCT handling ( Nevertheless, @Aircoookie and @softhack007 what would be your idea for adding CCT handling to |
|
@blazoncek this is something I already thought a lot about when originally adding CCT support. I'd say adding support for CCT to Also thought about cramming everything into the |
|
So what is conclusion about API? At least for this change. |
|
I'm trying to compile this right now and give it a try as I have a similar light strip! Wish me luck! |
|
Unfortunately the chips in my strips appear to be something other than the FW1906, despite having a similar LED configuration. The chips themselves are entirely unlabled. Sorry Robert, I wish I could have helped test this:
|
|
Hi folks, I've squashed all the cleanup to create a minimal set of patches, and rebased on top of the current main. @Robert-github-com , you can grab it here: https://github.com/deece/WLED/tree/fw1906 |
Great! Seems to work fine. |
It worked for me. One small problem is that I can't seem to be able to change it to a mode where only the white LEDs are used with a blend of cold and warm color temperatures. As shown in the screenshot, when I change the first slider to the left (turning off the color LED altogether), the second slide automatically slides to the rightmost (cold white), If I drag the second slider (white point) anywhere towards the left, the first slide immediately goes to 100%, and the color LEDs turn on (lower CRI) and make the color of objects in the room look weird. Thanks! |
|
It appears that because I implemented enabled Calculate CCT from RGB WLED can integrate with home assistant. Not sure if this is the expected behavior 🤔 |
|
@deece Hey man I compiled your FW1906 fork and uploaded to my ESP32. I cant see the option for FW1906 in the LED config. |
That is a HA issue. It will not allow WLED with CCT. |
If you modify UI files in wled00/data, make sure to run https://kno.wled.ge/advanced/custom-features/#changing-web-ui |
Hey man thanks for the reply. Im not much of a coder but can manage to compile wled in vscode. I am not quite sure on how i do this at all. Any extra help would be muchly appreciated. Thanks |
softhack007
added
the
major
| @@ -232,6 +232,7 @@ | |||
| #define TYPE_WS2811_400KHZ 24 //half-speed WS2812 protocol, used by very old WS2811 units | |||
| #define TYPE_TM1829 25 | |||
| #define TYPE_UCS8903 26 | |||
| #define TYPE_FW1906 28 //RGB + CW + WW + unused channel (6 channels per IC) | |||
There was a problem hiding this comment.
I think the ordering here is an incorrect assumption that impacts the flexibility of this PR.
FW1906 is just a dual RGB (or, more accurately, GRB) IC — there aren't specific CW/WW pins.
It's up to the manufacturer how to connect their LEDs, and unfortunately, they take liberties here.
The FW1906 strip I have, for instance, orders its channels as WW, null, G R B, CW. This ordering, while strange and annoying, actually does make sense — it lines up nicely with the physical ordering of the LEDs on the strip and optimized for the traces vs how humans want to think about it.
I'd think before this PR is super viable to merge, we need the ability to fully customize the channel orders like we can in RGB.
There was a problem hiding this comment.
Hey there shnhrrsn. I ordered 200 FW1906 ICs and have designed a driver board utilizing this IC. My only problem is getting the firmware working on my ESP32. I have done the whole npm run but i always get an error. What steps did you do to get this working or do you have a firmware build you could share so i can test my boards. Cheers mate
There was a problem hiding this comment.
@oakey81 This thread is about line 235 in const.h, and color ordering of FW1906 chips.
please don't drift off-topic.
There was a problem hiding this comment.
Yeah mate i get that but I was just asking how he actually got the firmware compiled properly and working. As he stated he got it working. Anyway this whole thread jumps form one issue to the next if you scroll back a fair way.
There was a problem hiding this comment.
we call that "code review". In contrast to "support request" ;-)
|
@oakey81 first you need to install NodeJS 11.0+ globally. Then run https://kno.wled.ge/advanced/custom-features/#changing-web-ui
|
Thanks mate yeah it was the 'npm i' that i missed. sorted now and my setup works perfectly with all colours the right way round. I think some of the strips are not FW1906 IC and thats why the colour orders arent matching up. I actually ordered 200 x FW1906 ICs and have designed my own 5ch driver and RGBCW/WW downlight. PXL_20240119_013555174.TS.2.mp4 |
|
@oakey81 please update and rebase for 0_15 branch if you think it is viable solution worth to be included and make a PR. |
|
@blazoncek I am not sure what you mean by rebase as i am not a coder. My mate is very good at it and he saw what i was missing within about 30 sec. I just downloaded the FW1906 code off github and done the npm install and compiled to upload on my ESP32. And as i had designed my own 5ch driver board I followed the manufacturers data sheet and it runs perfectly. I do believe some of the RGBCCT strips might not be FW1906 ICs on them. But to answer your question. Yes I believe this is a viable PR for the next update. And also I see that the WS2805 RGBCCT IC has been released as a true 5ch chip not like the FW1906 6ch chip. So that could be another to add later aswell. |
|
I believe this was intended for @Robert-github-com . I already rebased and squashed down to a minimal set (see my earlier comments), so hopefully they're isn't much to change from there.
|
|
@deece Sorry I was just answering @blazoncek. As this is what he asked me. But as far as i can tell as a user of this it seems to work well with my hardware. Cheers for what u had done. |
|
Unless someone makes a PR for 0_15 branch we're not going anywhere with this (until 0_15 is merged to main which may be months away). As you can see from comments above the PR may require change in the very core of how WLED handles color and CCT so it will not be lightly taken. |
|
Hi guys, I'm seeing more and more of those chips out there, any news on official support? Thank you |
|
@blazoncek Is main sufficient, or do you want 0_15? |
|
0_15 |
|
I just received my FW1906 RGB+CTT ledstrip, and I notice that it's not actually implemented in wled, but it's a work in progress here. In the same time NeoPixelBus is now at v2.7.8, with support for rgbwww. Maybe wled should use this version directly instead of 2.7.6 Using a wemos mini-S2 board, I already had to compile from GIT sources. But I never had to select a branch at any moment. Is there a git command to select another branch ? Thank you for the work, and for your help. |
|
@blazoncek I've created a PR against 0-15 here: #3810 |
|
@blazoncek With the merge of #3810, this can also be closed. |
Add support for RgbwwColor and GRBCW strips that use FW1906 chips.
This change requires:
#3293
and
Makuna/NeoPixelBus@978b62f
The fix in NeoPixelBus is not in any release yet, and I don't know the way to force platformio to fetch specific commit, so I temporary changed lib_deps to my branch, if someone wants to test. This needs to be changed before merging.