ddp from openRGB to wled stopped working in some later v16 nightly version

[greekthano]

EDIT: updating typos and misstates

What happened?

one of the later v16 nightly versions broke ability to control wled device using openRGB ddp. i had been updating nightly v16 versions over the last few months without issue and i only recently observed this problem, so it must be in one of the later v16 nightly but unfortunately i cant tell which one.
rolling back to v0.15.4 resolves the problem.
switching from DDP protocol to E1.31 in openRGB is a workaround unless you exceed max leds supported by E1.31

very basic config with 2 outputs of same type, attached configs

wled_cfg_stripscase.json

wled_cfg_stripscase.json

To Reproduce Bug

install latest pawnio version of openrgb and add manual ddp device and point it to wled esp32 controller. this works in v0.15.4 but doesnt work in v16 although it did work in v16 one of the earlier nightly versions.

Expected Behavior

ddp should continue working

Install Method

Binary from WLED.me

What version of WLED?

v16 beta

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

Anything else?

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[DedeHai]

please share the exact DDP header that is output by openRGB. Either they violate the protocol or WLED is not properly handling all possible valid formats.

[greekthano]

im sorry that I dont understand what it means to share "the exact DDP header that is output by openRGB" but if you can give me instruction then I will do it.
also I believe i misstated the problem so i'm updating the report. specifically, the problem is happening when using openRGB to control wled device. openRGB works to control wled device when wled is v15.4. openRGB fails to control wled device when wled is v16

[DedeHai]

either need to search the source code or use a network packet sniffer. I never did this so can't tell you exactly how. DDP uses a 10byte (or 14byte) header with info about the data that follows. if that header has incorrect information, WLED rejects the packet.

[greekthano]

okay well without instruction then i cannot give you that information. should i just close this issue or something else?

[DedeHai]

found it:
https://gitlab.com/CalcProgrammer1/OpenRGB/-/blob/master/Controllers/DDPController/DDPController.cpp?ref_type=heads#L225

they do send "undefined LED type" which WLED now rejects as undefined can not be parsed.

edit: the protocol must specify what type of data it is, RGB, RGBW.
@softhack007 should we allow "undefined" and fallback to RGB for the sake of compatibility?

edit2:
its not only "undefined", it is datatype=1, which is just plain wrong:

byte  2:    data type
            set to zero if not used or undefined, otherwise:
            bits: C R TTT SSS
             C is 0 for standard types or 1 for Customer defined
             R is reserved and should be 0.
             TTT is data type 
              000 = undefined
              001 = RGB
              010 = HSL
              011 = RGBW
              100 = grayscale
             SSS is size in bits per pixel element (like just R or G or B data)
              0=undefined, 1=1, 2=4, 3=8, 4=16, 5=24, 6=32

"0x01" means "standard type" with an "undefined data type" and "1 bit per color" , which is not supported by WLED. This question on why they use this incorrect format needs to go to openRGB devs.

[greekthano]

thanks for this information although I dont fully understand it..

if it is decided that no wled change will be done, could you please share precise wording that I will then send to openRGB and ask them for a change ?

otherwise, if change in wled, i could think of looking at the first output in order to determine what type to use. maybe its not desirable but seems like it could work.. although simply reverting back to allowing undefined would also work in this usecase - was that causing problems?

[DedeHai]

simply reverting back to allowing undefined would also work in this usecase - was that causing problems?

yes, it was causing problems, that's why we decided to change it. A protocol is well defined, if unsupported data is sent, it should not just be accepted.

[DedeHai]

we need to check this more thoroughly and then decide what to do. But ultimately openRGB may need to fix the bug on their end.

[greekthano]

EDIT: i reported the issue to openRGB but did so in the wrong place resulting in closed issue. still trying to understand how to properly raise this with openRGB...

[DedeHai]

looking into this in more detail.
@netmindz the DDP protocol DOES have 2bits to ID the version but it is not being used, it is and always has been "version 1" even though the header definition has changed.
Here is what I have done (PR almost ready):

• not reject any malformed dataType in the header but always assume RGB24 except if type=RGBW
• always assumes 8bit per color channel
• instead, the packet size is checked against the required data length for the assumed format, if it is too small, it is rejected (OOB read safety)

I also found that for artnet and E1.31 packets, no checks against incomplete or malformed packets was done. I added checks to make sure that no OOB reads are possible anymore.