Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get after Set captures transitional value rather than final value for some devices #452

Closed
RoboPhred opened this issue Dec 1, 2019 · 28 comments
Assignees

Comments

@RoboPhred
Copy link
Contributor

When changing the value of dimmers using the commandClass .set command, I get a value updated event for currentValue on a slightly-changed transitional brightness value as the dimmer smoothly transitions from off=>on or on=>off. I would expect it to receive the final value state, or keep updating until the final state is reached.

I have seen references to a property called targetValue, but that is always undefined for my devices. Is this something I should be seeing? Is it Z-Wave Plus?

I expect this might be a fluke with the type of wall module I use, so the question might be whether the zwave-js library should attempt to refresh the value after a delay, or that should be handled by the application code. I am fine handling it on my side if it is a legacy device issue that you do not want to support.

REPL example:

zwave.controller.nodes.get(15).commandClasses[38].get().then(console.log)
> { currentValue: 0, targetValue: undefined, duration: undefined }

zwave.controller.nodes.get(15).commandClasses[38].set(255)
[`value updated` called for node 15 with {commandClassName: "Multilevel Switch", propertyName: "currentValue", newValue: 2}]

// At this point, the light reaches its final brightness

zwave.controller.nodes.get(15).getValue({commandClass: 0x26, endpoint: 0, property: "currentValue"})
> 2

// The light now reports currentValue of 2 until I call Multilevel Switch .get()

zwave.controller.nodes.get(15).commandClasses[38].get().then(console.log)
> { currentValue: 97, targetValue: undefined, duration: undefined }
[`value updated` called for node 15 with {commandClassName: "Multilevel Switch", propertyName: "currentValue", newValue: 97}]

Device cache:

 "15": {
            "id": 15,
            "interviewStage": "Complete",
            "deviceClass": {
                "basic": 4,
                "generic": 17,
                "specific": 1
            },
            "isListening": true,
            "isFrequentListening": false,
            "isRouting": true,
            "maxBaudRate": 40000,
            "isSecure": false,
            "isBeaming": true,
            "version": 4,
            "commandClasses": {
                "0x26": {
                    "name": "Multilevel Switch",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 1,
                    "values": [
                        {
                            "endpoint": 0,
                            "property": "currentValue",
                            "value": 0
                        },
                        {
                            "endpoint": 0,
                            "property": "interviewComplete",
                            "value": true
                        }
                    ]
                },
                "0x27": {
                    "name": "All Switch",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 0
                },
                "0x70": {
                    "name": "Configuration",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 1,
                    "values": [
                        {
                            "endpoint": 0,
                            "property": 3,
                            "value": 0
                        },
                        {
                            "endpoint": 0,
                            "property": 4,
                            "value": 0
                        },
                        {
                            "endpoint": 0,
                            "property": 5,
                            "value": 1
                        },
                        {
                            "endpoint": 0,
                            "property": 7,
                            "value": 1
                        },
                        {
                            "endpoint": 0,
                            "property": 8,
                            "value": 3
                        },
                        {
                            "endpoint": 0,
                            "property": 9,
                            "value": 1
                        },
                        {
                            "endpoint": 0,
                            "property": 10,
                            "value": 3
                        },
                        {
                            "endpoint": 0,
                            "property": 11,
                            "value": 1
                        },
                        {
                            "endpoint": 0,
                            "property": 12,
                            "value": 3
                        },
                        {
                            "endpoint": 0,
                            "property": "interviewComplete",
                            "value": true
                        }
                    ],
                    "metadata": [
                        {
                            "endpoint": 0,
                            "property": 3,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 0,
                                "max": 255,
                                "default": 0,
                                "format": 1,
                                "allowManualEntry": false,
                                "states": {
                                    "0": "LED on when switch is OFF",
                                    "1": "LED on when switch is ON",
                                    "2": "LED always off"
                                },
                                "label": "Night Light",
                                "description": "Night Light",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 4,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 0,
                                "max": 255,
                                "default": 0,
                                "format": 1,
                                "allowManualEntry": false,
                                "states": {
                                    "0": "No",
                                    "1": "Yes"
                                },
                                "label": "Invert Switch",
                                "description": "Invert Switch",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 5,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 0,
                                "max": 255,
                                "default": 0,
                                "format": 1,
                                "allowManualEntry": false,
                                "states": {
                                    "0": "No",
                                    "1": "Yes"
                                },
                                "label": "Ignore Start-Level (Receiving)",
                                "description": "This dimmer will start dimming from its current level.",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 7,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 99,
                                "default": 1,
                                "format": 0,
                                "allowManualEntry": true,
                                "label": "On/Off Command Dim Step",
                                "description": "On/Off Command Dim Step",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 8,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 255,
                                "default": 3,
                                "format": 1,
                                "allowManualEntry": true,
                                "label": "On/Off Command Dim Rate",
                                "description": "On/Off Command Dim Rate",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 9,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 99,
                                "default": 1,
                                "format": 0,
                                "allowManualEntry": true,
                                "label": "Local Control Dim Step",
                                "description": "Local Control Dim Step",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 10,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 255,
                                "default": 3,
                                "format": 1,
                                "allowManualEntry": true,
                                "label": "Local Control Dim Rate",
                                "description": "Local Control Dim Rate",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 11,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 99,
                                "default": 1,
                                "format": 0,
                                "allowManualEntry": true,
                                "label": "ALL ON/ALL OFF Dim Step",
                                "description": "ALL ON/ALL OFF Dim Step",
                                "isFromConfig": true
                            }
                        },
                        {
                            "endpoint": 0,
                            "property": 12,
                            "metadata": {
                                "type": "number",
                                "readable": true,
                                "writeable": true,
                                "valueSize": 1,
                                "min": 1,
                                "max": 255,
                                "default": 3,
                                "format": 1,
                                "allowManualEntry": true,
                                "label": "ALL ON/ALL OFF Dim Rate",
                                "description": "ALL ON/ALL OFF Dim Rate",
                                "isFromConfig": true
                            }
                        }
                    ]
                },
                "0x72": {
                    "name": "Manufacturer Specific",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 1,
                    "values": [
                        {
                            "endpoint": 0,
                            "property": "manufacturerId",
                            "value": 99
                        },
                        {
                            "endpoint": 0,
                            "property": "productType",
                            "value": 18756
                        },
                        {
                            "endpoint": 0,
                            "property": "productId",
                            "value": 12337
                        },
                        {
                            "endpoint": 0,
                            "property": "interviewComplete",
                            "value": true
                        }
                    ]
                },
                "0x73": {
                    "name": "Powerlevel",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 0
                },
                "0x77": {
                    "name": "Node Naming and Location",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 0
                },
                "0x86": {
                    "name": "Version",
                    "isSupported": true,
                    "isControlled": false,
                    "version": 1,
                    "values": [
                        {
                            "endpoint": 0,
                            "property": "libraryType",
                            "value": 6
                        },
                        {
                            "endpoint": 0,
                            "property": "protocolVersion",
                            "value": "3.40"
                        },
                        {
                            "endpoint": 0,
                            "property": "firmwareVersions",
                            "value": [
                                "3.35"
                            ]
                        },
                        {
                            "endpoint": 0,
                            "property": "interviewComplete",
                            "value": true
                        }
                    ]
                }
            }
        },
@AlCalzone AlCalzone self-assigned this Dec 2, 2019
@AlCalzone
Copy link
Member

I'll need to read the specs to see if there's something about this case. Your device does not support the Association CC, so it won't report the continuous changes by itself.

@AlCalzone
Copy link
Member

This seems to be a larger problem in OpenZWave aswell. They solve it by regularly polling these kinds of devices. I'll need to figure out which interval to use and when to stop.

@AlCalzone AlCalzone modified the milestone: v2.2.0 Dec 8, 2019
@AlCalzone AlCalzone modified the milestone: 2.10.x Jan 13, 2020
@lukescott
Copy link

lukescott commented Feb 23, 2020

I have a similar issue with the HS-WD100+ as well. And interestingly enough, I only get a final value with either manual control or remote control - I forget which. But the HS-FC200+ only sends the final value. I don't have an HS-WD200+ to know if they updated it.

What I did to handle this is put a debounce on receiving updates before I send "the final value". When that expires I do a refresh at the end just in case it doesn't send a final value.

I've pleaded with HomeSeer to issue a firmware update for the final-value issue but they are slow and seem to be more interested in updating their 200 line. Kind of discouraging considering these devices are expensive.

@AlCalzone
Copy link
Member

What I did to handle this is put a debounce on receiving updates before I send "the final value". When that expires I do a refresh at the end just in case it doesn't send a final value.

What I can think of is:

  1. If the reported value is different than the target value, re-query the current value after a short time (debounced). Fast devices (like lights) should be handled by this.
  2. If the transition seems slow (like a roller shutter), extrapolate when the target value is expected to be reached. Schedule another query for that time.
  3. If the target value has been reached, report it. If there was a value change but the target hasn't been reached, back to 2. If there was no change, report that (but schedule one more query to be sure).

Thoughts?

@lukescott
Copy link

lukescott commented Feb 23, 2020

This is code from my system that currently uses OpenZWave. I have a generic multilevel class, and then I optionally use an extended version for the HD-WD100+ switches.

(I kind of consider this a device bug more than anything else. If you handled it, I would probably just have the user turn it on a case by case basis. Because even the same device might not have the same bug after a firmware update.)

import {ValueId} from "openzwave-shared"
import {Attribute} from "../../"
import {
	CC_SWITCH_MULTILEVEL,
	CommandClass,
	unhandledAttributeError,
} from "../cc"

export default class MultiLevel extends CommandClass {
	protected on = false
	protected level = 0

	declare(vi: ValueId, value: boolean | number | string): void {
		if (vi.class_id === CC_SWITCH_MULTILEVEL && vi.index === 0) {
			this.level = value as number
			this.on = this.level > 0
		}
	}

	update(vi: ValueId, value: boolean | number | string): void {
		if (vi.class_id === CC_SWITCH_MULTILEVEL && vi.index === 0) {
			this.updateLevel(value as number)
		}
	}

	input(attribute: Attribute): void {
		switch (attribute.type) {
			case "on":
				this.emitter.emit(
					"input",
					{
						node_id: this.nodeId,
						class_id: CC_SWITCH_MULTILEVEL,
						instance: 1,
						index: 0,
					},
					attribute.value ? 255 : 0
				)
				break
			case "level":
				this.emitter.emit(
					"input",
					{
						node_id: this.nodeId,
						class_id: CC_SWITCH_MULTILEVEL,
						instance: 1,
						index: 0,
					},
					Math.max(0, attribute.value - 1)
				)
				break
			default:
				throw unhandledAttributeError(attribute)
		}
	}

	attributes(): Attribute[] {
		return [
			{
				type: "on",
				value: this.on,
			},
			{
				type: "level",
				value: this.level + 1,
			},
		]
	}

	protected updateOn(value: boolean): void {
		if (this.on !== value) {
			this.on = value
			this.emitter.emit("change", this.nodeId, {
				type: "on",
				value: this.on,
			})
		}
	}

	protected updateLevel(value: number): void {
		if (this.level !== value) {
			this.level = value
			const on = value > 0
			if (on) {
				this.emitter.emit("change", this.nodeId, {
					type: "level",
					value: value + 1,
				})
			}
			this.updateOn(on)
		}
	}
}

And then I have an optional extended class I use:

import debounce from "lodash/debounce"
import {Attribute} from "../../../../attribute"
import Multilevel from "../../../cc/multiLevel"
import {CC_SWITCH_MULTILEVEL} from "../../../cc"

export default class MultilevelRefresh extends Multilevel {
	protected refresh = false

	init(): void {
		this.debounceUpdateLevel = debounce(
			this.debounceUpdateLevel.bind(this),
			500
		)
		this.refreshLevel = debounce(this.refreshLevel.bind(this), 250)
	}

	input(attribute: Attribute): void {
		switch (attribute.type) {
			case "on":
			case "level":
				this.refresh = true
				this.refreshLevel()
				break
		}
		super.input(attribute)
	}

