1.6 RC1 brightness off by value of 1 sometimes

[Skinah]

I have only tested for this in 1.6 RC1 firmware but when I send {"brightness":50} the update and state report the brightness has changed to 51. If I send {"brightness":51} the values all match correctly. Been testing this with mosquitto_pub commands with openhab turned off to ensure this is not a bug on my end.

milight/commands/0xE6C/rgb_cct/4 {"brightness":50}
milight/updates/0xE6C/rgb_cct/4 {"brightness":51}
milight/states/0xE6C/rgb_cct/4 {"state":"ON","brightness":51,"bulb_mode":"white","color_temp":153}

[sidoh]

I'm pretty sure most devices only support brightness in increments of 10 or so, so I'm not sure if this makes a practical difference. Have you been able to tell if 50 and 51 are different?

If I had to guess, I'd say this is just harmless collisions in unit conversion. brightness is a value in the range [0, 255], but internally it's projected down to [0, 100]. It's probably re-projected to [0, 255] here.

[sidoh]

(50*(100/255)) = 19.6078431373

which is probably getting rounded up to 20. then:

(20*(255/100)) = 51

but these both get mapped to the same value in the range that's used by the RF protocol, so I don't think it'd make any difference except that the behavior you're pointing out is a little unexpected.

[Skinah]

It is more about the extra back and forth and bandwidth used when openhab tells milight to use 50, the milight tells openhab it is at 51, so openhab changes and then publishes to mqtt it is now 51, then milight reports it is 51 and the madness stops thankfully. Openhab can also have the value printed on a control for you to see the value, when doing automated fades you see the numbers not climbing smoothly. I don't care about the brightness because as you say it would be hard to pick it unless you watch an automated fade, in which case < 8bit resolution is not enough for smooth changes.

I have the same issue as Openhab only has 0-100 controls so having the brightness in 0-255 is making me do the same conversions. Since we both use 0-100 can we have the {"level"=50} added to the state this way it passes the values over without any conversions?

{"state":"ON","brightness":51,"level":20,"bulb_mode":"white","color_temp":153}

[sidoh]

Got it, this makes sense.

Yes, I feel like having an option to expose a [0, 100] value is the correct solution. As far as I know, HomeAssistant's mqtt_json component only supports a brightness value in the [0, 255] range, which is why the [0, 100] range is up-scaled to [0, 255].

#103 is exactly what we're looking for here, I think. It's certainly doable, but it's not a super trivial feature.

Can you help me understand how big of an issue this is for you? The options I'm weighing are delaying the 1.6 release in order to add this feature, or doing a patch release or adding it as a feature to 1.7. I wouldn't want to bless 1.6 as a stable release if this causes significant issues.

[Skinah]

It is not a big deal in white mode, but medium priority for colour modes. This can create 7 different back and forths until a value matches and both ends agree as the brightness effects the hue and saturation. Last night I did some debugging and noticed something like this happens.

I send correct format for JSON {status,level, and R G B codes}
If light is off, only the status is read and the globe reports the new state which then halts the further commands from working.
If the light is already on, the level and rgb get updated until the hub reports back the level is off by 1.
The RGB code is now different so this goes back and forth many times until the numbers all match up.

This behaviour could be helped by a feature that processes ALL COMMANDS in the JSON before sending a new state to MQTT. The espmh currently will send a new status after only processing a single command in the string of commands. This premature status update causes issues far more then a value off by one.

[sidoh]

Delaying a state update until all commands have been processed makes sense. Let me think about how to best support that. It's a little tricky because of the way the code is structured, but I agree this is not ideal behavior.

[sidoh]

This is fixed in 1.6.0-rc3. Please feel free to re-open if it doesn't solve the problem.

[Skinah]

Big THANKS for the changes they are working great.