Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.48k stars 3.1k forks source link

sACN doesn't respect priority levels with multiple e1.31 senders. #768

Open urbanvisuals opened 4 years ago

urbanvisuals commented 4 years ago

It looks like when two e1.31 devices are sending on the network, the WLED devices receive them and try to use both values at the same time, causing the LEDs to flicker as they try to display conflicting data streams.

They should respond only to the lowest priority sender.

huggy-d1 commented 4 years ago

Shouldn’t they only receive (and process) info for the one universe they belong to?

urbanvisuals commented 4 years ago

The priority function is supposed to make it so that multiple devices can send on the same universe(s), but only the data from the sender with the higher priority will be processed and displayed. Priority values can be between 1 and 200, with lower numbers being higher priority.

Currently, WLED receives and processes data from two sources on the same universe which can lead to undesirable results. Of course, an easy fix is to disable the source you don’t want to display, but implementation of the priority function would be nice.

(Great work on WLED! I did a test yesterday with 15 universes on 15 different WLED devices and it worked well. Definitely need to ensure you have a good WiFi infrastructure, but that’s an issue with WiFi not WLED.)

pille commented 4 years ago

you're right, priority is not implemented. so my idea would be to start tracking the last priority per universe and then only update LEDs, when the current priority is less or equal. but how is this value reset? device reboot? timeouts?

Aircoookie commented 4 years ago

Yes, of course priority would be nice to have. I highly recommend against attempting to have multiple E1.31 sources active at the same time, this can quickly lead to denial of service. Furthermore, I fail to understand the use case of multiple senders simultaneously, so I believe it is too much of an edge case to be be worth the memory usage for tracking sender priorities and timeouts, but of course am open to discussion :)

urbanvisuals commented 4 years ago

sounds good to me! Maybe someone can put a note on the wiki to say it’s not implemented.

Aircoookie commented 4 years ago

Added it! https://github.com/Aircoookie/WLED/wiki/E1.31-DMX

stale[bot] commented 4 years ago

Hey! This issue has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs. Thank you for using WLED!

pille commented 4 years ago

still interested in implementing this. please reopen and assign.

mxklb commented 1 year ago

Simplest would be setting priority in WLED E1.31 sync settings UI (default: 0 = ignores prio => as is).

In e31 handling then just skip packages with priorities > this UI priority if != 0

Doing so, "lower" priority devices could be skipped easily by setting priority respectively in UI.

That would require minimum code changes. e131_packet_t should already receive uint8_t priority data.

@Aircoookie I could add this to my active DMX refactoring PR. Shall I? But: I don't have multiple e31 source devices to test it properly. Nevertheless the implementation as described should be really straight forward ..

May this solve this issue? Maybe @pille or @urbanvisuals is able to test it finally when added?

Note: This would affect all universes in the same manner!

mxklb commented 1 year ago

https://support.etcconnect.com/ETC/Networking/General/Difference_between_sACN_per-address_and_per-port_priority

It's called port priority. By the way it says lower priority value = lower priority, makes more sence, so correct is

In e131 handling skip packages with priorities < UI config priority (if != 0)

Not tested in detail but the following change should add this priority handling: https://github.com/mxklb/WLED/commit/2278fc4edb3245156ffb843c20c2b7277bf490b5

Here all universes have the same priority applied! But if senders define different ones there should be an effect ..

Nevertheless, the explicit skip priorities != UI config priority is even better - more selective, hmm TBD

Finally: As @Aircoookie said, it may be a rare usecase and slow down WLED devices, and should be avoided if possible. But in cases where it can't be guaranteed, this would somehow handle priority packages per device better than crossfire.

Port priority is about how to merge single DMX address values in case of "crossfire" (=mutiple senders). This is not what I try to achive here, so don't expect that. This is a minimum code change to deal with crossfire by E1.31 priority usage more restrictively. This could become useful in situations where WLED is or must be controlled by mutiple sACN senders.

mxklb commented 1 year ago

Not exactly as the standard says but skips packages with priority != UI config priority, if UI priority != 0

This is the most restrictive way to handle e131 priority in WLED (with minimum code changes): https://github.com/mxklb/WLED/tree/e131-prio

Could be discussed if != shall be replaced by < Note: < is closer to (but still not) the standard and less restrictive.

I'll try to test it when I find time & hard-/software to do so. I could make PR if wanted ..

mxklb commented 1 year ago

Implemented highest priority sender handling: Looking for someone to test this -> https://github.com/Aircoookie/WLED/pull/3052