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

fix: barrier operator hass replace barrier states with integers #556

Merged
merged 7 commits into from
Feb 22, 2021

Conversation

varet80
Copy link
Collaborator

@varet80 varet80 commented Feb 11, 2021

This PR tries to fix #555
We need to test it before implementing it
@ratsputin can you try it?

@varet80 varet80 changed the title fix : barrier operator hass -replace barrier states with integers fix: barrier operator hass replace barrier states with integers Feb 11, 2021
@coveralls
Copy link

coveralls commented Feb 11, 2021

Pull Request Test Coverage Report for Build 588667991

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 19.388%

Totals Coverage Status
Change from base Build 588628128: 0.0%
Covered Lines: 2027
Relevant Lines: 10724

💛 - Coveralls

robertsLando
robertsLando previously approved these changes Feb 11, 2021
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

From a code side, LGTM

@robertsLando
Copy link
Member

@billiaz can we merge this?

@aretakisv
Copy link
Contributor

no, there is a small issue I want to discuss with reporter of this! as it is a state we need to cover.
I will try to fix this too

@varet80
Copy link
Collaborator Author

varet80 commented Feb 17, 2021

@robertsLando merged fresh master branch to this project

@robertsLando
Copy link
Member

@billiaz Is this ready to be merged?

@varet80
Copy link
Collaborator Author

varet80 commented Feb 17, 2021

I think yes. But I want to be sure for one details about the unknown state. Looking on possible "if" statement
I need 5 more minutes :)

@varet80
Copy link
Collaborator Author

varet80 commented Feb 17, 2021

@robertsLando it is good for now

@robertsLando
Copy link
Member

Tell me when it's ready to merge so :)

@robertsLando
Copy link
Member

@billiaz ?

@varet80
Copy link
Collaborator Author

varet80 commented Feb 22, 2021

Updated from Master, ready to be merged back to master after this, if no problems found in linting

@robertsLando robertsLando merged commit 11e1bde into master Feb 22, 2021
@robertsLando robertsLando deleted the fix-555 branch February 22, 2021 09:47
@kpine
Copy link
Contributor

kpine commented Feb 23, 2021

How does the payload_stop command work?

The Barrier Operator CC Set command does not have a stop value, only open and close. My understanding was that the only way to stop a cover is issuing an open command when it's in the closing state.

@robertsLando
Copy link
Member

@billiaz ?

@varet80
Copy link
Collaborator Author

varet80 commented Feb 23, 2021

Hmm that becomes tricky.... @robertsLando @kpine

One thought @robertsLando is to program a function in Zwavejs2mqtt, if this is the state and a stop command comes in, to execute the proper in driver...
I need to think that

@robertsLando
Copy link
Member

robertsLando commented Feb 23, 2021

@billiaz multilevel CC has startLevelChange and stopLevelChange, in writeValue function I have handle them by checking if the CC is multilevel switch and the payload is start or stop, we could easily do the same here for barrier operator if it provides such functions, But you need to change the payload in the template here. https://zwave-js.github.io/node-zwave-js/#/api/CCs/BarrierOperator?id=set

@varet80
Copy link
Collaborator Author

varet80 commented Feb 23, 2021

Thanks @robertsLando for pointing out
I need to finish: RGBWWCW adaptation now we have a new driver, then some other low hanging fruits and check this

@kpine I look into doing it in a few days and testing. Is this ok? (usually few days is ~4 days)

@kpine
Copy link
Contributor

kpine commented Feb 23, 2021

Well, it was more of a question. I implemented the Barrier Operator support for the cover platform in Home Assistant, and from looking at the z-wave specs and the node-zwave-js API, there is no Stop command, so I did not implement support for it. The spec also shows that you stop the cover only when it's closing, using an Open command. I was following the work here and noticed that you added the payload_stop discovery field, which I believe would allow HA to send a "Stop" command? I was wondering if that actually worked or not, because I don't see how it would. Perhaps the payload_stop value could be set to 255 (Open), but it would only be functional when the barrier is closing, as far as I understand.

image

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

Successfully merging this pull request may close these issues.

Cover state is not being interpreted correctly in Home Assistant
5 participants