nmoschkin/wizlib

WiZ Light Bulb .NET API Library

I exist in this space.

Mastodon

wizlib's Issues

When querying pilot from several bulbs in quick succession, more often than not one bulb fails to respond

When querying pilot from several bulbs in quick succession, more often than not one bulb fails to respond.

If I loop through 7 bulbs to get their pilot, on a regular basis there is always one bulb that doesn't respond, and it is usually a different bulb each time that fails to respond.

Is this a problem with my router or the wiz bulbs and is there anything that can be done in code to resolve the issue?

I tried increasing the timeout which doesn't seem to help, reducing the delay between the loop in receiving the datagrams doesn't help either.

From what I can see the bulbs respond in bursts, some are slower than others and some respond so fast that the loop misses them, but when I tinker with the code it seems to get more reliable in some cases and then other times the same change starts to cause problems such as a backlog of tasks that are being awaited.

Name of bulb is mac address, is this correct?

When I call Bulb.ScanForBulbs() and I iterate through the bulbs, each bulb has a Name property, which is just the MAC address for the bulb. In the app I have, each bulb has been given a name. Is the name stored in the app only or when you name a bulb does it store the name of the bulb on the bulb itself? The reason I'm asking is I'm not sure if the Name property should contain the name given to each of the bulbs in the app or whether it is indeed correct to see the mac address.

MAC address: D8:A0:11:1C:21:D8
Name: D8:A0:11:1C:21:D8
Name in app: Festoon 1

Missing MessageboxEx Project

Suggestions

First off, I would like to say thanks for the hard work and effort you've put into the library.

Before moving on I would like to say that I'm not bashing your code in any way, had you not carried out the work you did I would not be able to control my Wiz lights as I would have found it hard to translate the Python and Java code, whereas you have managed to, which I consider an achievement within itself.

I'm just reviewing it and I have some suggestions:

• BulbMethod has a flag for methods that are inbound only, but has no flat to indicate outbound methods, I personally think the class should be split up so that inbound methods and outbound methods are defined into InboundBulbMethod and OutboundBulbMethod or something of that nature.
• You should allow for people to pass an option to specify the number of bulbs they have if known, as this will allow you to break from the where loop if all the bulbs expected have been found, making scanning a lot quicker.
• BulbCommand mixes both transformation and domain concerns, the properties should be split from the transformation methods into a separate class to reduce the complexity of the code.
• The MAC address conversion is pointless, there is no real reason to overcomplicate it in the way you have, you could have just used PhysicalAddress
• There is a bug somewhere in which if you know the MAC address of the bulb and have not performed a scan, the datagram is not sent to the bulb, I'm still looking into where this issue is being caused and will open a separate bug report for this.
• The scanning of bulbs and caching of bulbs found should be separated out, as the two are currently mashed together in the ScanForBulbs method along with various other concerns, again to reduce the complexity of the code.
• Your use of defining the constant DefaultPort and then the code setting the value of it to a variable int port = DefaultPort; seems pointless to me, you could have just made reference to the const.

I haven't finished going through everything yet, I only started doing so as I was having serious performance issues when trying to control 10 lights every 10ms, and I was trying to track down where they might be. I will add more suggestions as I work through the code and report back here.

Once again, a huge thanks for your efforts 👍

When an app external to WizLib changes the state of a bulb, the cached bulb state in WizLib is incorrect.

One of the bugs I was trying to figure out was why the WizLib library reports an incorrect colour state sometimes, then I realised that if the colour of a bulb is changed by some other software, the WizLib library doesn't know about this and returns what it has cached about the bulb.

This effectively makes the cache return wrong information.

I'm not convinced that you need to cache the bulb state and I'm of the opinion that the caching should be removed in entirety, therefore simplifying the code and removing the need for locks. The bulb itself will then be the single source of truth.