Check for None instead of truthy value for light sensor

[fstancu]

Fix light sensor detection on models which return LigSen = 0.

[RobHofmann]

Looking for people who can test this one (I don't have this feature)

[RobHofmann]

Thanks for tracking this down. The root cause analysis is correct.

I confirmed the mechanism on my own unit. GreeGetValues turns a single-property reply into a scalar:

return result["dat"][0] if len(result["dat"]) == 1 else result["dat"]

So LigSen comes back as 0. That is falsy, so the check marks the sensor as absent. My test AC has this problem too.

I don't want to merge is not None as it stands, because it does not cover the other case. If the firmware does not know a property, it leaves that property out of both cols and dat. A single-property query then returns cols: [] and dat: [], and GreeGetValues returns an empty list. [] is not None is True.

Measured on my test unit:

Property	Known to firmware	dat	GreeGetValues returns
LigSen	yes	[0]	0
TemSen	yes	[64]	64
DwatSen	yes	[0]	0
AntiDirectBlow	no	[]	[]
OutEnvTem	no	[]	[]

So on a model that does not know LigSen, the check says the sensor is there. LigSen gets added to _optionsToFetch. The next poll then returns a dat list that is one item short. SetAcOptions maps values to keys by list position. Neither it nor async_update guards that, so every poll raises IndexError. The old truthy check was safe here by accident, because [] is falsy too.

About the evidence: I do not have a unit that lacks LigSen. That last part is a prediction from the measured protocol behaviour, not something I reproduced on hardware. If every model does carry the property, the failure is milder: a light sensor switch that shows up and does nothing on units without the hardware. Either way, the check tests the wrong thing.

One extra clause separates the two cases:

# An unsupported property comes back as an empty dat list. A supported one
# returns its value, which can legitimately be 0.
if light_sensor is not None and light_sensor != []:

I tested this against the component with a local fake device, in three cases: sensor present reporting 0, sensor present reporting 1, and property not supported at all. Current master misses the first. This PR breaks the third. The version above handles all three.

One more thing: DwatSen has the same problem. It returns 0 on my unit, so it is also detected as absent. I will open that separately instead of widening this PR.

On branches: this fix belongs on master, which is the line that ships today (4.0.5). The rewrite on 4.0-pre-release is not affected. It decides feature support by whether the property is present in the fetched state, not by its value. That presence-based approach is the right fix going forward.

[fstancu]

Hi @RobHofmann
Thanks for your efforts. Understood your point. Your proposed solution should work fine, but wouldn't it be cleaner to adapt GreeGetValues method to return None instead of [] ?
Other than that, just wanted to share that I just got the update for 4.0.5. Still has the issue, my light sensor is not detected, and the same patch fixes the issue.

[RobHofmann]

@fstancu Yes, agreed, that is cleaner. Returning None from GreeGetValues puts the "property unknown" case in one place instead of repeating a guard at every call site. Let's do it that way.

Two things to add.

The same fix is needed at all five detection sites, not just the light sensor. SyncState uses a truthy check for TemSen, AntiDirectBlow, LigSen, OutEnvTem and DwatSen. Any of them can legitimately return 0. On my unit DwatSen returns 0 and is mis-detected as absent, the same as your light sensor. So:

dat = result["dat"]
if not dat:
    return None
return dat[0] if len(dat) == 1 else dat

plus is not None at all five checks. That also covers the DwatSen problem I said I would file separately, so it does not need its own PR.

Digging further, the index-based mapping is the actual root cause. I measured this on my test unit. When a requested property is unknown, the device drops it from both cols and dat and returns a shorter list:

requested : ['Pow', 'Mod', 'SetTem', 'WdSpd', 'Air', 'AntiDirectBlow', 'Blo', 'Health', 'SwhSlp']
cols back : ['Pow', 'Mod', 'SetTem', 'WdSpd', 'Air',                   'Blo', 'Health', 'SwhSlp']
dat  back : [0, 1, 21, 1, 0, 1, 1, 0]

SetAcOptions maps values to keys by position in the requested list. So everything after the dropped property shifts by one: AntiDirectBlow gets Blo's value, Blo gets Health's, and the last key runs off the end with IndexError. async_update does not wrap SyncState, so that is a traceback on every poll.

This is the failure I predicted in my earlier comment. It is now measured, not predicted.

The device already tells us which properties it answered. Keying on cols instead of on position fixes both problems at once:

values = dict(zip(result["cols"], result["dat"]))

Feature detection then becomes "is the property in cols", which is the presence-based approach the 4.0-pre-release rewrite already uses.

My suggestion: keep this PR to the minimal None fix across all five checks, so your light sensor works in the next release. I will do the cols keying in a separate PR, since it touches the whole poll path.

On 4.0.5: correct, the fix is not in it. This PR is still open, so no release contains it yet.