Add Custom mapping of Risco states #39218
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OnFreund
commented
Aug 24, 2020
Comment on lines
+168
to
170
| if not risco_state: | ||
| _LOGGER.warning("No mapping for mode %s", mode) | ||
| return |
There was a problem hiding this comment.
Not sure how to cover this with a test. Hypothetically, this condition can't happen, but it seems wise to add this guard to protect against a bug that would cause it to.
There was a problem hiding this comment.
have you tried initializing the mock entity with options that are missing one of the keys? might that trigger it?
There was a problem hiding this comment.
Any keys that are missing in ha_to_risco mean that the entity wouldn't report those services as supported, thus there's no way to run them...
This was referenced Aug 24, 2020
ctalkington
approved these changes
Aug 28, 2020
Platinumwrist
approved these changes
Aug 28, 2020
weissm
pushed a commit
to weissm/core
that referenced
this pull request
Aug 28, 2020
* Custom mapping of Risco states * More informative error log * Add alternative Risco terms * Black formatting
leikoilja
pushed a commit
to leikoilja/core
that referenced
this pull request
Sep 6, 2020
* Custom mapping of Risco states * More informative error log * Add alternative Risco terms * Black formatting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed change
Enable custom configuration of Risco and HA alarm states. The user can define a two-way mapping - which HA state the entity will be in when Risco is reporting state X, and how to arm Risco when arming the entity in mode Y.
The possible Risco states are:
The possible HA states are:
STATE_ALARM_ARMED_AWAYSTATE_ALARM_ARMED_CUSTOM_BYPASSSTATE_ALARM_ARMED_HOMESTATE_ALARM_ARMED_NIGHTOnly HA states that are mapped will be supported by the entity.
This also marks the last PR in the roadmap I laid out in the initial PR for this integration. I do have other minor stuff potentially coming up, like (un)bypassing zones, but nothing major.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: