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

Update concord232.py to fix the home/stay command #7789

Merged
merged 1 commit into from May 28, 2017
Merged

Update concord232.py to fix the home/stay command #7789

merged 1 commit into from May 28, 2017

Conversation

CTLS
Copy link
Contributor

@CTLS CTLS commented May 26, 2017

Description:

Related issue (if applicable): fixes #5049

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link

@CTLS, thanks for your PR! By analyzing the history of the files in this pull request, we identified @JasonCarter80, @fabaff and @pvizeli to be potential reviewers.

@homeassistant
Copy link
Contributor

Hi @CTLS,

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!

@CTLS
Copy link
Contributor Author

CTLS commented May 26, 2017

I have not solved why the away mode does not work still, but this allows the stay/ home mode to work so at least the alarm can be armed and disarmed.

@MartinHjelmare
Copy link
Member

Did you try the 'exit' arm type as mentioned in #2194?

@CTLS
Copy link
Contributor Author

CTLS commented May 28, 2017

Just tried that and it doesn't work. I am not sure if it is an error with the concord232 server or the component.

@CTLS
Copy link
Contributor Author

CTLS commented May 28, 2017

After looking at the code of the concord232 server and comparing to the nx584 which it was built off of it looks like the concord232 server is missing the code for away mode and auto. I believe updating the stay code as I have changed it is the best that can be done on this side everything else would need to be updated on the concord232 server code by @JasonCarter80 and not in the home assistant component.

@MartinHjelmare
Copy link
Member

Ok. If you know what to fix upstream, you can make a PR to concord232, and hopefully get that in.

I'll merge this now. This will autoclose #5049. Either open a new issue for the remaining problem or let me know in #5049 if we should keep that open.

@MartinHjelmare MartinHjelmare merged commit 10367eb into home-assistant:dev May 28, 2017
@JasonCarter80
Copy link
Contributor

JasonCarter80 commented May 29, 2017 via email

@balloob balloob mentioned this pull request Jun 2, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
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.

Cannot arm concord232 alarm
6 participants