Skip to content

Move totalconnect from platform to component config#24427

Merged
MartinHjelmare merged 27 commits intohome-assistant:devfrom
austinmroczek:dev
Jul 14, 2019
Merged

Move totalconnect from platform to component config#24427
MartinHjelmare merged 27 commits intohome-assistant:devfrom
austinmroczek:dev

Conversation

@austinmroczek
Copy link
Copy Markdown
Contributor

@austinmroczek austinmroczek commented Jun 8, 2019

Breaking Change:

Total Connect Client was upgraded to support more than one alarm panel and allow future support for additional sensors. Previous alarm_control_panel entries must be removed, and a new totalconnect entry must be added to configuration.yaml. See https://www.home-assistant.io/components/totalconnect/ for configuration details.

Description:

Move toward multi-platform for Total Connect. Support multiple alarm panels. Changed totalconnect so we can add additional platforms moving forward. Moved the total_connect_client into init so a single instance will be shared across platforms. It should now allow for multiple alarm panels.

Related issue (if applicable):
craigjmidwinter/total-connect-client#26

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#
home-assistant/home-assistant.io#9589

Example entry for configuration.yaml (if applicable):

totalconnect:
    username: !secret totalconnect_username
    password: !secret totalconnect_password

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the [development checklist][dev-checklist]

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][manifest-docs] has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • [n/a] Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • [n/a] Tests have been added to verify that the new code works.

@MartinHjelmare MartinHjelmare changed the title Move toward multi-platform for Total Connect. Support multiple alarm panels. Move totalconnect from platform to component config Jun 30, 2019
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/alarm_control_panel.py
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

Please extend the breaking change paragraph and explain in more detail what the user needs to do to cope with the breaking change. We don't need to explain the config options. It's ok to reference the docs for those.

@MartinHjelmare
Copy link
Copy Markdown
Member

.coveragerc needs to be updated. We can use a star * to mark the whole totalconnect package as excluded from coverage calculation.

@austinmroczek
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare thanks for the detailed code review. I made updates per your guidance, changed .coveragerc and expanded the breaking change notes.

Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py
Comment thread homeassistant/components/totalconnect/__init__.py
Comment thread homeassistant/components/totalconnect/alarm_control_panel.py Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

There's only some small fixes left needed here.

@austinmroczek
Copy link
Copy Markdown
Contributor Author

Sorry I’ve been away. Should finish up this weekend

Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Comment thread homeassistant/components/totalconnect/alarm_control_panel.py Outdated
Comment thread homeassistant/components/totalconnect/__init__.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Great!

@MartinHjelmare MartinHjelmare merged commit 369e6a3 into home-assistant:dev Jul 14, 2019
@balloob balloob mentioned this pull request Aug 7, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Oct 12, 2019
…4427)

* Move totalconnect component toward being a multi-platform integration.  Bump total_connect_client to 0.28.

* add missing total-connect alarm state mappings

* Made recommended changes of MartinHjelmare at
home-assistant#24427

* Update __init__.py

* Updates per MartinHjelmare comments

* flake8/pydocstyle fixes

* removed . at end of log message

* added blank line between logging and voluptuous

* more fixes
springstan added a commit that referenced this pull request Feb 1, 2020
* Bump skybellpy to 0.4.0

* Bump skybellpy to 0.4.0 in requirements_all.txt

* Added extra states for STATE_ALARM_TRIGGERED to allow users to know if
it is a burglar or fire or carbon monoxide so automations can take
appropriate actions.  Updated TotalConnect component to handle these new
states.

* Fix const import

* Fix const import

* Fix const imports

* Bump total-connect-client to 0.26.

* Catch details of alarm trigger in state attributes.

Also bumps total_connect_client to 0.27.

* Change state_attributes() to device_state_attributes()

* Move totalconnect component toward being a multi-platform integration.  Bump total_connect_client to 0.28.

* add missing total-connect alarm state mappings

* Made recommended changes of MartinHjelmare at
#24427

* Update __init__.py

* Updates per MartinHjelmare comments

* flake8/pydocstyle fixes

* removed . at end of log message

* added blank line between logging and voluptuous

* more fixes

* Adding totalconnect zones as HA binary_sensors

* fix manifest.json

* flake8/pydocstyle fixes.  Added codeowner.

* Update formatting per @springstan guidance.

* Fixed pylint

* Add zone ID to log message for easier troubleshooting

* Account for bypassed zones in update()

* More status handling fixes.

* Fixed flake8 error

* Another attempt at black/isort fixes.

* Bump total-connect-client to 0.50.  Simplify code using new functions in
total-connect-client package instead of importing constants.  Run black
and isort.

* Fix manifest file

* Another manifest fix

* one more manifest fix

* more manifest changes.

* sync up

* fix indent

* one more pylint fix

* Hopefully the last pylint fix

* make variable names understandable

* create and fill dict in one step

* Fix name and attributes

* rename to logical variable in alarm_control_panel

