ZeroOne3010 / yetanotherhueapi

A Java library for controlling Philips Hue lights. Available from the Maven Central.
MIT License
68 stars 21 forks source link

Possible NPE in isDaylightTime() #34

Closed fk0ff92 closed 3 years ago

fk0ff92 commented 3 years ago

Hey, it's me again, I went a little deeper into your code and wanted to understand the tests, so I also ran the test run on my bridge. On daylight sensor i get a null pointer exception and it dies. I don't understand what daylight sensors are supposed to be, only have hue motion light sensors and was wondering if it could be those. Did some debugging and my bridge says I have a daylight sensor but don't quite understand it. Can you tell me what falls under this kind of sensors. And well, in general I just wanted to warn about a null pointer exception that may not show up for you

public boolean isDaylightTime() { return readStateValue("daylight", Boolean.class); } This method causes the NPE.

ZeroOne3010 commented 3 years ago

Hello there! The daylight sensor is integrated into Hue motion sensors. Their official description says:

The Hue Motion sensor's integrated daylight sensor detects when there is still enough daylight so that lights do not turn on until you need them. If it's still too dark, adjust the light sensitivity of the sensor in the Philips Hue app to personalize when you'd like the motion sensor to activate.

I wonder if you'd be able to show me the raw JSON output of your bridge, when you fetch the sensors? In my case, for example, a daylight sensor looks like this:

    "1": {
        "state": {
            "daylight": false,
            "lastupdated": "2021-02-08T14:22:00"
        },
        "config": {
            "on": true,
            "configured": true,
            "sunriseoffset": 30,
            "sunsetoffset": -30
        },
        "name": "Daylight",
        "type": "Daylight",
        "modelid": "PHDL00",
        "manufacturername": "Signify Netherlands B.V.",
        "swversion": "1.0"
    }
fk0ff92 commented 3 years ago
{
    "name":"Daylight",
    "type":"Daylight",
    "state":{
        "daylight":null,
        "lastupdated":"none"
        },
    "swupdate":null,
    "config":{
        "on":true,"
        configured":false,
        "sunriseoffset":30,
        "sunsetoffset":-30
        }
        ,
    "modelid":"PHDL00",
    "manufacturername":"Signify Netherlands B.V.",
    "swversion":"1.0",
    "uniqueid":null,
    "recycle":null,
    "productname":null,
    "capabilities":null
}
fk0ff92 commented 3 years ago

This could be the reason, why there occurs a NPE, because "daylight" is null. I don't have configured my sensors with the Philipps hue app, because I use Apple HomeKit. It sounds like those daylight sensors are kind of "virtuell" sensors, which are configured by the Phillips hue app. The ambient sensors in the hue motion sensors are working fine.

I was surprised, that so much config overload exists in my hue raw output. There are so much scenes from 3rd party apps and so much rules I never configured.

fk0ff92 commented 3 years ago

I woke up this morning and did some research in the hue app. Under Settings > Advanced > Home Location, you can set your location, since I did this, the Daylight sensor works. By the way, the -30 and 30 come from the setting under Home Location "sunrise and sunset". So the sensor is virtual and is calculated from the geo data. If someone doesn't enter a home location because they may not use the app 100% like I do, then your api will always run into the NPE. Thanks for your advice.

ZeroOne3010 commented 3 years ago

Great, thanks for your research! In the JSON data you posted there's the part "configured": false which is kind of revealing: the sensor just wasn't configured. There's not much one can do with such a sensor, is there? I'm inclined to think that it probably shouldn't throw an NPE but some more sensible exception instead, explaining what went wrong.

ZeroOne3010 commented 3 years ago

...or then again I might just return false and log an error. 🤔 I checked the official docs, and they do not mention the configured property at all, so I'm hesitant to use it. Maybe it's nicer to not crash and to just document this behavior.

ZeroOne3010 commented 3 years ago

Also, the piece I copied from their documentation seems to be wrong in this context: clearly the daylight sensor we are talking about here is built into the Bridge, it's not a part of any motion sensor. The motion sensors have what are called ambient light sensors, but their marketing material refers to them as daylight sensors, which is very confusing. 🤦‍♂️

ZeroOne3010 commented 3 years ago

This fix has now been released in version 2.1.0. An uninitialized daylight sensor will not throw an NPE anymore but will instead just return false.