LedLayout is not considering Led Hardware setup

[Lord-Grey]

Unfortunately, the Led-Layout is not considering the maximum Leds available by the LedDevice.
In the code, even the number of Leds defined by the layout is considered valid when it is bigger than the number of Hardware Leds.

hyperion.ng/libsrc/hyperion/Hyperion.cpp

Line 219 in ed54554

_hwLedCount = qMax(unsigned(getSetting(settings::DEVICE).object()["hardwareLedCount"].toInt(getLedCount())), getLedCount());

This currently leads to the situation, that more Led updates are written than a Led-Device supports (i.e. unexpected behaviour) , as well as some other negative side effects.
A layout which has less or equal number of Leds as the Led-Device defined, is not a problem given that missing values are already filled.
The current way of handling layout and device independently is not very user friendly, as it does not provide a clear configuration framework (which avoids error scenarios).

[Lord-Grey]

Solution proposal:

Logical order to set up the Led hardware should be:

1. User configures Led Controller
   -> User Saves

2. System initialises the Led Device
   (LedDevice may resolve its "real" LedCount by itself, e.g. NanoLeaf,
   not sure, if the LedCount should be pushed to the screen or only shared with teh next reload of the page)

3. User configures Led Layout
   -> User Saves

4. System validates the Led Layout number & Led Device Number
   (System does an API call to the latest HW LedCount for device)
   4a. OK - Layout# <= Device#
   4b. Not OK - Layout# > Device,
   System provides Error message that LedLayout has more Leds defined than HW-LedCount

The Hyperion backend code will handle the "error" scenario too (I already started to look what is required to change), nevertheless the Led Administration User should be actively guided not running in an error scenario.
In addition, this would allow that LedDevices which resolve their LedNumber themselves can share it with the screens actively.

@brindosch I have seen that you have lately done changes in the UI code which I am not very familiar with.
Would it be possible/reasonable to do this additional validation check during the Led Layout save.
Note: The LedDevice Number is now exposed via the LedWrapper.getLedCount() method.
I will take care on the Hyperion.cpp updates....

Any other thoughts or ideas?

PS: In that context, it may be worth fixing #669.

[brindosch]

The hwLedCount exists for the reason to fill unused leds at a stripe with black. If the layout is larger it will be ignored. The ui might be confusing.

Questionable how much meaning the hwLed option should get, what it means, does it bring something to the table and how should we show that.

[Lord-Grey]

The hwLedCount exists for the reason to fill unused leds at a stripe with black. If the layout is larger it will be ignored. The ui might be confusing.

That would be aligned to my expectation that more configured Leds would be ignored and no output happens for those.
Unfortunately, with the introduction of the "qMax" logic, the number of output Leds will not ignore the additional Leds, but uses the layout number instead,
e.g. HW = 9 , Layout = 20, the Led Output string is 20 (and not 9, as I would have expected).

We can have the behaviour you well described, but that may result in the situation, that users are wondering why only half of the colours are shown. They may not identify easily that there is configuration mismatch.
Therefore, I put the proposal forward, that the user is actively informed about that mismatch.

[Lord-Grey]

@brindosch OK. I further thought about the topic....
Following the idea disentangling the components further and have clear encapsulation,
the UI piece is maybe the 2nd step to increase usability.

First step would be that a Led-Device is handling the Led output self-sufficient;
i.e. depending on the number of available Leds, stripping Led input or padding with black is done by the LedDevice class and not longer via Hyperion (given that the device "knows" its boundaries). This would include logging warnings and/or errors to support hyperion admin users to follow-up with "unexpected behavior"
2nd step may then be to further prevent mis-configurations through the UI as outlined above.

I will give it a try, unless serious concerns are raised....

[brindosch]

I also thought about a move to the LedDevice

[Lord-Grey]

Addressed by #1164