-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Add integration for Olarm #147591
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
base: dev
Are you sure you want to change the base?
Add integration for Olarm #147591
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @pelicarno
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the core Olarm integration to Home Assistant, including OAuth2 config flow, device coordinator (API + MQTT), entity platforms, services, and tests.
- Introduces
OlarmFlowClientCoordinator
for API/MQTT management and integrates it with config entries. - Implements OAuth2-based
config_flow
, entity platforms (button
,binary_sensor
,alarm_control_panel
), services, and application credentials. - Adds comprehensive tests for
init
andconfig_flow
, updates manifest, requirements, strings, and quality scale.
Reviewed Changes
Copilot reviewed 21 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/components/olarm/test_init.py | Add init tests for setup/unload and auth handling |
tests/components/olarm/test_config_flow.py | Add full-flow, error, and reauth config flow tests |
tests/components/olarm/const.py | Define mock device response constants |
tests/components/olarm/conftest.py | Fixture for application credentials setup |
requirements_all.txt & requirements_test_all.txt | Add olarmflowclient==1.0.0 dependency |
homeassistant/components/olarm/manifest.json | Add integration metadata |
homeassistant/components/olarm/quality_scale.yaml | Define Bronze quality scale rules |
homeassistant/components/olarm/strings.json | Localized UI strings |
homeassistant/components/olarm/services.yaml | Define service schemas and selectors |
homeassistant/components/olarm/application_credentials.py | OAuth2 authorization server implementation |
homeassistant/components/olarm/coordinator.py | API/MQTT coordinator implementation |
homeassistant/components/olarm/config_flow.py | OAuth2-based configuration flow |
homeassistant/components/olarm/button.py | Button entity platform for PGM and LINK controls |
homeassistant/components/olarm/binary_sensor.py | Binary sensor entity platform for zones and links |
homeassistant/components/olarm/alarm_control_panel.py | Alarm control panel entity platform for areas |
homeassistant/components/olarm/init.py | Core setup/unload and service registration |
homeassistant/components/olarm/const.py | Integration constants |
homeassistant/components/olarm/icons.json | Service icons mapping |
CODEOWNERS | Add @olarmtech code owner entries |
Comments suppressed due to low confidence (5)
homeassistant/components/olarm/coordinator.py:290
- Rename variable 'flagDispatch' to 'flag_dispatch' to follow PEP 8 snake_case naming conventions.
flagDispatch = False
homeassistant/components/olarm/config_flow.py:159
- The schema field 'load_zones_bypass' and the stored key 'load_zones_bypass_entities' are inconsistent; consider using a single consistent key across schema and data.
"load_zones_bypass",
homeassistant/components/olarm/button.py:1
- Add tests for button entities to cover name construction, unique IDs, and async_press behaviors in OlarmButton.
"""Support for PGMS, link outputs through the Olarm cloud API."""
homeassistant/components/olarm/binary_sensor.py:1
- Add tests for binary sensors to verify initial state mapping and dispatcher-based MQTT updates in OlarmBinarySensor.
"""Support for Olarm binary sensors."""
homeassistant/components/olarm/alarm_control_panel.py:1
- Add tests for alarm control panels to validate initial alarm state mapping and command methods in OlarmAlarmControlPanel.
"""Support for Olarm Alarm Control Panels."""
tests/components/olarm/test_init.py
Outdated
# patch( | ||
# "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation" | ||
# ) as mock_impl, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented-out patch lines to comply with the 'no commented out code' checklist and improve readability.
# patch( | |
# "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation" | |
# ) as mock_impl, |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for your contribution. Please limit your PR to a single platform, remove all the services and remove reauth and reauthentication from your config flow.
The smaller you get that first PR, the easier it will be to review and the faster it will be merged, you can add all those things back later in smaller chunks.
afc3dc5
to
5958a47
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few quick notes from a translator's perspective.
"config": { | ||
"step": { | ||
"device": { | ||
"title": "Select Device", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be sentence-cased:
"title": "Select Device", | |
"title": "Select device", |
"description": "Choose the Olarm device you would like to integrate. If your device is not listed, please ensure you have enabled public API access in your Olarm account for that device.", | ||
"data": { | ||
"select_device": "Device", | ||
"load_zones_bypass": "Would you like to also load entities for zone bypassing?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should sound like a label, not a dialog question:
"load_zones_bypass": "Would you like to also load entities for zone bypassing?" | |
"load_zones_bypass": "Load entities for zone bypassing" |
You should use the currently empty data_description
for this field to explain why this is optional.
"already_configured": "Account is already configured", | ||
"already_in_progress": "Configuration flow is already in progress", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are common strings for those which save translator time to do them again:
"already_configured": "Account is already configured", | |
"already_in_progress": "Configuration flow is already in progress", | |
"already_configured": "[%key:common::config_flow::abort::already_configured_account%]", | |
"already_in_progress": "[%key:common::config_flow::abort::already_in_progress%]", |
Perhaps you also find some that match some of those below.
9450717
to
872cd58
Compare
Hi All! Thanks for the feedback, much appreciated. Have made following changes
|
Breaking change
Proposed change
Adding core integration for Olarm Olarm
Type of change
Additional information
Checklist
ruff format 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
.To help with the load of incoming pull requests: