Correct illuminance formula and treat special cases

[andrei-lazarov]

1. reverted Lx values calculated incorrectly in Z2M #11866

2. fixes: Illuminance Measurement Attributes (Value 0 and 0xffff) zigbee2mqtt#31431

[Koenkk]

Thanks, I already reverted 1

[andrei-lazarov]

@MikaFromTheRoof I still don't fully understand this, but I added 0 and 0xffff like in ZHA (as suggested in the linked issue)

[Nerivec]

Should use 0x0000 and 0xffff (it makes it easier to identify when searching code).
Should also move the luxScale const outside the illuminance fn for good measure.

Though can't return undefined here; will bite the assertNumber in numeric.
ZHC is tricky to use with anything undefined since it doesn't use TS strict (lets stuff pass through that it shouldn't, but it's a massive refactor to enforce strict).

[Nerivec]

This cluster is a little strange, usually, the attr has a range that is outside the special cases (which we handle in ZH), but for some reason, this one has full range + special cases. And it was updated in latest version of the spec, so, for some reason, this does not appear to be a mistake (or it's just a backwards compat issue now...)...
We could update the cluster in ZH to use max: 0xfffe, but with above, not sure what's the right call here...

The ZH layer maps non-values to Number.NaN (and vice-versa), but for now very few converters actually detect this and handle it.
Also, in general, support for special cases is tricky with Z2M, because we don't "know" the format the other end (frontend, HA, node red, etc.) requires to signal invalid/impossible/etc..

[MikaFromTheRoof]

@MikaFromTheRoof I still don't fully understand this, but I added 0 and 0xffff like in ZHA (as suggested in the linked issue)

Would have also been my suggestion to look how ZHA does it. But I am not deep enough into the ZHC to be able to assess, if it is working for Z2M this way.

Thanks, @Nerivec for looking into that!

I am more on the Zigbee device building side of that whole process and only did write some external converters for my DIY devices. The way I understand the approach of ZHA is, that you will only get 0 lx, if the raw illuminance reported by the device is exactly zero. As soon as it is above that, the illuminance will be interpreted as 1 lx, even if it mathematically should still be rounded down to 0 lx, right?
But that would be fine, because I could define a threshold for the raw illuminance in my devices firmware for all the values, that would resolve to 0 lx and then just always report 0, when raw illuminance falls in that range.

I am using simple photoresistors, that normally never really read the value 0, even if it is in pitch darkness. I could imagine, that it might be the same for most of the off-the-shelf devices. So even with that fix, I think most of the devices might still never reach the raw illuminance value of 0. But at least it would be possible now to report 0 lx without writing a custom external converter.

[Nerivec]

ZHA has a completely different architecture, and they only have to work with HA.

The approach with that attribute isn't that of ZHA, it's that of the Zigbee specification.

Thing is, in other cases, when there are special cases like this, the attribute Range is actually excluding them, but not here, which makes it a little weird.

The zero return if zero in this PR is correct.
As for the invalid... @Koenkk do you think we should move the max to 0xfffe in ZH for the attribute, so it is mapped to NaN (as it should be really, just that weird table in the spec...)?
We probably should take a closer look at how NaN is handled in ZHC in general (lots of attributes do that) and how that translates to the MQTT payload being sent. Make sure the behavior is aligned across the board.

[Koenkk]

Reading the spec 0xfffe should be indeed mapped to NaN, so I think if (result === 0xffff) return undefined; has to be changed to if (result === Number.NaN) return undefined;

[Nerivec]

Can't return undefined in ZHC though, it will fail (see #11868 (comment)).
Also have to adjust the attribute in ZH to have max: 0xfffe.

[andrei-lazarov]

So should we return something like -1 to signal invalid? 🤔

[Nerivec]

-1 has proven to have weird reactions in the past. Some automation systems don't treat it properly, and it can cascade.

I think undefined (null json) is the way to go, but need to fix the whole chain to support it properly.
Problem is, numeric has a boatload of uses, so, the change is massive in effect (i.e. should be correct, but need to be sure).
Then the spot in the fn can just do:

if (Number.isNaN(result) {
    return undefined;
}

If you search for isNaN in ZHC, you will find places that already "somewhat" handle non-values, but I think in most cases, it just doesn't return.