michielpost / Q42.HueApi

C# helper library to talk to the Philips Hue bridge
MIT License
411 stars 114 forks source link

Processing error with light disconnected #229

Closed Exergist closed 3 years ago

Exergist commented 3 years ago

I have 6 registered Hue lights, and let's say I set all of them to blue. If I disconnect one of them and then try to set all the lights to a given color (let's say red for this example) occasionally I receive an error within SendCommandAsync that looks like this:

image

For reference I'm setting the light colors like this (invoked with SetAllLightsColor):

image

When I handle the error (or if the error doesn't occur), all the lights that ARE connected change to red as expected. I can also see that the disconnected light on the Hue app looks like it changed to red (though it still shows as "unreachable" in the app). But if I then reconnect that light it immediately shines with the color blue. It looks like the Hue app updates to show the light as blue as soon as the UI refreshes.

The part that really has me stumped is why the error is intermittent. I suppose I could change color for individual lights in a loop and pre-query the light state, but I wanted to mention this behavior in case it should be looked at further.

michielpost commented 3 years ago

Can you provide the json that the Hue bridge is returning when you receive this error? Maybe the bridge returns this message in the json response.

Exergist commented 3 years ago

I'm novice when it comes to C# (call me a hobbyist I guess). Would you mind explaining how to obtain the JSON response? When the error is caught hueResults is null.

michielpost commented 3 years ago

You can use Fiddler to see the http calls between your app and the Hue bridge. https://www.telerik.com/fiddler

Exergist commented 3 years ago

I installed Fiddler on a 2nd PC (wifi) and ran my app on my 1st PC (lan cable). Unfortunately I didn't detect traffic when running my app (I'm probably doing something wrong with Fiddler).

However I have discovered that I can replicate the Source array was not long enough. Check srcIndex and length, and the array's lower bounds error with decent consistency via the following:

image

image

image

image

Here is the stack trace (if it helps?):

StackTrace = " at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)\r\n at System.Collections.Generic.List1.set_Capacity(Int32 value)\r\n at System.Collections.Generic.List1.En...

The exception occurrs at the line with SendCommandAsync. Note that this is with all lights connected, so the error (based on my experience) seems unrelated to light connection state (contrary to what I posted earlier). Also note that receiving this error is not 100% consistent. As you can see I've set my code up to basically ignore this error (aside from debug message), but this assumes there isn't something bad happening in the background as a result (and I haven't found a meaningful reason NOT to ignore the error since commands seem to execute correctly).

michielpost commented 3 years ago

Ok, I think I found the issue.

When you use SendCommandAsync with a list of lights, it will send individual state updates to each light. I think that if one of those updates fails, it will break out of a foreach loop unexpectelty and cause this error.

One workaround is to create a group from your list of lights. If you use the same list of lights often, it's better to create a group. This results in 1 API call to the bridge, everything will be faster.

I also did an update that will probably fix the problem. It's in version 3.15.8 and available on NuGet

Exergist commented 3 years ago

Is there a way to create a "temporary" group for executing these kinds of things quicker than what I'm doing above? It would have to be done at run-time since this will be distributed and light combinations will be unknown (plus making these temporary groups visible to the user would likely be undesirable).

I tried version 3.15.8 with no changes to my code and OCCASIONALLY I can trigger the following situation: image ...which results in an Index was outside the bounds of the array exception. Again, the actual light action seems to pass through and implement correctly even though this exception is encountered.

For what it's worth, I have more success generating the error at the beginning of a series of light commands.

michielpost commented 3 years ago

It's not possible to create temporary invisible groups. If you want to keep it dynamic, you don't have to change anything.

Maybe you can include the source code of the Q42.HueApi in your app and debug it to see the actual line of code in the source code where this error originates from.