	protected updateLevel(value: number): void {
		if (this.refresh) {
			if (this.level !== value) {
				this.level = value
				this.refreshLevel()
				this.debounceUpdateLevel(value)
			}
		} else {
			super.updateLevel(value)
		}
	}

	private debounceUpdateLevel(value: number): void {
		this.refresh = false
		this.level = -1
		this.updateLevel(value)
	}

	private refreshLevel(): void {
		this.emitter.emit("refresh", {
			node_id: this.nodeId,
			class_id: CC_SWITCH_MULTILEVEL,
			instance: 1,
			index: 0,
		})
	}
}

@kpine
Copy link
Contributor

kpine commented Jan 9, 2021

A bit of a warning with all the HA/OZW refugees incoming... this is one of the most commonly encountered issues I've seen with HA/OZW. The result is that the switch state in HA will appear off or on when it is physically in the opposite state. A search of the community forum shows dozens of threads of people complaining about the issue. It's come up in the HA Discord too many times to count as well.

OZW tried to solve this with the VerifyChanged device compatibility flag. This issue tracks the implementation of the value refresh that was never finished properly. There are problems with the unfinished implementation.

Consider the difference between devices that are Multilevel Switch V1-V3 (no target value reported) vs. Multilevel Switch V4+ (target value reported). If there is no target value in the report, are you able to reliably track the desired target (what happens if a new Set is sent in the meantime, or if the switch is toggled locally)? Or would you need to take the approach of OZW and refresh until the current value is stable? With the target in the report it's much more straightforward. The spec has a note about the current value in the report:

A controlling device MUST NOT assume that the Value is identical to a value previously issued with a Set
command when a transition has ended.

I'm not 100% of the interpretation, but I'm leaning towards it saying that you cannot compare the current value to any value from a Set to determine if a transition has ended.