* Remove location_name from alarm_control_panel attributes since it is
already the name of the alarm.

* Multiple fixes to improve code per @springstan suggestions

* Update homeassistant/components/totalconnect/binary_sensor.py

Co-Authored-By: springstan <46536646+springstan@users.noreply.github.com>

* Multiple changes per @MartinHjelmare review

* simplify alarm adding

* Fix binary_sensor.py is_on

Co-authored-by: springstan <46536646+springstan@users.noreply.github.com>
bdraco pushed a commit that referenced this pull request Apr 13, 2020
* Bump skybellpy to 0.4.0

* Bump skybellpy to 0.4.0 in requirements_all.txt

* Added extra states for STATE_ALARM_TRIGGERED to allow users to know if
it is a burglar or fire or carbon monoxide so automations can take
appropriate actions.  Updated TotalConnect component to handle these new
states.

* Fix const import

* Fix const import

* Fix const imports

* Bump total-connect-client to 0.26.

* Catch details of alarm trigger in state attributes.

Also bumps total_connect_client to 0.27.

* Change state_attributes() to device_state_attributes()

* Move totalconnect component toward being a multi-platform integration.  Bump total_connect_client to 0.28.

* add missing total-connect alarm state mappings

* Made recommended changes of MartinHjelmare at
#24427

* Update __init__.py

* Updates per MartinHjelmare comments

* flake8/pydocstyle fixes

* removed . at end of log message

* added blank line between logging and voluptuous

* more fixes

* Adding totalconnect zones as HA binary_sensors

* fix manifest.json

* flake8/pydocstyle fixes.  Added codeowner.

* Update formatting per @springstan guidance.

* Fixed pylint

* Add zone ID to log message for easier troubleshooting

* Account for bypassed zones in update()

* More status handling fixes.

* Fixed flake8 error

* Another attempt at black/isort fixes.

* Bump total-connect-client to 0.50.  Simplify code using new functions in
total-connect-client package instead of importing constants.  Run black
and isort.

* Fix manifest file

* Another manifest fix

* one more manifest fix

* more manifest changes.

* sync up

* fix indent

* one more pylint fix

* Hopefully the last pylint fix

* make variable names understandable

* create and fill dict in one step

* Fix name and attributes

* rename to logical variable in alarm_control_panel

* Remove location_name from alarm_control_panel attributes since it is
already the name of the alarm.

* Multiple fixes to improve code per @springstan suggestions

* Update homeassistant/components/totalconnect/binary_sensor.py

Co-Authored-By: springstan <46536646+springstan@users.noreply.github.com>

* Multiple changes per @MartinHjelmare review

* simplify alarm adding

* Fix binary_sensor.py is_on

* Move DOMAIN to .const in line with examples.

* Move to async_setup

* Simplify code using new features of total-connect-client 0.51

* First crack at config flow for totalconnect

* bump totalconnect to 0.52

* use client.is_logged_in() to avoid total-connect-client details.

* updated generated/config_flow.py

* use is_logged_in()

* Hopefully final touches for config flow

* Add tests for config flow

* Updated requirements for test

* Fixes to test_config_flow

* Removed leftover comments and code

* fix const.py flake8 error

* Simplify text per @Kane610 #32126 (review)

* Remove .get() to speed things up since the required items should always
be available.

* Move CONF_USERNAME and CONF_PASSWORD into .const to eliminate extra I/O
to import from homeassistant.const

* Fix I/O async issues

* Fix flake8 and black errors

* Mock the I/O in tests.

* Fix isort error

* Empty commit to re-start azure pipelines (per discord)

* bump total-connect-client to 0.53

* Update homeassistant/components/totalconnect/__init__.py

Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>

* Update homeassistant/components/totalconnect/config_flow.py

Co-Authored-By: Paulus Schoutsen <paulus@home-assistant.io>

* Fixes per @balloob comments

* Fix imports

* fix isort error

* Fix async_unload_entry

It still referenced CONF_USERNAME instead of entry.entity_id

* Added async_setup so not breaking change.  Fixed imports.

* Update tests/components/totalconnect/test_config_flow.py

Co-Authored-By: Martin Hjelmare <marhje52@gmail.com>

* Remove TotalConnectSystem() per @MartinHjelmare suggestion

* Moved from is_logged_in() to is_valid_credentials()

The second is more accurate for what we are checking for, because is_logged_in() could return False due to connection error.

* Fix import in test

* remove commented code and decorator

* Update tests/components/totalconnect/test_config_flow.py

Co-Authored-By: Martin Hjelmare <marhje52@gmail.com>

* fix test_config_flow.py

* bump total-connect-client to 0.54

* remove un-needed import of mock_coro

* bump to total-connect-client 0.54.1

* re-add CONFIG_SCHEMA

* disable pylint on line 10 to avoid pylint bug

Co-authored-by: springstan <46536646+springstan@users.noreply.github.com>
Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
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