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 homematicip cloud alarm_arm_home #20321

Merged
merged 1 commit into from Jan 25, 2019
Merged

Fix homematicip cloud alarm_arm_home #20321

merged 1 commit into from Jan 25, 2019

Conversation

coreGreenberet
Copy link
Contributor

@coreGreenberet coreGreenberet commented Jan 22, 2019

Description:

The parameters were switched, so I've just fixed it with exchanging True,False to False,True

Related issue (if applicable): fixes #20307

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.

Sadly I can't test the code locally, but I'm the maintainer of the api which is getting called in the background and the call to the API was wrong, so I've fixed that.

@homeassistant
Copy link
Contributor

Hi @coreGreenberet,

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!

@ghost ghost added the in progress label Jan 22, 2019
@MartinHjelmare MartinHjelmare changed the title fixing alarm_arm_home Fix homematicip cloud alarm_arm_home Jan 23, 2019
@MartinHjelmare
Copy link
Member

@mxworm do you have time to review?

@worm-ee
Copy link
Contributor

worm-ee commented Jan 23, 2019

Hi, sure. I will review and test tomorrow.

Ciao

@worm-ee
Copy link
Contributor

worm-ee commented Jan 25, 2019

Hi @coreGreenberet,
Can you please update the lib version in homematicip_cloud/init.py ?
Than the fix can be merged.
Thanks!

@coreGreenberet
Copy link
Contributor Author

I guess I've made a mistake while creating the other PR. I can't change anything here anymore. I will change the lib version in the PR #20398

@MartinHjelmare
Copy link
Member

MartinHjelmare commented Jan 25, 2019

Always take for habit to work in separate git feature branches. That way, if one branch is messed up, other branches are not affected. It also simplifies rebasing from upstream if not working locally on the same branch as upstream target branch (dev).

@coreGreenberet
Copy link
Contributor Author

I will for the next time =)

since the api version got implemented in PR #20398, can this be merged too now?

@MartinHjelmare MartinHjelmare merged commit 2bc7444 into home-assistant:dev Jan 25, 2019
@ghost ghost removed the in progress label Jan 25, 2019
fredrike pushed a commit to fredrike/home-assistant that referenced this pull request Jan 30, 2019
@Obicom
Copy link

Obicom commented Feb 1, 2019

Hi together, with Beta 0.87.0b0 HmIP Alarm option works better but not perfect. Now I am able to swicth to arm_home but the entity status in HA looks wrong. Switch on Arm_home -> Only Alarm extern shows "Armed_Home" (shoud it not be Alarm internal?) Switch on Arm_way -> Alam external and internal changed to "Armed_away" (should it be not only Alarm external?) Please be so kind to double check, if possible. In addition thank you very much for your hard work.

@MartinHjelmare
Copy link
Member

Please open an issue if you suspect a bug.

If you want to suggest an enhancement please open a feature request in the Feature Requests section of our community forum.

Merged PRs should not be used for enhancement discussion or bug reports. If you've found a bug it's ok to make a review with inline comments and link to an issue that reports the bug.

Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Feb 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alarm ever activate arm_away never arm_home
5 participants