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

Add is_stopped to Cover #269

Open
MarcHagen opened this issue Aug 5, 2019 · 7 comments

Comments

@MarcHagen
Copy link

commented Aug 5, 2019

Context

As a recent addition of more devices classes to the Cover component i have been missing one option.
The is_stopped state. This can be called when the cover, like a shade or garage door, is stopped half way moving up or down.

home-assistant/home-assistant#25698

Proposal

Adding a property called is_stopped to Cover.py which is called by the state() the same way as is_opening/is_closing

Consequences

This means we can add more functionality to the default component.
Now the problem is, if you don't have a device what is returning a position but only a state the UI is giving you only a option to go further down/close, because there is no "middle/half" state, if it's stopped thru the UI.
This would mean making a custom_component or submitting a very basic component to achieve the same result as just adding this simple state.

@MarcHagen MarcHagen referenced this issue Aug 5, 2019
5 of 9 tasks complete
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

Normally only features that many (possible) integrations have will be considered for addition to an entity model.

If you can show that there are many examples of devices of different brands that would benefit of this feature, it will probably be easier to accept this proposal.

@Jc2k

This comment has been minimized.

Copy link

commented Aug 5, 2019

I had a quick look to see if it would be possible to implement this for HomeKit window coverings.

The HomeKit spec for Window Coverings has a characteristic called Position State that is either 0 (closing), 1 (opening) or 2 (stopped). Here stopped doesn't map cleanly onto 'is_stopped' though. E.g. you'd have to check for is_closed too.

I think the real question is what is stopped and what is open? If i tell my cover to be 75% open and it is then i'd consider it open, but Position State would consider it stopped. And i think this proposal would consider it stopped?

Another option is to say if Position State is stopped and target position != current position its stopped, otherwise its open. But the spec doesn't really say if that is a state that can exist. And right now in Home Assistant i don't think it can because the only way to stop a cover is to give it a new position with set_position (there is a 'hold' characteristic but we don't use it). I'm not sure what happens if you physically jam it, and i don't have one to test it. And if the device has a physical control i believe that just manipulates the target position.

@MarcHagen

This comment has been minimized.

Copy link
Author

commented Aug 5, 2019

@Jc2k You was just a bit quicker

Having done some research this is what is have now.

  • Apple HomeKit does report the stopped state. But the HomeKit_controllers has the is_opening, is_closing but not using the stopped state. This would be the biggest number of devices.
  • GaradgetCover has stopped, but not using it,
  • OpenGarageCover has a _state_before_move which is exactly the is_stopped state. It could make more sense.
  • Some ESPHome Covers are configured to go open, when the closed button is pressed second time, using a compare to the previous state.
    For now they have in the EsphomeCover a idle state, between opening and closing
  • BruntDevice has a move_state, which also can be the stopped

Possible integration would be the Lovelace UI, i not yet sure how that's working because i'm still quite new to HA, but the default cover card could use the stopped to display both arrows for up and down.

Links for now:
https://community.home-assistant.io/t/mqtt-cover-additional-states-opening-closing-stopped/57586
https://community.home-assistant.io/t/mqtt-cover-with-more-states-opening-and-closing/107866

Jumping in on @Jc2k
In my head the stopped state is when it's stopped by interaction or no able to reach the target destination.

If you have a mqtt device what is not able to send the current position or target position and you manually stop, i think this would then be the stopped state.

@MartinHjelmare

This comment has been minimized.

@MarcHagen

This comment has been minimized.

Copy link
Author

commented Aug 5, 2019

We do have a stop service and entity method:
https://github.com/home-assistant/home-assistant/blob/cecfb2d65751336f619d24fbade6ba0d4beefd22/homeassistant/components/cover/__init__.py#L312-L321

Yes also that, the close and open have is_closing and is_opening so why not have is_stopped for stop

@Jc2k

This comment has been minimized.

Copy link

commented Aug 5, 2019

HomeKit has a way to tell something is opening/closing and a way to open and close a cover. It does have the equivalent of stop ('hold', a write only characteristic). But HomeKit doesn't have a "at target state" Position State, its idea of stopped conflates "at requested position" and "transition interupted". I think stop has too many connatations and it's best to think of it as an idle state like ESPHome.

We could implement the stop service and set a _stop_used flag on the entity, and clear it the next time the current position changes or open/close/set_position are invoked. But thats very heuristicy and could end up being be very fragile, and the naive implementation i briefly considered was actually racy. I like to think i would get stopped at code review if i tried it.

Based on what you are asking for, for HomeKit is_stopped would probably best be implemented as: Position State is stopped and Target Pos != Current Pos. However without testing a few devices I'm not sure if thats a real state that it can be in. It's conceivable that as soon as you use the 'hold' state to do a stop the device would set target pos = current pos and not show as stopped and more. So I wouldn't want to add this logic until we had a real world device where we could see it work.

@MarcHagen

This comment has been minimized.

Copy link
Author

commented Aug 5, 2019

Is it possible to then only use this for the dumber devices that are not reporting a position? Because i can see that a stopped state is not really usable when you have current position state.

I have very dump shades on my windows. They are controlled only by power up/down. I made my own Sonoff Dual firmware to control the shade.
I'm only using a state open, opening, close, closing and middle (stopped) using MQTT and not programming a target because this would have to much problems because i cannot very if the screen is really in this position.
Now the HA default cover does only allow a up or down if the cover is in the opposite state. No middle/stopped. This is fine if you only want them up or down. But sometimes i only want it manually to be in the middle, only the top bit.
I also can control them by a button at the window itself.

In home-assistant/home-assistant#25698, as example, i "ignore" the opening/closing/stopped state if the position-topic is set.
That's why i made the PR and this "issue"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.