Skip to content

Conversation

@FrankTub
Copy link

@FrankTub FrankTub commented Jun 10, 2025

I would like some feedback on my implementation direction.

Some trade-offs I made:

  • It has the same trade off as the other force features, the state if the option was not on/true can not be reproduced when this changes back to false
  • Still in doubt if below logic should be applied to all actions where the action cover.set_cover_position or cover.open/close_cover is called. I currently think did not do this for the "close" situation but this might be a mistake.
  • if a cover is not moved because of this option, the tilt change is also not applied. I think I made it clear enough in the description that this is the behavior and as far as I can imagine this will most likely only be applied for covers that are outside and thus do not have a tilt option.
  • If another force feature is applied it will still take a look at the new feature auto_prevent_opening_force if this force feature is allowed to happen. To me it makes the most sense if someone is using multiple force features
  • I placed this feature under force since it behaves differently compared to prevent_* due to the fact that the state is not reproduced when this option changes to false/off.

I'm still testing this a bit, so might be best to wait for my confirmation that everything works OK in my case at least All tests look OK

@FrankTub
Copy link
Author

FrankTub commented Jun 12, 2025

I've done very basic tests and this does exactly what it should do in my case.

I have tested the following:

  • auto_prevent_opening_force = off
    • automatically opens
    • automatically closes
  • auto_prevent_opening_force = on
    • automatically closes
    • Does not open automatically

Now I think about it some more I guess it is a good practice that even for the closing action to apply a check that the target position should be below the current position if the boolean is on. It really depends on what kind of closing position a user has configured. I will implement this as well.

@FrankTub FrankTub changed the title Draft: Feature #213: Optional feature to prevent any opening movement of a cover, could be used to force drying of a cover Feature #213: Optional feature to prevent any opening movement of a cover, could be used to force drying of a cover Jun 12, 2025
@FrankTub FrankTub changed the title Feature #213: Optional feature to prevent any opening movement of a cover, could be used to force drying of a cover Feature #213: Optional feature to prevent any opening movement of a cover while still allowing closing movements, could be used to force drying of a cover Jun 12, 2025
@hvorragend
Copy link
Owner

hvorragend commented Jun 12, 2025

I really appreciate it when someone helps out here. Many thanks for that. But I'm a little unsure about the merge.

First of all, all existing force features are there to trigger a certain state. This does not allow the mode or behavior of the automation to be changed. I have added the feature flags under Automation Options for this purpose. Here you can control very deficient little things.
Okay, admittedly. Until now, you could not specify an entity here.

IMG_7569

I would prefer that up there.
Unfortunately, the code base has changed completely today. A merge would not be possible at the moment anyway. I'm sorry, but my local repository was already further along.

@hvorragend hvorragend added the enhancement New feature or request label Jun 12, 2025
@FrankTub
Copy link
Author

Thanks for your feedback!

The thing I'm still a bit worried on is if we add it to the prevent list will this behave in the same way as the other prevent options or do I need to make it clear in the description or something that the behavior might be different compared to the rest of the list?

No problem for me to rewrite it to the new situation. Will do so once you come back to the above question.

I was also considering yaml anchors in this PR but decided not to do so since you have not used it elsewhere. Great to see that you implemented this now so that it becomes even better maintainable and readable!

@hvorragend
Copy link
Owner

The more I think about it, the more I wonder whether this is even necessary.

Is it not possible to simply deactivate the closing of the roller blinds via the additional conditions? This is not exactly what we wanted. But we don't confuse users with the multitude of configuration options.

@FrankTub
Copy link
Author

I agree that complexity always comes at a price of the maintainability. So perhaps I should elaborate a bit more on what I want to achieve and what I considered as alternatives.

Setup

I have 3 roller shutters and 1 screen with 2 on the south side and 2 on the north side.

For the 2 on the north side the automation is relatively limited in terms of the options I use. I use generic opening and closing of the cover and in addition use the sun elevation option and intend to start using the residence setting when I got proper sensors to depend it on.

The 2 on the south side are also straightforward. In addition to the above options I also use sun shading functionality.

I also configured the working day sensor to distinguish between late and early drives of the automation. In addition I use a helper in all 4 automations.

Use case

When it is raining my covers get wet since they are placed at the outside of the windows. Especially for the screen it is important that it does not move to an open position to prevent mold. But also for the roller shutters it is advised by the manufacturer to not open the roller shutters when they are wet. This will enhance life expectancy for these devices.

Ideal solution

When it is raining or if a cover still needs to dry after the rain has stopped it should be templates in a binary sensor. This binary sensor should be used as an input for a new feature.

Not closed

When my covers are in a not closed position (i.e. open/ventilation/sun shading (unlikely)) and apparently we prefer to have light from the outside in our house and it will start raining nothing needs to happen directly.

If it keeps on raining and the cover should move the a more closed position this is no problem and this should still happen. This could be a closed position, but also perhaps a sun shading/ventilation position.

If it stops raining and the cover was still not closed, then it should move to the position it otherwise should have been in without this feature. Given that closing movements were still allowed this allows only movement from sun shading /ventilation / open position.

Closed

When it stops raining and it is still before the time that the cover should move to an upward position then it should stay as it is.

When it keeps on raining till the point where normally the cover would have opened, it should stay as it was -> closed.

When it stops raining after the point where the cover normally would have opened if this feature wasn't there, then it should move to the position where it would have been without this feature. I think this could also be the case that it could move to a ventilation or sun shading position aside from open. It depends on the entity and your drying period what the right position might be.

Alternatives considered

  • Having a custom automation that sets the above mentioned binary sensor and disable the automation through a force feature in cca. A need to reproduce the state the cca automation would have been in without this binary sensor. This is very hard given that you need to rebuild all logic in cca to determine proper location.
  • Using one of the other force features. I don't want to force a specific state of it is raining. For example I don't want to always open a cover to prevent it from getting wet, this is not useful for example in the middle of the night. I also don't want to make it unnecessary dark in rooms simply always closing the covers when it is raining.

Perhaps I'm missing something obvious that is already there that I could use. Please let me.know if that is the case

Ps: this PR does not adhere to my description of the ideal solution. It does not add a trigger when this entity moves from on to off, which is probably what would be required.

@FrankTub
Copy link
Author

Now I think about it some more, perhaps a trigger could be added that it behaves somehow similar to what the residence setting is doing when this moves to off. Haven't taken a look at the code if this is doing exactly what I describe above. Otherwise it is required to have available what the expected output of the cca automation would be if it was not blocked by this feature (similar to the force features #217)

@FrankTub FrankTub force-pushed the feature/force-prevent-opening branch from ff4d5b1 to 53349b0 Compare June 14, 2025 18:11
@FrankTub
Copy link
Author

FrankTub commented Jun 16, 2025

@hvorragend I refactored and tested the new code and I'm really happy about it the way it is working for me. I introduced a new anchor and hope this is something you agree on 🙏

In the UI it looks like:

image

This new option prevent_opening_movement does the following:

  • When this entity of prevent_opening_movement changes from off to on, then nothing happens right away
  • When this entity of prevent_opening_movement changes from on to off, it will trigger the automation.
    • When the exepected state of the cover would have been open, it will change to open (Verified and tested)
    • When the expected state of the cover is closed, then it will stay closed (Verified and tested)
    • When the current state is closed and the expected state is ventilation, then it will change to ventilation position (Verified and tested)
    • When the current state is closed or ventilation and the expected state is shading (and the position is higher than closed/ventilation), I guess I made the required changes but I have not tested this in my setup.
    • When the current state is shading position and shading has ended it should change to the new expected situation (that his higher than the shading position), I think I made the required changes, but I have not tested this and unsure how to do this.

Also verified the following, prevent_opening_movement = on:

  • all downward movements are executed as they normally would.
  • upward movements as open/ventilation position are not occuring
    When prevent_opening_movement = off:
  • all downward movements are executed as they normally would.
  • All upward movements are happening as they normally would

The thing I was not sure on was if we should prefer a force feature to still allow a specific state even if prevent_opening_movement = on and the target position is higher than the current position. Given that it was easier to not allow that to happen I currently decided against it. But one could also argue that the force features takes precedence over prevent_opening_movement.

Let me know what you think and if anything needs to be tweaked. Also open for renaming some of these parts of the code where ever necessary. Otherwise also happy to just use my custom version of the code if you don't agree on it.

@FrankTub FrankTub force-pushed the feature/force-prevent-opening branch from 13285c2 to f3a14d3 Compare June 20, 2025 18:10
@hvorragend
Copy link
Owner

