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 tahoma reversed closed status for non horizontal awnings #26840

Merged
merged 1 commit into from Oct 5, 2019

Conversation

@psicot
Copy link
Contributor

commented Sep 23, 2019

Home Assistant release with the issue:

0.99.2

Operating environment (Hass.io/Docker/Windows/etc.):

Hass.io

Description:

Related issue: fixes #25240
Also fixes the closed status when the position is changed when the position is <= 5 or when the position is >= 95.

The problem appeared in #23257 where the right closed status has been modified for non horizontal awnings (file homeassistant/components/tahoma/cover.py, removed line 117).

Example entry for configuration.yaml

tahoma:
  username: TAHOMA_USERNAME
  password: TAHOMA_PASSWORD
@homeassistant

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2019

Hi @psicot,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@psicot

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2019

@kbickar as the author of #23257, could you please check if the change to the closed status is required for horizontal awnings?
I restored the functioning of the closed state for non-horizontal awnings, but I'd like to make sure that it also works for horizontal awnings. Thanks!

@frenck

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

This feels like a never-ending PR/discussion. The PR you mentioned as the source of the issue, has actually been reverted, so your callout makes no sense.

  • Tahoma: Do not reverse open/close calls (#24879)
  • Position is reversed for horizontal awnings (#23257)

Related discussions and issues:

  • Tahoma HorizontalAwning not working anymore (#24813) (largest discussion)
  • Tahoma cover open / closed reversed (#24818)
  • tahoma service call cover op en closed reversed (#24832)
  • Tahoma: Windows and Awnings open/close status reversed (#25240)
  • Thahoma Cover percentages reversed (#16868)

I feel like we should stop the juggle and add support for a reverse flag or similar.

@psicot

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2019

Thanks @frenck for your reply.

#24879 has not resolved this issue. In 0.99.2 the problem still persists because this PR has not fixed the reversed closed state introduced by #23257, ie:

self._closed = self._position == 100

I think that the HORIZONTAL_AWNING option only need to change the calculation of the position, and must not reverse the closed state of all other accessories (roller shutters and garage door for my case).

@MartinHjelmare MartinHjelmare changed the title Fix reversed closed status for non horizontal awnings. Fix tahoma reversed closed status for non horizontal awnings Sep 24, 2019
@MartinHjelmare MartinHjelmare moved this from Needs review to Review in progress in Dev Sep 26, 2019
@psicot

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2019

Hello, any news concerning this fix?
I run this code has a custom_components since 10 days without issue.
Thanks!

All Tahoma users should be impacting by the change in #23257, which reverses the open/closed states.

@pax2011

This comment has been minimized.

Copy link

commented Oct 3, 2019

Hello, any news concerning this fix?
I run this code has a custom_components since 10 days without issue.
Thanks!

All Tahoma users should be impacting by the change in #23257, which reverses the open/closed states.

I don't understand how to solve it. I'm on HA 0.94.4, if update i get wrong open/close status.

@psicot

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2019

I don't understand how to solve it. I'm on HA 0.94.4, if update i get wrong open/close status.

@pax2011, fix the reversed statuses is the purpose of this PR (pull request).

To fix this problem, you can wait until this correction is included in a future version of HA.

Another solution is to use my modified code by downloading my version of the tahoma component, and use it has a remplacement of the one embedded in home assistant. You can follow these instructions to install a custom component in your config directory.

My modified Tahoma component is available here.

@pax2011

This comment has been minimized.

Copy link

commented Oct 4, 2019

I don't understand how to solve it. I'm on HA 0.94.4, if update i get wrong open/close status.

@pax2011, fix the reversed statuses is the purpose of this PR (pull request).

To fix this problem, you can wait until this correction is included in a future version of HA.

Another solution is to use my modified code by downloading my version of the tahoma component, and use it has a remplacement of the one embedded in home assistant. You can follow these instructions to install a custom component in your config directory.

My modified Tahoma component is available here.

I did it! Thanks man. Updated HA, used your code and now is working.

@psicot

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

I did it! Thanks man. Updated HA, used your code and now is working.

@pax2011 🎉

Dev automation moved this from Review in progress to Reviewer approved Oct 5, 2019
@balloob
balloob approved these changes Oct 5, 2019
@balloob balloob merged commit 43d1413 into home-assistant:dev Oct 5, 2019
12 checks passed
12 checks passed
CI Build #20190923.3 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot All contributors have signed the CLA
codecov/patch Coverage not affected when comparing 60f0988...ad44672
Details
codecov/project 94.07% (target 90%)
Details
docs-missing Documentation ok.
Dev automation moved this from Reviewer approved to Done Oct 5, 2019
@frenck frenck added the Hacktoberfest label Oct 6, 2019
@medaemon

This comment has been minimized.

Copy link

commented Oct 6, 2019

Hello, I have updated to 0.99.3 but I still have the issue on the state of my covers, closed when open open when closed.
Perhaps I miss understand and that the fix is not present in the last HA version, status is not very clear for me (I'm a rookie on the HA fix status ^^).

@DjMoren

This comment has been minimized.

Copy link

commented Oct 6, 2019

@medaemon, 0.99.3 was released 11 days ago and this PR has been merged 20 hours ago, so no, it's not included in 0.99.3. I don't know the release scheme HA follows (in which release will be included), but meanwhile, you could give a try to it following the instructions given by @psicot.

@balloob balloob added this to the 0.100 milestone Oct 6, 2019
@balloob

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

This will be released on Wednesday.

@lock lock bot locked and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
7 participants
You can’t perform that action at this time.