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 tado turn on off #22291

Merged
merged 3 commits into from Apr 5, 2019

Conversation

Projects
None yet
9 participants
@wmalgadey
Copy link
Contributor

commented Mar 22, 2019

Breaking Change:

separated the off mode from the normal tado operations.

Description:

Related issue (if applicable): fixes #22290

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@michaelarnauts I am still not able to push a branch to github, with the main.workflow file. Could you please create a working branch/pr?

I would like to test this changes on my hass installation. But I have a docker install! Can you tell me how I can make this changes in docker to test?

@wmalgadey wmalgadey changed the title Fix tado turn on off WIP: Fix tado turn on off Mar 22, 2019

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

please wait, I have to look into this again. No mode is working in the moment!

@michaelarnauts

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

@wmalgadey are you sure you are starting your topic branch from an up to date dev? I don't see a reason why you shouldn't be able to commit with that file to your own fork. What error do you get?

You can just docker exec -it containername bash to a running container and change the files there. If you do a docker restart containername then, it should just restart with the new code. You can see your container name with docker ps.

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

wmalgadey/PyTado#14 just for the references

@Kugelfang666

This comment has been minimized.

Copy link

commented Apr 1, 2019

any idea whether this PR will make it into the next release? currently the broken OFF mode for Tado is the only thing which keeps me from updating from 89.2

@bruvv

This comment has been minimized.

Copy link

commented Apr 2, 2019

Tested this in my docker container hassio and works fine. I am not using tado mode but smart shedule not sure why i would want to use tado mode so it is fine for me.

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

sorry for the delay. I tested it locally and it works.

@michaelarnauts can you please cherry pick my changes and create a new PR? I am still not able to push the main.workflow

@wmalgadey wmalgadey changed the title WIP: Fix tado turn on off Fix tado turn on off Apr 3, 2019

@bruvv

This comment has been minimized.

Copy link

commented Apr 3, 2019

PS turning water off doesn't work

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

@d1slact0r can I test this somehow? Or can you sent me the error you get?

@bruvv

This comment has been minimized.

Copy link

commented Apr 3, 2019

Screenshot_20190403-101442__01
If you own a tado and setup it up modulair you can use tado to control the tap water heater :)

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

If you own a tado and setup it up modulair you can use tado to control the tap water heater :)

In german I would call you a "Scherzbold" :D

we once had a PR to add the water heater functionality to the component. Currently the climate devices doesn't distinguish between device types (AC, HEATER, WATER HEATER).

@michaelarnauts

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

sorry for the delay. I tested it locally and it works.

@michaelarnauts can you please cherry pick my changes and create a new PR? I am still not able to push the main.workflow

Sure, but I still don't see how it is possible that you can't push a branch with that file. What OS are you using?

EDIT: I will wait a bit to be sure the issue from @d1slact0r is okay.

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Sure, but I still don't see how it is possible that you can't push a branch with that file. What OS are you using?

I don't know either. I am using "github desktop" on mac, but I can't push on windows too. It is a message from github.

The issue from d1slact0r will not be fixed with this PR. We need to create a new one to distinguish between ac, heater and hot water devices.

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@michaelarnauts, I can't tell whats changed, but... I just reverted the "can't push.."-commit and could push the changes!?

@michaelarnauts

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

Nice. The PR looks okay now.

@michaelarnauts

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

I don't see why the test fails. It doesn't seem related.

@michaelarnauts

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

@wmalgadey I'm not sure if I can merge this while the tests are failling. Maybe you need to rebase on upstream dev for the tests to succeed.

wmalgadey added some commits Mar 22, 2019

@wmalgadey wmalgadey force-pushed the wmalgadey:fix_tado_turn_on_off branch from bb87493 to e215872 Apr 4, 2019

@wmalgadey

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@michaelarnauts I did rebase and squash the commits! Sadly I am again not able to push with main.workflow :/

@awarecan awarecan merged commit 6996fec into home-assistant:dev Apr 5, 2019

13 checks passed

build Workflow: build
Details
ci/circleci: pre-install-all-requirements Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: pre-test 3.7 Your tests passed on CircleCI!
Details
ci/circleci: pylint Your tests passed on CircleCI!
Details
ci/circleci: static-check Your tests passed on CircleCI!
Details
ci/circleci: test 3.5.5 Your tests passed on CircleCI!
Details
ci/circleci: test 3.6 Your tests passed on CircleCI!
Details
ci/circleci: test 3.7 Your tests passed on CircleCI!
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected when comparing 07d739c...c1db06a
Details
codecov/project 93.84% (target 90%)
Details

@ghost ghost removed the in progress label Apr 5, 2019

@pbellon70

This comment has been minimized.

Copy link

commented Apr 6, 2019

Hi, in new release 0.91 the problem (tado off not working with error 422) is still present, so I guess the fix has not been issued yet. Do you know when this will be available in homeassitant release?

@awarecan awarecan added this to the 0.91.2 milestone Apr 7, 2019

unibeck pushed a commit to unibeck/home-assistant that referenced this pull request Apr 7, 2019

Fix tado turn on off (home-assistant#22291)
* fix for turn on and off, with new pyTado

missing blank line

* removed, because can't push

* uploaded the file through github again

pvizeli added a commit that referenced this pull request Apr 8, 2019

Fix tado turn on off (#22291)
* fix for turn on and off, with new pyTado

missing blank line

* removed, because can't push

* uploaded the file through github again

@pvizeli pvizeli referenced this pull request Apr 8, 2019

Merged

0.91.2 #22883

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