8000 Add integration for Olarm by pelicarno · Pull Request #147591 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from

Conversation

pelicarno
Copy link
@pelicarno pelicarno commented Jun 26, 2025

Breaking change

Proposed change

Adding core integration for Olarm Olarm

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format 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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

Copy link
@home-assistant home-assistant bot left a 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!

Copy link
@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor
@Copilot Copilot AI left a 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 and config_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."""

Comment on lines 38 to 40
# patch(
# "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation"
# ) as mock_impl,
Copy link
Preview
Copilot AI Jun 26, 2025

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.

Suggested change
# patch(
# "homeassistant.helpers.config_entry_oauth2_flow.async_get_config_entry_implementation"
# ) as mock_impl,

Copilot uses AI. Check for mistakes.

@pelicarno pelicarno marked this pull request as ready for review June 27, 2025 11:32
@pelicarno pelicarno marked this pull request as draft June 28, 2025 13:53
Copy link
Member
@zweckj zweckj left a 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.

Copy link
Contributor
@NoRi2909 NoRi2909 left a 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",
Copy link
Contributor

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:

Suggested change
"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?"
Copy link
Contributor

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:

Suggested change
"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.

Comment on lines 22 to 23
"already_configured": "Account is already configured",
"already_in_progress": "Configuration flow is already in progress",
Copy link
Contributor

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:

Suggested change
"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.

@pelicarno pelicarno force-pushed the olarm-integration-clean branch from 9450717 to 872cd58 Compare July 1, 2025 18:08
@pelicarno
Copy link
Author

Hi All!

Thanks for the feedback, much appreciated.

Have made following changes

  • Fixed copilot issue with commented code
  • Limited PR to only one Platform and will included others in future PRs
  • Also removed re-authentication flows to be included in future PRs
  • Fixed translations issues

@pelicarno pelicarno marked this pull request as ready for review July 1, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0