berrywhite96 / lovelace-shutter-row

Home Assistant Lovelace Shutter Row Card
MIT License
19 stars 4 forks source link

in release 0.2 stop button doesnt work anymore #11

Closed mogim0gi closed 1 year ago

mogim0gi commented 1 year ago

In previous version you could stop the blinds in movement, now the stop button is grayed out and not functional.

berrywhite96 commented 1 year ago

I thought I fixed it, but it seems like it fixes only for some shutters lol. I can confirm it currently works with this cover integration.

Can you post the state and the attributes of your cover entity?

mogim0gi commented 1 year ago
image

my tuya cover motors are integrated through zigbee2mqtt:

{ "linkquality": 57, "position": 0, "running": false, "state": "CLOSE" }

berrywhite96 commented 1 year ago

Does the state change from closed to opening if it moves up?

mogim0gi commented 1 year ago

no, state only changes after curtain stops, but the service cover.stop_cover stops curtain at any given moment.

berrywhite96 commented 1 year ago

Ah that's weird. So your cover entity does not report opening and closing states, I never thought this happens. So I think that can only be fixed with another config attribute to ignore the state.

berrywhite96 commented 1 year ago

Config implemented with v0.2.1.

convicte commented 1 year ago

@berrywhite96, I was wondering if there would be a more robust solution than that, and please feel free tell me if this is beyond the scope of the card or something you are not able to implement, but:

The covers I use which are also integrated via Zigbee2mqtt have a moving: up/down/stop attribute: image

From what I can tell @mogim0gi has his with a running: true/false attribute serving mostly the same function, to inform of the current movement of the shutter, beyond reporting the opening/closing state.

If possible, it would be great if the card would recognize a NOT stop/false attribute for moving or running and grey the stop button out only when that test is false (or the other way round) make it available when moving or running.

Having said all that, I have no way of knowing how many variants of such attributes exist in the wild and what states they can attain, but it feels like these two, plus potentially a stopped: true/false, should cover >90% of all cases. Though I recognize how wrong I may be in the IOT world of 'I do what I want and to hell with everyone else'.

Looking forward to your thoughts!

berrywhite96 commented 1 year ago

The covers I use which are also integrated via Zigbee2mqtt have a moving: up/down/stop attribute: image

From what I can tell @mogim0gi has his with a running: true/false attribute serving mostly the same function, to inform of the current movement of the shutter, beyond reporting the opening/closing state.

The funny thing about it, v.0.1.x used the moving attribute and I changed it to the state because I thought it will support all covers. Sadly I was wrong.

Having said all that, I have no way of knowing how many variants of such attributes exist in the wild and what states they can attain, but it feels like these two, plus potentially a stopped: true/false, should cover >90% of all cases. Though I recognize how wrong I may be in the IOT world of 'I do what I want and to hell with everyone else'.

I am quite sure there are not many people who can guess the working quote nearly right. And that's literally the problem. There is besides the base structure for plugins no documentation for developers. Basically everything I had to learn through reading source code from other plugins. So I need to dig really deep into HA core to find this out, so an update will take long time.

dm82m commented 1 year ago

5A9A33F9-450D-4282-9A48-5E3AA3A0E463

for me it worked with 0.1.x but not more with 2.x

i am using homematic ip and Status of cover is open or closed.

only other thing I see is current_position.

Maybe an idea could be to have the stop button enabled without checking if there is a movement of the cover? That is the behavior that the mushroom cover card is doing. Open and close button is enabled depending on state. Stop button is enabled at every time…

convicte commented 1 year ago

I am quite sure there are not many people who can guess the working quote nearly right. And that's literally the problem. There is besides the base structure for plugins no documentation for developers. Basically everything I had to learn through reading source code from other plugins. So I need to dig really deep into HA core to find this out, so an update will take long time.

I understand your pain and apologize for the inconvenience! If you feel there is a better way forward, please don't feel bound by this solution proposal.

Just an idea! ;)

Thank you for a very nice card so far!!

berrywhite96 commented 1 year ago

I understand your pain and apologize for the inconvenience! If you feel there is a better way forward, please don't feel bound by this solution proposal.

Just an idea! ;)

Thank you for a very nice card so far!!

No need to apologize, if I can make something that makes this card better I will do it.

So back to topic and a little update: I checked the source code from HA and they also just check the state of the entity. If it's not available they just enable the buttons. As this is just not enough for me I want to add additionally to check for attributes like moving. So I need more data from other devices which uses different attributes that show the current movement.

dm82m commented 1 year ago

Would it be possible to add something like alwaysenablestopbutton: true? The card is currently no more usable for me. Workaround would be that I remove it and switch back to the old one.

berrywhite96 commented 1 year ago

Would it be possible to add something like alwaysenablestopbutton: true? The card is currently no more usable for me. Workaround would be that I remove it and switch back to the old one.

As I said in the other issue there is an extra config attribute for it.

dm82m commented 1 year ago

As I said in the other issue there is an extra config attribute for it.

Yes but this also influences the up down buttons. So up button is available even if the cover is already full up. As I am on homematic ip and there is no running state i can go with state_confidence and lose enabling/disabling of up/down button or I go without state_condfidence and will never be able to stop the cover. To be honest: neither the first nor the second option is hitting my desire. But as it is your component I will move back to the last component I used. Thanks for your work!

berrywhite96 commented 1 year ago

As I said in the other issue there is an extra config attribute for it.

Yes but this also influences the up down buttons. So up button is available even if the cover is already full up. As I am ok homematic ip and there is no running state i can go with state_confidence and lose enabling/disabling of up/down button or I go without state_condfidence and will never be able to stop the cover. To be honest: neither the first nor the second option is hitting my desire. But as it is your component I will move back to the last component I used. Thanks for your work!

This is true but at least it works for now until I have time for a new release.

berrywhite96 commented 1 year ago

So back to topic and a little update: I checked the source code from HA and they also just check the state of the entity. If it's not available they just enable the buttons. As this is just not enough for me I want to add additionally to check for attributes like moving. So I need more data from other devices which uses different attributes that show the current movement.

I made a potential candidate for 0.2.2 with the behaviour above as described and I made the buttons a bit prettier. Also I removed the state_confidence config because it just didn't make sense and was just a fix for the bug in the previous version. As I got Covid this week the code quality may not be what to expect, so before I release this you can try it from the commit here: 55bab9c. Let me know if something is missing/buggy.

I still search for attributes like the moving parameter which is called differently on other devices. So if you have one, just let me know.

berrywhite96 commented 1 year ago

Hopefully now fixed with v0.2.2.

dm82m commented 1 year ago

Thank you! Works for me!