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.

Sign up for GitHub

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

Jump to bottom

Add MQTT Alarm Control Panel custom bypass state #32541

Merged
merged 4 commits into from
Apr 6, 2020
Merged

Add MQTT Alarm Control Panel custom bypass state #32541

merged 4 commits into from
Apr 6, 2020

Conversation

yozik04
Copy link
Contributor

@yozik04 yozik04 commented Mar 6, 2020 •
edited
Loading

Breaking change

Proposed change

Currently Alarm Control Panel component has more states than is available in it's MQTT version.
This pull request fixes that.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@homeassistant
Copy link
Contributor

Hi @yozik04,

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!

@probot-home-assistant
Copy link

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with a integration (mqtt) you are listed as a codeowner for? Thanks!

yozik04 added a commit to yozik04/home-assistant.io that referenced this pull request Mar 6, 2020
@yozik04
MQTT Alarm Control Panel: States
4f9a16a 
Adding missing alarm states
Complements: home-assistant/core#32541
yozik04 added a commit to yozik04/home-assistant.io that referenced this pull request Mar 6, 2020
@yozik04
MQTT Alarm Control panel: States
3734781 
Adds missing states:
Complements: home-assistant/core#32541
yozik04 added a commit to yozik04/home-assistant.io that referenced this pull request Mar 6, 2020
@yozik04
Update alarm_control_panel.mqtt.markdown
f341f90 
Adds missing states:
Complements: home-assistant/core#32541
@yozik04 yozik04 mentioned this pull request Mar 6, 2020
Merged
7 tasks
@yozik04 yozik04 force-pushed the mqtt_alarm_control_panel_states branch from caf63ec to df28380 Compare March 6, 2020 17:33
@yozik04 yozik04 force-pushed the mqtt_alarm_control_panel_states branch from df28380 to c7b7b03 Compare March 6, 2020 17:44
MatthewFlamm
MatthewFlamm reviewed Mar 6, 2020
View reviewed changes
Copy link
Contributor

@MatthewFlamm MatthewFlamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://developers.home-assistant.io/docs/entity_alarm_control_panel/#alarm-custom-bypass

I think you need to add the async_alarm_arm_custom_bypass method and also add to supported_features.

@yozik04
Copy link
Contributor Author

yozik04 commented Mar 6, 2020

@MatthewFlamm Valid point

@yozik04
Copy link
Contributor Author

yozik04 commented Mar 6, 2020

Should I squash all commits into one?

@yozik04 yozik04 marked this pull request as ready for review March 6, 2020 20:20
@MatthewFlamm
Copy link
Contributor

I think it is usually customary in most cases to keep all the commits available during review, and then the person who merges it can squash them. Otherwise, for example, my comment will be out of context if someone cannot see the history of the commits.

@yozik04
Copy link
Contributor Author

yozik04 commented Mar 17, 2020

This one is ready for a review.

@yozik04
Copy link
Contributor Author

yozik04 commented Apr 4, 2020

Is anything else required to get this reviewed and merged? It does not break anything. Just makes it up to date with main alarm_control_panel component.

@MartinHjelmare MartinHjelmare changed the title MQTT Alarm Control Panel to have all available states Add MQTT Alarm Control Panel custom bypass state Apr 4, 2020
emontnemery
emontnemery approved these changes Apr 6, 2020
View reviewed changes
Copy link
Contributor

@emontnemery emontnemery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@emontnemery emontnemery merged commit a1aebe9 into home-assistant:dev Apr 6, 2020
frenck added a commit to home-assistant/home-assistant.io that referenced this pull request Apr 6, 2020
@yozik04 @frenck
Update alarm_control_panel.mqtt.markdown (#12291)
194b3d3 
* Update alarm_control_panel.mqtt.markdown

Adds missing states:
Complements: home-assistant/core#32541

* payload_arm_custom_bypass added

* MQTT Abbreviation for payload_arm_custom_bypass

MQTT Abbreviation for payload_arm_custom_bypass -> pl_arm_custom_b

Co-authored-by: Franck Nijhof <git@frenck.dev>
@lock lock bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MatthewFlamm MatthewFlamm MatthewFlamm left review comments

@emontnemery emontnemery emontnemery approved these changes

Assignees
No one assigned
Labels
cla-signed core has-tests integration: mqtt small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yozik04 @homeassistant @MatthewFlamm @emontnemery