kpsuperplane / homebridge-wiz-lan

Control Wiz lights over LAN
Apache License 2.0
115 stars 37 forks source link

Questions about the dimming.ts file #92

Open MoTechnicalities opened 2 years ago

MoTechnicalities commented 2 years ago

@kpsuperplane

I am trying to help debug. RE: https://github.com/kpsuperplane/homebridge-wiz-lan/blob/master/src/accessories/WizBulb/characteristics/dimming.ts I plugged the essence of the following functions from your dimming.ts file into an excel spreadsheet and I think they produce inappropriate results for the Characteristic.Brightness.

Line 16: return Number(Math.round((Math.max(10, Number(pilot.dimming)) - 100) * 1.1 + 100));

Line 44: { dimming: Math.round((Math.max(1, Number(newValue)) + 10) / 1.1) },

The Characteristic.Brightness is supposed to be an integer value from 0 to 100 with a minimum step value of 1. In the current setup, neither Line 16 nor Line 44 allow for a value of zero. see https://developers.homebridge.io/#/characteristic/Brightness

MoTechnicalities commented 2 years ago

I would recommend using Math.floor() function to derive the needed integer value, as it returns the largest integer less than or equal to a given number.

Using Math.floor(), an input Number range of 0 <--> 100.99 would yield output integer values of 0 <--> 100.

xmanu commented 2 years ago

The issue you describe in #88 seems to stem from something different, as NaN is passed as brightness (not 0), at least that is what the log message complains about.

The floor function would not suffice for your proposed fix. The issue I solved initially is that Wiz Lights don't accept values from 1 to 9 as brightness. So the code you mentioned above will "normalize" brightness from 1 to 100 to the range of 10 to 100 accepted by the wiz lights. This code was written before the scene support was added. Without scene support I can set up automations that turn off lights without issues. I guess that there is some interaction between the new scene feature and automations that makes this problematic.

I agree though that the above code should accept 0 as a brightness value to be conformant with the homebridge documentation. When I find the time I will update the code to also accept 0. Note though, that I cannot currently test this in conjunction with the scene feature, as my homebridge setup cannot enable the feature without breaking my setup (I have lots of grouped lights that by documentation don't work with scene support).

MoTechnicalities commented 2 years ago

@xmanu Hi, please see #93 to see if that would work. Thanks.

MoTechnicalities commented 2 years ago

@xmanu

The issue you describe in #88 seems to stem from something different, as NaN is passed as brightness (not 0), at least that is what the log message complains about.

You are correct. I tried adjusting the function in the above lines on my own system. I found specifically that NaN is passed when getPilot fails to receive dimmer information from a light. ( See my logs posted in Issue #84 ).

Additionally, adjusting the above functions has no effect on whatever bug is preventing us from setting a light to 0% dimming when attempting to create a scene ( Per Issue #88 ).

Thanks for your response.

kpsuperplane commented 2 years ago

Should be fixed in 3.2.1. Can anyone verify?

MoTechnicalities commented 2 years ago

I said a lot but deleted it because I was wrong on some points. I will continue to experiment. But, the problem is not fixed.

catongates commented 1 year ago

I thought that pr-99 addressed this issue, but as of today, with v3.2.5 of the plugin, I'm still seeing quite a lot of this in my logs:

[1/14/2023, 12:21:00 PM] [homebridge-wiz-lan] This plugin generated a warning from the characteristic 'Brightness': characteristic value expected valid finite number and received "NaN" (number). See https://homebridge.io/w/JtMGR for more info.

I assume I'm not alone in this.