Skip to content
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

Nissanleaf #21145

Merged
merged 32 commits into from Feb 22, 2019

Conversation

@filcole
Copy link
Contributor

filcole commented Feb 17, 2019

Description:

Follow-up to pull request #19786 for Nissan Leaf integration to try to fix issues identified by @MartinHjelmare.

Improvements remaining to be made (at least!):

  • how to implement start charging. Switch (status quo), scene + sensor, or service + sensor.
  • icons
  • remove time.sleep(x)

**Pull request for home-assistant.io with documentation has already been merged: home-assistant/home-assistant.io#8073 but needs to be updated to include new service.

Example entry for configuration.yaml:

nissan_leaf:
  username: "username"
  password: "password"
  region: 'NE'     # Indicates Europe, must be changed for other regions
  nissan_connect: true  # Set to false for early Leafs 24kWh that don't report location
  update_interval: 
    hours: 1
  update_interval_charging: 
    minutes: 15
  update_interval_climate: 
    minutes: 5
  force_miles: true

Two new services exposed:

- service: nissan_leaf.update
  data:
    vin: XXXXXXXXXX

- service: nissan_leaf.start_charge
  data:
    vin: XXXXXXXXXX

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:

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

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • 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

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 17, 2019

Please rebase the branch on latest dev, so only the new commits after the last PR was merged are in this branch.

@filcole filcole force-pushed the filcole:nissanleaf branch from 09f5fbe to 75904d9 Feb 18, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 18, 2019

For charging I think a custom service in the component + a sensor or binary sensor entity that exposes charging status sounds good.

Show resolved Hide resolved homeassistant/components/nissan_leaf/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/__init__.py
Show resolved Hide resolved homeassistant/components/nissan_leaf/device_tracker.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/sensor.py Outdated
Show resolved Hide resolved homeassistant/components/nissan_leaf/switch.py
Show resolved Hide resolved homeassistant/components/nissan_leaf/switch.py Outdated
# Maybe it should be a scene?
# Unsure if better to provide as a sensor for the state, and a service to
# start a charge, e.g.
# - service: nissan_leaf.start_charging

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Feb 18, 2019

Member

Yes! Service + sensor is a good idea! 👍

The service should be registered in the nissan_leaf component.

This comment has been minimized.

@filcole

filcole Feb 18, 2019

Author Contributor

Converted to a service and sensor in commit 9fa7369

@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Great!

@MartinHjelmare MartinHjelmare merged commit caa3b12 into home-assistant:dev Feb 22, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on nissanleaf at 92.702%
Details

@wafflebot wafflebot bot removed the in progress label Feb 22, 2019

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Feb 22, 2019

Please link a docs PR with the update for the new service.

@filcole

This comment has been minimized.

Copy link
Contributor Author

filcole commented Feb 23, 2019

Here's the PR for the updated docs that include the new service, home-assistant/home-assistant.io#8630

@Cloudenius

This comment has been minimized.

Copy link

Cloudenius commented Feb 24, 2019

Thanks for this great new component! I'm testing it now with my Nissan Leaf 2018 TEKNA. Everything seems to work fine and fast! I'm confident that this works much better than the Nissan app, as I've been using the other non-official component as well. 👍

@filcole filcole referenced this pull request Mar 2, 2019

Merged

Add nissan_leaf.start_charge service and doc fixes. #8801

2 of 2 tasks complete
@filcole

This comment has been minimized.

Copy link
Contributor Author

filcole commented Mar 2, 2019

Please link a docs PR with the update for the new service.

@MartinHjelmare apologies for the delay - I've linked the new docs PR to this PR.

@balloob balloob referenced this pull request Mar 6, 2019

Merged

0.89.0 #21712

@balloob balloob removed the new-platform label Mar 6, 2019

@BenWoodford

This comment has been minimized.

Copy link

BenWoodford commented Mar 7, 2019

Slight correction in the docs: 2015s have CarWings too. The cut-off was the 16 plate 24kwh and the 65 plate 30kwh I believe.

The easiest way to identify is that Nissan Connect's T&C buttons are orange and blue, CarWings they're both blue. Or just look for anything saying "CarWings" in Settings.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Mar 7, 2019

Please open an issue in or make a PR to the docs repo.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Mar 7, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.