Thank you very much for your effort. You're making a real effort and testing it too. I think that's really good.

I'm still unsure about this change. But I understand it very well. So you don't want the roller blind to open any further. And that across the entire code in all features. And since you have to check the positions, you can't use the existing conditions for this. That's the reason, isn't it?

Everything understandable up to this point.

I'm just annoyed that you have combined my three YAML anchors (*cover_move_action, *tilt_move_action, *helper_update) into "*cover_move_and_tilt_and_update_helper".
This makes it much harder to maintain the code and the flexibility is lost. But I don't have a better idea right now.

Although I would try to incorporate this into the existing YAML anchors. I'll take a look at it.

I also find this nesting absolutely unfortunate. At first glance, I would say that it should be much easier. I think I can see a lot of redundancies.

    - if:
        - or:
            - and:
                - or:
                    - "{{ prevent_opening_movement_disabled }}"
                    - "{{ not prevent_opening_movement_disabled and (current_position >= target_position | default(101)) }}"
                - or:
                    - "{{ should_cover_move_and_tilt is not defined }}"
                    - "{{ should_cover_move_and_tilt is defined and should_cover_move_and_tilt }}"
            - "{{ should_cover_move_and_tilt is defined }}"

@FrankTub
Copy link
Author

I'm still unsure about this change. But I understand it very well. So you don't want the roller blind to open any further. And that across the entire code in all features. And since you have to check the positions, you can't use the existing conditions for this. That's the reason, isn't it?

Yes, correct. Given that in all the actions it is decided what the actual target position should be, you can only decide past that point (cover_move_action) if it should move or not. The alternative would be to have such a check in each action part where the cover should move, my take on it was that it was harder to maintain.

I'm just annoyed that you have combined my three YAML anchors (*cover_move_action, *tilt_move_action, *helper_update) into "*cover_move_and_tilt_and_update_helper".
This makes it much harder to maintain the code and the flexibility is lost. But I don't have a better idea right now.

I understand your point. What I tried, but perhaps failed to achieve, is to have a single anchor/function that is the single method called in all actions and simplify things 😄 . The trade off is that some parts of the code ALWAYS execute *helper_update, while *cover_move_action and *tilt_move_action are still conditional. This makes it more complex, and I guess that is what you call a loss of flexibility. In hindsight I agree that it could be simpler to add the check to the existing YAML anchor(s). I can rearrange the code so that the condition is applied to *cover_move_action and *tilt_move_action.

I also find this nesting absolutely unfortunate. At first glance, I would say that it should be much easier. I think I can see a lot of redundancies.

The added benefit of moving the condition to the different anchors is that such a condition is no longer needed 😄

@FrankTub
Copy link
Author

Now I tried to simplify I realised I also had another reason for combining the three YAML anchors into a single anchor.

The trade off is that some parts of the code ALWAYS execute *helper_update, while *cover_move_action and *tilt_move_action are still conditional.

This is still true. The hard part here is that if the part where you call one of the anchors, the part where you called it from is not aware of if actually executed that step. Now I realise that that was the reason why I decided to combine the YAML anchors in the first place. So if the cover is not moved, it does not makes sense to update the helper. However, sometimes you still need to update the helper, even if the cover is not moved (not sure why though). → You can't simply place the same logic in helper_update as you would in cover_move_action. I'm a bit out of inspiration how I can solve this without an ugly if statement and a new variable in the places where you always want to update the helper independent if you move the cover. What is your take on it @hvorragend ?

@hvorragend
Copy link
Owner

But I can also have a look at it. I had been playing around with the code over the last few days and had an idea. But I couldn't solve the structure of the anchors any other way. I couldn't think of anything.

I can have a look.

@hvorragend
Copy link
Owner

We replied at the same time. My last answer does not refer to your last post from just now. But to the one before that. 😀

@FrankTub
Copy link
Author

What I have so far is https://github.com/FrankTub/ha-blueprints/blob/feature/feedback-prevent-opening/blueprints/automation/cover_control_automation.yaml. But it needs work on the helper cause that is really not correct in this commit. Would be really appreciated if you could help out here 🙏

@hvorragend
Copy link
Owner

Please give me a little more time.

@FrankTub
Copy link
Author

@hvorragend do you expect me to rebase it given that there is a new version available?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants