WIP: Add iCloud devices battery status + services by Quentame · Pull Request #24053 · home-assistant/core

Quentame · 2019-05-23T12:10:51Z

Breaking Change:

The iCloud component leaves the device_tracker component to be a all in one component (device_tracker + sensor)

From:

device_tracker:
  - platform: icloud
    username: USERNAME
    password: PASSWORD

To:

icloud:
  username: USERNAME
  password: PASSWORD

It also adds some services :
SERVICE_ICLOUD_PLAY_SOUND = 'play_sound'
SERVICE_ICLOUD_DISPLAY_MESSAGE = 'display_message'
SERVICE_ICLOUD_LOST_DEVICE = 'lost_device'
SERVICE_ICLOUD_UPDATE = 'update'
SERVICE_ICLOUD_RESET = 'reset'

And rename some :
'icloud_lost_device' --> 'lost_device'
'icloud_update' --> 'update'
'icloud_reset' --> 'reset'

Description:

Hi everyone!

I would like to created sensors entity for all Apple devices link to an iCloud account, as I already did it, but using the template platform, and I saw a lot of people doing this.

I am new at Python development and even more in HomeAssistant dev, so don't hesitate to comment.

All reviewers and reviews are welcome 😉 !

Done:

iCloud data retrieved and fetched
iCloud data updated at interval
iCloud data stored
sensor created and updated
device_tracker created and updated
add default icons for device_tracker from device "class" (a.k.a iPhone, iPad, iPod, iMac & MacBook)

Remaining work:

Take care of CONF_GPS_ACCURACY_THRESHOLD
~~Delete CONF_MAX_INTERVAL if first optional work not made~~
Add the iCloud integration from the config flow
Translate the component

Optional work:

Retreive iCloud data at determined interval (depending on speed and battery), but I'm not sure if it's necessary because we are not asking the device directly, but the iCloud service, which updates himself to a certain interval.
Not trigger device_tracker.see if device.statut != 'online' to make the device "Away" since it is shut dow even if it is still at the same place. Now, the position of the device is updated once it's connected to the internet and if the device is off, the tracker will still say "Home" even after hours)

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
NOT YET

Example entry for `configuration.yaml` (if applicable):

icloud:
  username: USERNAME
  password: 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.

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

Documentation added/updated in home-assistant.io

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

The manifest file has all fields filled out correctly (example).
New dependencies have been added to requirements in the manifest (example).
New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

MartinHjelmare

I've made some comments after my first brief read and impressions.

MartinHjelmare · 2019-07-09T19:31:45Z

+
+        if self._accountname in _CONFIGURING:
+            request_id = _CONFIGURING.pop(self._accountname)
+            configurator = self._hass.components.configurator


The configurator is deprecated. Please make use of a config flow and config entry instead.

https://developers.home-assistant.io/docs/en/config_entries_index.html

Yeah, that would be very nice, I thought about that too.

But should I make it in a new PR to be less messy, then rebase this one, or directly here ?

You can do it in a separate PR if you prefer that. We like small PRs. 👍

Hi !

I have some problem to initiate the config flow, can you take a look at this commit (d2bf926) to help me a bit ?

I inspired me from AdGuard Home, Hangout, Hue, and HomeKit.

I have to logged error event if I added some at async_setup_entry, async_step_user ...

And nothing came at the integration panel.

You can notice the "Search" label not well placed, but it used to most of the time (Safari 12.1.1)

I think that I am missing a little thing but I don't know what.

Also :
How can I replace self._hass.config.path('icloud') in the config flow as I can't access _hass ?

Thanks.

hass can be accessed as self.hass in the config flow methods. That is set by the core when instantiating the flow.

If strings are missing in the frontend, that can be due to missing translation file. Run the translation script to generate the default english translation file from the strings.json file that you should create manually in the package:
https://developers.home-assistant.io/docs/en/internationalization_backend_localization.html#translation-strings
https://developers.home-assistant.io/docs/en/internationalization_backend_localization.html#configuration-flow-localization

Also make sure you have registered the config flow class:
https://developers.home-assistant.io/docs/en/data_entry_flow_index.html#flow-handler

Thanks for the hass

I've created a translation and had registered the config flow here, still nothing.

Make sure you have activated config flow in manifest.json.

Same with "config_flow": true, in the manifest

MartinHjelmare · 2019-07-09T19:41:52Z

-
-    else:
-        _LOGGER.error("No ICLOUDTRACKERS added")
+def setup_scanner(hass, config, see, discovery_info=None):


After moving this integration to use config entries, the device tracker platform should subclass the new TrackerEntity class in the device tracker component.

See the gpslogger integration for example:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/gpslogger/device_tracker.py