It can also be an issue if you poll too quickly (250ms makes my GE's perform erratically, 10-12 GETs for a single change, but the refresh does get the final value eventually) and if toggle more than one light (or worse a roller-shade) at a time.

You might also consider the duration that is being set. If it's 0xFF (factory default) then you might expect it to change fairly quickly, except of course my GE switches take a few seconds to turn on or off. What if the user sets a duration that is 1 minute? Do you refresh after the 1 minute, or some interval in-between? Should the refresh interval scale by the duration? etc.

@towerhand
Copy link
Contributor

Just installed zjs2z and got hit with this issue on my GE dimmer in HA, for me it worked great with the VerifyChanged device compatibility flag in OZW like Kpine mentioned, you guys are doing a great job fixing the basis CC (double taps) for the same switches so hopefully this issue can be fixed in Zjs at some point too.

In the meanwhile, what polling interval do you guys recommend?

@chilicheech
Copy link
Contributor

Can we please change the title of this issue as this is not specific to the GE dimmers. I have some Zooz and GE dimmers and they all have the same behavior as described here. This is a more generic issue in that zjs is reporting a transitional value instead of the final value 😄

@AlCalzone AlCalzone changed the title GE in-wall dimmer module captures transitional value rather than final value when changing brightness Get after Set captures transitional value rather than final value for some devices Jan 16, 2021
@AlCalzone
Copy link
Member

Please test #1453

@towerhand
Copy link
Contributor

With the PR the behavior of the dimmers is still pretty funky in HA, but it does seem to help a lot at updating the state of the switch and IMO it’s a great starting point.

@AlCalzone
Copy link
Member

Thanks for the feedback. Can you elaborate what you mean with funky?

@kpine
Copy link
Contributor

kpine commented Jan 21, 2021

  1. toggle switch on in HA
  2. switch widget turns on
  3. after a short time, switch widget turns off
  4. after ~5 seconds from 1, switch widget turns back on

It's basically an HA UI issue. The current value does not change until 5 seconds later, so I guess it's updating state in-between that time. This has always been the case with z-wave and HA, even with the original refresh_value hack for ozw 1.4.

@towerhand
Copy link
Contributor

towerhand commented Jan 21, 2021

When the light is on and you turn it off with HA, the toggle turn off and back on then a little later it turns back off to show the final state. Makes sense?

Edit: Isn’t the UI mirroring what happening in the backend, so in the example I mentioned, changing the state of the dimmer 3 times instead of 1?

@AlCalzone
Copy link
Member

Okay that seems like HA thinks switching didn't work until the update comes.
Don't think there's much I can do, since even if we know the transition time instead of guessing 5 seconds, it might take longer than the delay after which HA assumes it didn't work.

@kpine
Copy link
Contributor

kpine commented Jan 21, 2021

Edit: Isn’t the UI mirroring what happening in the backend, so in the example I mentioned, changing the state of the dimmer 3 times instead of 1?

Not sure what you mean. This PR delays the Get after Set by 5 seconds (or some variation based on the duration). That means there is a single Set, and one Get.

@towerhand
Copy link
Contributor

Edit: Isn’t the UI mirroring what happening in the backend, so in the example I mentioned, changing the state of the dimmer 3 times instead of 1?

Not sure what you mean. This PR delays the Get after Set by 5 seconds (or some variation based on the duration). That means there is a single Set, and one Get.

@kpine I think Al explained better.

I know we talked about this issue in discord but since no one had reported back here I figured to do so.

@kpine
Copy link
Contributor

kpine commented Jan 21, 2021

Adding to my previous comment, the implementation of the refresh_value in the old zwave integration did have a different behavior. That was Set -> Get -> +N seconds Get, so the switch widget could reflect the actual state in the backend.

My understanding of this PR, correct me if I'm wrong, was that there was no immediate Get, just a Set -> +N seconds Get, and so something in HA is reverting the switch state until it gets the updated value.

@towerhand
Copy link
Contributor

Okay that seems like HA thinks switching didn't work until the update comes.
Don't think there's much I can do, since even if we know the transition time instead of guessing 5 seconds, it might take longer than the delay after which HA assumes it didn't work.

What would be the next step? Should we create an issue in Hass or zjs2m/zjs_server?

@AlCalzone
Copy link
Member

I'm not sure if this is easy to solve. We could cheat by immediately reporting the new "currentValue", but that is likely breaking something else. For now, I'd create an issue in Hass to see if that behavior can be improved.

Closing since the zwave-js side is done.

@blhoward2
Copy link
Collaborator

@AlCalzone Just to add to this...what if the api changed the current value immediately and only updated it to a transitional value if the device failed to reach the targetValue? Is there a reason why it needs to read the transitional value on the way down?

@AlCalzone
Copy link
Member

Is there a reason why it needs to read the transitional value on the way down?

It does not care about transitional values, that's just a symptom. Currently the code refreshes the current value from the node after setting the target value, just in case the device does not report updates itself.
That used to happen immediately, which can capture transitional or old values. If the device then sends no update on its own we were stuck with the wrong value.
Now the poll is delated until after the transition time (or 5 seconds if there is none), which should solve most of these problems.

@blhoward2 blhoward2 reopened this Jan 28, 2021
@chilicheech
Copy link
Contributor

This is working much better now. Thanks 😄

@blhoward2
Copy link
Collaborator

Are you seeing your switch turn back on in the ui? Which parts are working better?

@chilicheech
Copy link
Contributor

so, when i turn a dimmer switch from off to on in the ha ui the switch will move to on, then like a second later it moves back to off, and eventually it moves back to on. and it no longer captures a transitional value, but captures the final value.

this is the behavior i used to get with OZW. and it works pretty well for my purposes. it would be really nice for the switch to not flop back and forth, but this works great for me 😄

@AlCalzone
Copy link
Member

AlCalzone commented Jan 28, 2021

I've discussed with @blhoward2 and the HA devs - I'm going to immediately update the currentValue with the targetValue, which should avoid the flickering. If the device did not accept the command, the value will be updated a couple of seconds later.

@chilicheech
Copy link
Contributor

that sounds like a good compromise

@towerhand
Copy link
Contributor

Sounds great, thank you.

@nevir
Copy link

nevir commented Jan 29, 2021

relevant PR for those following along: #1522

mdoggydog pushed a commit to mdoggydog/node-zwave-js that referenced this issue Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants