michielpost / Q42.HueApi

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

Odd behavior when using EventStream #321

Open Christanoid opened 1 day ago

Christanoid commented 1 day ago

Hi, first up thanks again for the quick response in yesterdays ticket ;) Sadly I found another oddity today. I tried to rewrite my code away from polling light colors over to using the EventStream feature. While doing that I noticed some oddities. There are three scenarios:

  1. Using one LocalApiClient for EventStream subscription and sending update commands to the bridge.

    • The first update will send
    • The second update will throw this exception: System.ObjectDisposedException: Cannot access a disposed object. Object name: 'System.Net.Http.HttpClient'. at System.Net.Http.HttpClient.CheckRequestBeforeSend(HttpRequestMessage request) at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken) at HueApi.BaseHueApi.HuePutRequestAsync[D](String url, D data)
    • Additionally after the first send the stream does not appear to be receiving anymore
  2. Using two LocalApiClients, one for receiving the EventStream subscription, the other for sending updates.

    • Now I can send endless messages, it just keeps working
    • The EventStream however still stops receiving properly.
    • It does also not matter if the update is sent from me or from the hue app, I don't get updates anymore
  3. Not using the EventStream, instead just polling and updating with the same client.

    • It just works.

Just in case I did something wrong here are my receiving and sending functions:

private void OnEventStreamMessage(string bridgeIp, List<EventStreamResponse> events)
{
    Log.Information($"Update received from {bridgeIp}");
    foreach (var receivedEvent in events)
    {
        foreach (var data in receivedEvent.Data)
        {
            var matchingDevice = Devices.FirstOrDefault(x => x.Id == data.Owner?.Rid);
            if (matchingDevice != null)
            {
                ColorChangedArgs colorChangedArgs = new()
                {
                    DeviceGuid = matchingDevice.Id,
                };
                if (data.ExtensionData.TryGetValue("dimming", out var dimmingJson)
                    && dimmingJson.Deserialize<Dimming>() is Dimming dimming)
                {
                    colorChangedArgs.NormalizedBrightness = dimming.Brightness / 100.0;
                }
                if (data.ExtensionData.TryGetValue("color", out var colorJson)
                    && dimmingJson.Deserialize<Color>() is Color color)
                {
                    colorChangedArgs.NewColor = ToIColor(color);
                }
                if (colorChangedArgs.NewColor != null || colorChangedArgs.NormalizedBrightness != null)
                {
                    DeviceColorChanged?.Invoke(this, colorChangedArgs);
                }

                if (data.ExtensionData.TryGetValue("on", out var onJson)
                    && onJson.Deserialize<On>() is On on)
                {
                    DeviceIsOnChanged?.Invoke(this, (matchingDevice.Id, on.IsOn));
                }
            }
        }
    }
}

and for updating:

public async Task<bool> UpdateDevices(Dictionary<Guid, (IColor col, double brightness, bool isOn)> updateInstruction, TimeSpan? transitionTime)
{
    bool successful = true;
    try
    {
        foreach (var instruction in updateInstruction)
        {
            IColor.RGB rgb = instruction.Value.col.ToRGB();
            UpdateLight command = new();

            var lightHsv = rgb.ToHSV();
            var brightness = instruction.Value.brightness * 100.0;
            var maxedLightBrightness = new IColor.HSV(lightHsv.H, lightHsv.S, 1.0f).ToRGB();
            if (instruction.Value.isOn)
            {
                if (transitionTime.HasValue)
                {
                    command.Dynamics = new()
                    {
                        Duration = (int)transitionTime.Value.TotalMilliseconds,
                    };
                }
                command.SetColor(new RGBColor(maxedLightBrightness.R / 255f, maxedLightBrightness.G / 255f, maxedLightBrightness.B / 255f))
                    .SetBrightness(brightness)
                    .TurnOn();
            }
            else
                command.TurnOff();

            var id = Devices.FirstOrDefault(x => x.Id == instruction.Key);
            if (id != null)
            {
                var res = await _client.UpdateLightAsync(id.LightSId, command);
                successful = !res.HasErrors;
                res.LogErrors();
            }
        }
    }
    catch (Exception ex)
    {
        Log.Error(ex, "Updating philips hue devices failed.");
        successful = false;
    }
    return successful;
}

If you need any assistance please feel free to ask.

michielpost commented 1 day ago

I've seen this behavior before when the EventStream API was just introduced. There was an error after each event. I had some code in the project to catch any exception and reconnect. But with a firmware upgrade this went away. Are you on the latest bridge firmware? Because the EventStream should be able to receive multiple messages.

Christanoid commented 17 hours ago

I have checked on my Philips app and all bridges are up to date (1.67.1967054020). On that note, in case that is a problem, I have two bridges that I connect to at the same time.

michielpost commented 17 hours ago

Can you run the console sample? It only shows the eventstream changes. It's a basic scenario that should be able to show multiple updates. https://github.com/michielpost/Q42.HueApi/tree/master/src/HueApi.ConsoleSample

Christanoid commented 17 hours ago

Ah, I left out a detail in my original Ticket, sorry. The EventStream works for receiving every time, for as many messages as the bridge happens to send up until I use the attached Update once, after which it stops being able to receive

michielpost commented 12 hours ago

Ok, I'll make a test scenario later this week to check it out.