MartinHjelmare · 2019-07-25T13:12:51Z

+            accountname = slugify(accountname.partition('@')[0])
+            hass.data[DATA_ICLOUD][accountname].reset_account()
+
+    hass.services.register(


Use the home assistant async api in async context. hass.services.async_register

MartinHjelmare · 2019-07-25T13:18:27Z

+
+    def icloud_need_trusted_device(self):
+        """We need a trusted device."""
+        configurator = self.hass.components.configurator


This should be replaced with config flow steps.

MartinHjelmare · 2019-07-25T13:19:39Z

+    "config": {
+        "title": "Apple iCloud",
+        "step": {
+            "init": {


This should be the user step.

MartinHjelmare · 2019-07-25T13:21:04Z

+                "data": {
+                    "username": "Username",
+                    "password": "Password"
+                }


Make sure that all config flow form items are represented here in the data.

MartinHjelmare · 2019-07-25T13:22:58Z

@@ -0,0 +1,106 @@
+"""Battery state for iCloud devices."""


Please don't add new platforms in this PR. Let's do that in subsequent PRs.

Actually, this PR was originally made to add this platform, but, config flow first !

MartinHjelmare · 2019-08-08T11:03:26Z

@Quentame are you planning to finish here?

Quentame · 2019-08-09T19:55:55Z

Hi @MartinHjelmare, nope ! 😉

I've actually dev the config flow for iCloud and it's working (and taking care of your comments). I mean kinda since the UI is working, but I can't really add the integration due to iCloud API is not working right now (#24476).

So I'm gonna create a new PR, for the config flow only, when the API will be back and I can test all the component, or should I make a draft PR first ?
It will also fix #20195 and #13312.

Then a second one to add the battery sensors (new platform).

Maybe new ones also to add iCloud calendar events and other sensors ...

So I'm closing this PR.

Quentame · 2019-09-11T21:23:45Z

New PR is opened 😉

homeassistant added cla-signed integration: icloud new-platform labels May 23, 2019

balloob added the breaking-change label May 23, 2019

Quentame force-pushed the icloud_add_devices_battery_status branch 3 times, most recently from 0e4f9a8 to e001f3a Compare June 6, 2019 11:57

Quentame force-pushed the icloud_add_devices_battery_status branch from e001f3a to 31cb436 Compare June 14, 2019 11:56

Quentame changed the title ~~Add iCloud devices battery status + services~~ WIP: Add iCloud devices battery status + services Jun 24, 2019

Quentame force-pushed the icloud_add_devices_battery_status branch 2 times, most recently from 713b140 to 0693c23 Compare July 6, 2019 12:02

Quentame mentioned this pull request Jul 9, 2019

WIP: Add iCloud devices battery status #23682

Closed

17 tasks

MartinHjelmare requested changes Jul 9, 2019

View reviewed changes

Quentame force-pushed the icloud_add_devices_battery_status branch 2 times, most recently from c7e2492 to 9b01408 Compare July 11, 2019 12:01

Quentame force-pushed the icloud_add_devices_battery_status branch 3 times, most recently from d2bf926 to cad0571 Compare July 24, 2019 11:10

Quentame added 4 commits July 25, 2019 13:07

Add iCloud devices battery status + services

3dbe06d

WIP: determine_interval

d181c43

Fixes : for in loops + flake8 + remove useless logs

6cbcf6f

Fixing review from @MartinHjelmare part 1

1c757fc

Quentame force-pushed the icloud_add_devices_battery_status branch from cad0571 to 0dc0dfa Compare July 25, 2019 11:42

[WIP] Config Entry & Flow convertion

384d945

Quentame force-pushed the icloud_add_devices_battery_status branch from 0dc0dfa to 384d945 Compare July 25, 2019 11:45

MartinHjelmare reviewed Jul 25, 2019

View reviewed changes

Quentame closed this Aug 9, 2019

Quentame mentioned this pull request Sep 11, 2019

Add config flow to iCloud #26590

Closed

8 tasks

Quentame mentioned this pull request Nov 22, 2019

Add config flow to iCloud #28968

Merged

8 tasks

Quentame deleted the icloud_add_devices_battery_status branch December 11, 2019 22:53

Uh oh!

Conversation

Quentame commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking Change:

Description:

Done:

Remaining work:

Optional work:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quentame Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MartinHjelmare Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare commented Aug 8, 2019

Uh oh!

Quentame commented Aug 9, 2019

Uh oh!

Quentame commented Sep 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Quentame commented May 23, 2019 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

Quentame Jul 24, 2019 •

edited

Loading

MartinHjelmare Jul 9, 2019 •

edited

Loading