Throw exceptions while creating the instance

[henryruhs]

Hey,

I would love to have an connection exception to be thrown right after creating the instance.

def api_factory(ip : str) -> Any:
    api = None

    try:
        from pywizlight import wizlight
        from pywizlight.exceptions import WizLightError

        try:
            api = wizlight(ip)

            # hack start
            loop = get_loop() # instance of the loop
            builder = get_builder() # instace of pilot builder
            loop.run_until_complete(api.turn_on(builder()))
            # hack end

        except WizLightError:
            exit(wording.get('connection_no').format('WIZ LIGHT') + wording.get('exclamation_mark'))
        return api
    except ImportError:
        exit(wording.get('package_no').format('WIZ LIGHT') + wording.get('exclamation_mark'))

At the moment I need to turn the bulb on just to test the connection. See the hack start and hack end flags - that would be nice to be removable.

I integrated pywizlight to Chroma Feedback that let's you use your bulbs as a traffic light for CI builds. Still working on it but release of version 8.0.0 is planned this week.

[sbidy]

Hey, I avoided to have a connection exception on creation on the instance (create a connection to the bulb) because in some cases the bulb can be offline (switched off by an HW switch etc.). This should also be a problem to have a async call in init. But you can use an await api.updateState() to trigger an request to get the bulb state before you go into the loop. This should return a exception if the bulb is offline. I not you can use the await api.status() property to check if the bulb is on = True or off = False.

I'll do some other bug-fixes to the lib an will release a 0.4.2 with your pull merged.

[henryruhs]

Okay,

so what are you plans? I see that you tagged this issue for 0.4.2 and therefore will add my suggestion?

I mean both point of views are valid somehow. Having a false positive instance that can be used later once a bulb is online or the other way around of having only valid instances of online bulbs. In my opinion there is no big difference of a bulb being switched on and not being accessible or a bulb switched off - both are connected that are not working.

Beyond that, I can tell you that a couple of other libraries instantly throw execptions:

https://github.com/redaxmedia/chroma-feedback/blob/master/chroma_feedback/consumer/magic_hue/api.py#L21-L25 https://github.com/redaxmedia/chroma-feedback/blob/master/chroma_feedback/consumer/philips_hue/api.py#L21-L24 https://github.com/redaxmedia/chroma-feedback/blob/master/chroma_feedback/consumer/thingm_blink/api.py#L21-L24 https://github.com/redaxmedia/chroma-feedback/blob/master/chroma_feedback/consumer/xiaomi_yeelight/api.py#L21-L24

Whatsoever, thanks for the library and the fast release circles.

[sbidy]

I'll have a look into that. I'm also not happy with the call chain (call updateState and then status etc.). I think the main point is, that (as of my knowledge) some of the mentioned light systems are working with an REST API and/or a kind of bridge or gateway between the light and the controller. So in that case it makes sense to have a exception at the instancing because of a clear feedback from the bulb.

Thank you for your feedback - I'm looking forward to bringing a change into the 0.4.2. But as I mentioned, I have to check it with some tests and my Home Assistant integration.

[henryruhs]

@sbidy Good... just ping me and I can update my integration :-)

[sbidy]

Mh, I found one general issue ... how to throw an exception in the init if the connection will be async 😉 .. I don't want to have a bulb = await wizlight(ip) ... In my opinion that can be the main problem.

@redaxmedia do you have any idea to trigger a async function on class init?

EDIT: I made a quick review of the libs you are using .,.. and I don't saw a async implementation.

[henryruhs]

@sbidy What should I say? You entered the async / await hell and cannot leave it anymore^^

I found miyakogi/syncer but not sure you want to add another dependency.

Sometimes (mainly in test) we need to convert asynchronous functions to normal, synchronous functions and run them synchronously. It can be done by asyncio.get_event_loop().run_until_complete(), but it's quite long...

[sbidy]

0.4.2 released with the some additional bulbs. Please check for your needs. The exception on init will be postponed to later release.

[henryruhs]

@sbidy Hey, this is an hint about the wanted implementation. It would be nice to handle unsupported bulbs aswell... I am fine to use the loop for creating the instance aswell

def api_factory(ip : str) -> Any:
    api = None

    try:
        from pywizlight import wizlight
        from pywizlight.exceptions import WizLightConnectionError, WizLightTimeOutError, WizLightNotKnownBulb

        try:
            api = wizlight(ip)
        except (WizLightConnectionError, WizLightTimeOutError):
            exit(wording.get('connection_no').format('WIZ LIGHT') + wording.get('exclamation_mark'))
        except WizLightNotKnownBulb:
            exit(wording.get('support_no') + wording.get('exclamation_mark'))
        return api
    except ImportError:
        exit(wording.get('package_no').format('WIZ LIGHT') + wording.get('exclamation_mark'))

[sbidy]

Maybe the WizLightNotKnownBulb is miss leading because this exception will thrown if you call the getBulbtype. So in that case the lib can not determine what kind of bulb is connected and what features are supported. In that case it makes sense to throw. But you can use all other functions with the PilotBuilder to made calls to an "unknown" bulb.

Overall is the async a problem - in that case you should go with the loop. I'll ask my "python guru" and also made some research on that topic.

[henryruhs]

Oh, I see.

This brings me to the situation that I cannot warn users about unsupported bulbs just before my application is running and therefore I have to wrap larger parts inside the application in a try / catch.

[eibanez]

Could this behavior (throwing an exception if the bulb is not found) be controlled by a parameter? For instance:

api = wizlight(ip, test_connection=True)

As @sbidy mentioned, my lightbulbs are behind a switch but I'd rather catch connection errors when I can't reach the lightbulb than having to recreate the instance every time the lightbulb is turned on.

Having that parameter would allow both use cases.

[sbidy]

I'm digging into that. I have some possible solution for this but I have to check what will be the best at the end 😉 The "try-catch" is not the best way to solve the problem.

[henryruhs]

@sbidy A internal temporary solution is fine as long the public API does not change in the future.

[sbidy]

@redaxmedia I have added the connect_on_init property for init. But this is really rudimentary! So there are no high sophisticated retries etc, There will be only one UDP package send to the bulb which can be result in an false-positive because UDP is "fire and forget".

[henryruhs]

@sbidy I could not test this against a real bulb but this should be done right according your API changes: https://github.com/redaxmedia/chroma-feedback/commit/18f493b94d195496fe7004fe460e205969903720

It would be great you test a future release of Chroma Feedback and report potential bugs.

Thanks again