TS110E on_off fix add logic to use the correct state for multi switches#12657
TS110E on_off fix add logic to use the correct state for multi switches#12657phyzical wants to merge 15 commits into
Conversation
|
ive also decided to add logic to only perform this when the brightness is unchanged given the |
|
Hmm not sure why the CI doesnt like it i am able to confirm this works via an external converter |
Fix brightness lookups, as max brightness was always 1 behind max causing the force on work around never to fire
|
Added some more logic to handle the clamping to reference the min max instead of a static 254, as it resulted in |
|
had an issue where i was still getting precision errors i.e 31 setting vs 30 in state so instead keep the old logic of hardcoded 254 and allow a variance of 1 as a |
| const brightnessUnchanged = Math.abs(utils.mapNumberRange(brightness, 0, 254, 0, 254) - state[brightnessKey]) <= 1; | ||
|
|
||
| // if the brightness is unchanged then we need to force it on due to weirdness with moveToLevelTuya | ||
| if (state[stateKey] === "OFF" && brightnessUnchanged) { |
There was a problem hiding this comment.
Should we check if the target state is ON here? Because what if {state: OFF} is recevied?
There was a problem hiding this comment.
Do you mean in the message payload?
There was a problem hiding this comment.
I did have this thought, but I doubt anyone will send a brightness command with an off right?
There was a problem hiding this comment.
Yes indeed, it might depends on e.g. HA automations used by people so would say it's a valid use-case.
There was a problem hiding this comment.
oh actually i just realised that will be handled by the early return above https://github.com/Koenkk/zigbee-herdsman-converters/pull/12657/changes#diff-d45f20d30a43dc3e172b16bd55872492f4867c8831c5e03a8be9be65f1cd72d5R3314
did we instead want to handle brightness first and then perform an on/off
and avoid all the early returns?
nah i think keep it as it is in this regard what do you think?
|
Okay so i missunderstood some of the mechanisms going on, turns out so we need to simply always send the on always if its off (maybe this is what you were implying yesterday?) So tldr the issue was simply that we did not check the correct state before telling the light to turn on which is what caused the problem in state. and the bug around same brightness not turning on the light should naturally be resolved by the fact we turn it on if its off sadly this does result in twice as many events firing out to the network but at this time i couldnt get an all in one similar to the |
will review again but i think the state key being returned is a mistake and there is translation occuring down the line and the only issue was the state check to turn on Co-authored-by: Jack <5182053+phyzical@users.noreply.github.com>
Co-authored-by: Jack <5182053+phyzical@users.noreply.github.com>
|
i will reping you when i feel confident, something wasnt right i think this is it but will retest |
resolves Koenkk/zigbee2mqtt#32246
Basically when you send a same brightness command
moveToLevelTuyawill only turn on the device if the brightness changes, this is due to the helper assuming the state is always state but with multi switches we need to handlestate_l1for examplethis workaround just does a lookup, if we have a
state_blah then assume we need it to work like thatI'm unsure if other parts of the device suffer a similar fate assuming that it should be i.e
brightnessvsbrightness_l1but it may?i assume the reason device would still turn on when the brightness changes is a side effect of the device itself and how it handles
moveToLevelTuya