Add new component: BMW connected drive #12277

ChristianKuehnel · 2018-02-10T17:41:19Z

Description:

This is a basic implementation of a sensor for BMW Connected Drive vehicles. In the current version your're getting:

device tracker for your vehicle
current mileage, fuel and remaining range

I could only test this with one BMW model. So there might be changes necessary to make it work with other models...

Related issue (if applicable): N/A

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4617

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

bmw_connected_drive:
  mycar:
    username: your_name
    password: your_password
    country: country_of_your_connected_drive_account

Checklist:

The code change is tested and works locally.

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

Documentation added/updated in home-assistant.github.io

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

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

houndci-bot · 2018-02-10T17:41:26Z

homeassistant/components/bmw_connected_drive.py

+        from bimmer_connected import BimmerConnected
+        self.bimmer = BimmerConnected(vin, username, password, cache=True)
+        self.name = name
+


blank line at end of file

gerard33 · 2018-02-11T15:46:04Z

@ChristianKuehnel, let's get in touch to see how we can join forces.
I was already working on a BMW ConnectedDrive component as well, https://community.home-assistant.io/t/wip-bmw-connected-drive-custom-component/39569/65.

I my version the VIN isn't needed and I have some extra sensors and also switches in use.
I also have some API output for an electric (i3) and hybrid car. So we can use that to support those cars as well. And my X1 has head_unit: EntryEvo, so that's something that can be supported as well in your bimmer module on PyPi.

I was facing an issue however with warnings that updating the sensors took longer than the scheduled update interval of 0:00:30. @danielperna84 was just now helping me to find the cause of that issue.
So I suggest to expand your version with the useful parts of my code as mentioned above.

danielperna84 · 2018-02-11T15:49:07Z

What I noticed while looking at bimmer-connected: it uses the german API endpoint. Having followed the thread @gerard33 has mentioned it is actually required to be able to use different URLs so people from other countries can use this as well.

gerard33 · 2018-02-11T15:55:29Z

I tried to use the German url and that actually worked, but then I got errors using the BMW CD app, so using the local url is indeed the best solution.
These are all mentioned on www.bmw-connecteddrive.com.

ChristianKuehnel · 2018-02-11T16:15:33Z

@gerard33 just commented on the community page, lets discuss there how to proceed.
https://community.home-assistant.io/t/wip-bmw-connected-drive-custom-component/39569/65.

* Update are now triggered from BMWConnectedDriveVehicle. * removed polling from sensor and device_tracker * backend URL is not detected automatically based on current country * vehicles are discovered automatically * updates are async now resolves: * bimmerconnected/bimmer_connected#3 * bimmerconnected/bimmer_connected#5

houndci-bot · 2018-02-14T23:01:14Z

homeassistant/components/sensor/bmw_connected_drive.py

+"""
+import logging
+
+from homeassistant.components.bmw_connected_drive \


'homeassistant.components.bmw_connected_drive.BMWConnectedDriveEntity' imported but unused

houndci-bot · 2018-02-14T23:01:14Z

homeassistant/components/bmw_connected_drive.py

+        password = account_config[CONF_PASSWORD]
+        country = account_config[CONF_COUNTRY]
+        _LOGGER.debug('Adding new account %s', name)
+        bimmer = BMWConnectedDriveEntity(hass, username, password, country, name)


line too long (81 > 79 characters)

ChristianKuehnel · 2018-02-14T23:04:07Z

@danielperna84 the portal is now selected based on the country you define as config parameter.

@gerard33 I'm done with my major refactoring of the setup and update process. Now the update is triggered by a timer in the components/bmw_connected_drive.py and that then triggers the different sensors.

ChristianKuehnel · 2018-02-15T19:23:47Z

I ran the current implementation over the last 24 hours in my test setup and it worked fine. It does not yet contain all the values you could get from your BMW but I think we could start rolling it out and get some feedback from the users. I suspect there might be some issues with different BMW models and different countries.

There is a ton of more features available from the BMW portal, but we can add these later on.

ChristianKuehnel · 2018-02-17T17:29:04Z

@MartinHjelmare thx for the extensive code review! I just uploaded the changes.

To be honest, I did not really understand how async works in Home Assistant. Is there a good documentation available?

MartinHjelmare · 2018-02-17T17:33:42Z

Did you see https://home-assistant.io/developers/asyncio/?

MartinHjelmare · 2018-02-18T21:05:34Z

homeassistant/components/bmw_connected_drive.py

+    'mileage',
+    ]
+
+VAILD_ATTRIBUTES = LENGTH_ATTRIBUTES + [


Typo, same as before.

Ah, yes was a copy an past error. I removed the duplicated code.

MartinHjelmare · 2018-02-18T21:12:05Z

homeassistant/components/bmw_connected_drive.py

+    def add_update_listener(self, listener):
+        """Add a listener for update notifications."""
+        self._update_listeners.append(listener)
+        listener()


I don't think you should call the callback from this method. It's better to separate responsibilities, and let the caller call the callback explicitly if needed.

MartinHjelmare · 2018-02-18T21:20:47Z

homeassistant/components/device_tracker/bmw_connected_drive.py

+        )
+
+    @asyncio.coroutine
+    def async_added_to_hass(self):


You can't use this method here, since this is not an entity. The device tracker component is special like that. It creates its own entities and you can only create a device class that reports to the entity via the see method.

I don't think there will be a race for the device tracker if you register and call the callback during setup_scanner.

MartinHjelmare · 2018-02-18T21:22:25Z

homeassistant/components/sensor/bmw_connected_drive.py

+        Show latest data after startup.
+        """
+        self._account.add_update_listener(self.update)
+        self.update()


Remove this.

What should I remove?

The self.update()? I need this so the state of the sensor is available right after the start of hass. If I want to trigger this from the central component, I would have to find out when all of the setup operations were completed.

Yes remove this line here. See my comment above about how to add devices so that update will be called when they are added.

Ok, so an alternative if you really want to keep the logging with entity_id in update is to schedule a call to update here. In that case, use yield from self.hass.async_add_job(self.update).

When I call add_devices(devices, True) from setup_platform even without me logging the entity_id, I get this error:

2018-02-19 16:55:32 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved Traceback (most recent call last): File "/usr/lib/python3.5/asyncio/tasks.py", line 239, in _step result = coro.send(None) File "/home/kuehnelc/git/home-assistant/homeassistant/helpers/entity.py", line 196, in async_update_ha_state "No entity id specified for entity {}".format(self.name)) homeassistant.exceptions.NoEntitySpecifiedError: No entity id specified for entity 530i xDrive remaining_range_fuel

I'm not sure why that is.

Your version with yield from self.hass.async_add_job(self.update) works just as fine as self.update(). So I'll take your version. Not sure why you want update do be called via async_add_job, it's only doing a bit of string processing. There is no IO in the update function. But as it works, I'm fine with that 😄 ...

The update method calls schedule_update_ha_state which needs to be done from a sync context.

Regarding error, yeah sorry, the entity_id is also required when scheduling a state update, ie in schedule_update_ha_state, so the my suggestion to call add_devices like that was totally wrong. The logic about this changed recently and I hadn't really understood all the consequences until now.

MartinHjelmare · 2018-02-18T21:23:40Z

homeassistant/components/sensor/bmw_connected_drive.py

+            for sensor in VALID_ATTRIBUTES:
+                device = BMWConnectedDriveSensor(account, vehicle, sensor)
+                devices.append(device)
+    return add_devices(devices)


Remove the return. Just call add_devices(devices, True) to have the update method be called during entity addition.

add_devices(devices, True) resulted in some weird "no entity_id set" errors.
But `add_devices(devices)' (without true) works just fine.

Ok, the entity_id will not be set during the update before add operation. All other attributes will be available though. Either remove the logging about entity_id in update, or do the schedule the update from async_added_to_hass.

MartinHjelmare

This looks good! Fix the final comment and we can merge.

MartinHjelmare · 2018-02-19T20:45:07Z

homeassistant/components/device_tracker/bmw_connected_drive.py

+        """Initialize the Tracker."""
+        self._see = see
+        self.vehicle = vehicle
+        self._account = account


This doesn't seem to be used.

Ah yes, you have a good eye for these things...

MartinHjelmare

Great!

ChristianKuehnel · 2018-02-20T22:07:02Z

@MartinHjelmare thx for your help! 👍 😄

@gerard33 good news, it's merged 🥂 So you can now add your additional sensors on top of the dev branch.

robbz23 · 2018-03-06T13:17:29Z

@ChristianKuehnel @gerard33 I have a i3 and I am interested in helping although I don't have too much experience in Python, I am pretty good at other languages. How can I help make this better?

ChristianKuehnel · 2018-03-10T20:22:58Z

@robbz23
Do you happen to live in the US or Canada? Because there we're trying to figure out how to make it work there...

robbz23 · 2018-03-10T20:25:10Z

Sorry I'm an American living in Sweden. Both the component and extra work.

robbz23 · 2018-03-10T20:26:32Z

Sorry I'm an American living in Sweden. Both the component and extra work.

ChristianKuehnel added 2 commits February 10, 2018 18:33

first working version of BMW connected drive sensor

7157e28

extended coveragerc

4ab6775

ChristianKuehnel requested a review from andrey-git as a code owner February 10, 2018 17:41

homeassistant added cla-signed new-platform integration: bmw_connected_drive platform: device_tracker.bmw_connected_drive labels Feb 10, 2018

houndci-bot reviewed Feb 10, 2018

View reviewed changes

fixed blank line

5ef0860

ChristianKuehnel mentioned this pull request Feb 10, 2018

documentation for new component bmw_connected_drive home-assistant/home-assistant.io#4617

Merged

2 tasks

fixed pylint

af609ad

homeassistant added the cla-signed label Feb 10, 2018

ChristianKuehnel changed the title ~~new component: BMW connected drive [WIP]~~ new component: BMW connected drive Feb 11, 2018

ChristianKuehnel changed the title ~~new component: BMW connected drive~~ new component: BMW connected drive [WIP] Feb 11, 2018

homeassistant added the cla-signed label Feb 14, 2018

houndci-bot reviewed Feb 14, 2018

View reviewed changes

ChristianKuehnel added 2 commits February 15, 2018 20:07

improved exception handing

93200b6

fixed static analysis findings

72fbb85

homeassistant added the cla-signed label Feb 15, 2018

ChristianKuehnel changed the title ~~new component: BMW connected drive [WIP]~~ new component: BMW connected drive Feb 15, 2018

ChristianKuehnel closed this Feb 17, 2018

ChristianKuehnel reopened this Feb 17, 2018

fixed pylint issue

db0137e

homeassistant added the cla-signed label Feb 17, 2018

ChristianKuehnel added 2 commits February 18, 2018 19:33

updated to latest release of the bimmer_connected library

ae7e786

updated requirements-all.txt

e50ee97

MartinHjelmare requested changes Feb 18, 2018

View reviewed changes

ChristianKuehnel added 2 commits February 19, 2018 09:00

fixed comments from @MartinHjelmare

785138d

calling self.update from async_add_job

84c5a1c

ChristianKuehnel closed this Feb 19, 2018

ChristianKuehnel reopened this Feb 19, 2018

ChristianKuehnel closed this Feb 19, 2018

ChristianKuehnel reopened this Feb 19, 2018

MartinHjelmare requested changes Feb 19, 2018

View reviewed changes

removed unused attribute "account"

347b16c

homeassistant added the cla-signed label Feb 20, 2018

MartinHjelmare approved these changes Feb 20, 2018

View reviewed changes

MartinHjelmare changed the title ~~new component: BMW connected drive~~ Add new component: BMW connected drive Feb 20, 2018

MartinHjelmare merged commit 316eb59 into home-assistant:dev Feb 20, 2018

ChristianKuehnel deleted the bimmer branch February 20, 2018 22:06

balloob mentioned this pull request Feb 22, 2018

0.64.0 #12609

Merged

gerard33 mentioned this pull request Mar 6, 2018

Features for electric/hybrid cars bimmerconnected/bimmer_connected#41

Closed

home-assistant locked and limited conversation to collaborators Jul 26, 2018

ghost removed platform: device_tracker.bmw_connected_drive labels Mar 21, 2019

Add new component: BMW connected drive #12277

Add new component: BMW connected drive #12277

Conversation

ChristianKuehnel commented Feb 10, 2018 • edited by MartinHjelmare

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Choose a reason for hiding this comment

gerard33 commented Feb 11, 2018 • edited

danielperna84 commented Feb 11, 2018

gerard33 commented Feb 11, 2018 • edited

ChristianKuehnel commented Feb 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKuehnel commented Feb 14, 2018

ChristianKuehnel commented Feb 15, 2018

ChristianKuehnel commented Feb 17, 2018

MartinHjelmare commented Feb 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKuehnel Feb 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianKuehnel Feb 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

ChristianKuehnel commented Feb 20, 2018

robbz23 commented Mar 6, 2018

ChristianKuehnel commented Mar 10, 2018

robbz23 commented Mar 10, 2018

robbz23 commented Mar 10, 2018

ChristianKuehnel commented Feb 10, 2018 •

edited by MartinHjelmare

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

gerard33 commented Feb 11, 2018 •

edited

gerard33 commented Feb 11, 2018 •

edited

ChristianKuehnel Feb 19, 2018 •

edited

ChristianKuehnel Feb 19, 2018 •

edited