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

feat: add compat flag to treat BinarySwitchSet and ThermostatModeSet command as a report #6415

Closed
wants to merge 0 commits into from

Conversation

XLDom
Copy link
Contributor

@XLDom XLDom commented Oct 17, 2023

Hello, i've try to do the job for BinarySwitchSet and ThermostatModeSet command as a report.
(exemple for SRT321 HRT4-ZW)

for this : #5937

I've tested : it's ok (value updated)

just one thing I don't understand: The commands arrive during their first update (I don't know why)
image
if you have un idea ?

and if you can push this update please.

Thx you.

@XLDom XLDom changed the title Add compat flag to treat BinarySwitchSet and ThermostatModeSet command as a report (exemple for SRT321 HRT4-ZW Add compat flag to treat BinarySwitchSet and ThermostatModeSet command as a report Oct 17, 2023
Copy link
Member

@AlCalzone AlCalzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have one compat flat, probably treatSetAsReport, which is an array of command names (e.g. ["BinarySwitchCCSet", "ThermostatModeCCSet"]), instead of potentially having to add one or more compat flags per command class to map.

You don't have to implement the mapping for all of them, but laying the foundation for this once would be good.

@AlCalzone AlCalzone changed the title Add compat flag to treat BinarySwitchSet and ThermostatModeSet command as a report feat: add compat flag to treat BinarySwitchSet and ThermostatModeSet command as a report Oct 17, 2023
@AlCalzone
Copy link
Member

just one thing I don't understand: The commands arrive during their first update (I don't know why)

This is because the devices don't support the CCs (allow being controlled), but they only control them in other devices. Unless specifically handled by a compat flag, Z-Wave JS only exposes values for supported CCs.

@XLDom
Copy link
Contributor Author

XLDom commented Oct 17, 2023

I'd prefer to have one compat flat, probably treatSetAsReport, which is an array of command names (e.g. ["BinarySwitchCCSet", "ThermostatModeCCSet"]), instead of potentially having to add one or more compat flags per command class to map.

You don't have to implement the mapping for all of them, but laying the foundation for this once would be good.

Yes, I also thought about this (use array or end with "Set"), but the problem is that BinarySwitchCCSet returns "currentValue", and ThermostatModeCCSet retun "mode". How can I know (in code) the command name to use (currentValue or mode) ?

And how to use on this code for example ? this._valueDB.setValue(BinarySwitchCCValues.currentValue.endpoint(command.endpointIndex), command.targetValue);

@AlCalzone
Copy link
Member

but the problem is that BinarySwitchCCSet returns "currentValue", and ThermostatModeCCSet retun "mode"

Yeah we still need CC-specific code to handle this like you did in Node.ts. That shouldn't prevent us from unifying the compat flag into one though. We'll stick with the two you already added to Node.ts and add others on demand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config ⚙ Configuration issues or updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants