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

Srpenergy #18036

Merged
merged 23 commits into from Nov 8, 2018

Conversation

Projects
None yet
3 participants
@briglx
Contributor

briglx commented Oct 30, 2018

Description:

Platform for retrieving energy data from SRP Energy.

Related issue (if applicable): fixes #

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: srp_energy
    username: YOUR_USERNAME
    password: YOUR_PASSWORD
    id: YOUR_ACCOUNT_ID

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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).
  • New dependencies are only imported inside functions that use them (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.
@homeassistant

This comment has been minimized.

homeassistant commented Oct 30, 2018

Hi @briglx,

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!

@briglx

This comment has been minimized.

Contributor

briglx commented Oct 31, 2018

It looks like it the build server didn't download the dependency from pypi. Any idea on how to fix that?

Could not find a version that satisfies the requirement srpenergy==1.0.1 (from -r /home/travis/build/home-assistant/home-assistant/requirements_all.txt (line 1418)) (from versions: 1.0.0)

Show resolved Hide resolved .coveragerc Outdated
Show resolved Hide resolved homeassistant/components/sensor/srp_energy.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/srp_energy.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/srp_energy.py Outdated
Show resolved Hide resolved homeassistant/components/sensor/srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 31, 2018

@briglx Seems like the library requires Python 3.6+. Home Assistant needs to be compatible with Python 3.5.3+.

https://github.com/lamoreauxlab/srpenergy-api-client-python/blob/master/setup.py#L39

@briglx briglx referenced this pull request Nov 1, 2018

Merged

Add documentation for Srp Energy #7289

1 of 2 tasks complete
@briglx

This comment has been minimized.

Contributor

briglx commented Nov 6, 2018

Looks like the build broke due to something not in my pull request.

pylint runtests: commands[0] | pylint homeassistant
************* Module homeassistant.helpers.device_registry
homeassistant/helpers/device_registry.py:45:4: R1705: Unnecessary "elif" after "return" (no-else-return)

briglx added some commits Oct 29, 2018

@briglx briglx force-pushed the briglx:srpenergy branch from 467c9ab to c2592ce Nov 6, 2018

Show resolved Hide resolved homeassistant/components/sensor/srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
Show resolved Hide resolved tests/components/sensor/test_srp_energy.py Outdated
@MartinHjelmare

Looks good! Can be merged when build passes.

@briglx

This comment has been minimized.

Contributor

briglx commented Nov 8, 2018

Passed! Thanks for all the help

@MartinHjelmare MartinHjelmare merged commit 05eac91 into home-assistant:dev Nov 8, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 93.029%
Details

@wafflebot wafflebot bot removed the in progress label Nov 8, 2018

@briglx briglx deleted the briglx:srpenergy branch Nov 8, 2018

zxdavb added a commit to zxdavb/home-assistant that referenced this pull request Nov 13, 2018

Srpenergy (home-assistant#18036)
* Add srp_energy

* Update message on TypeError. Add check for None state.

* Add check for none in history

* Add srpenergy to Test requirements.

* Add srpenergy to requirments.

* Change = to ==.

* Change import for srpenergy

* Fix Flak8 errors

* Add srp to gen requirements script

* Change config name.

* Add daily usage test

* Add test for daily usage.

* Fix Flake8 message.

* Remove blank after docstring.

* Add srpenergy to coverage

* Bump requires version to srpenergy

* Fix type in coverage. Import from Sensor. Use dict.

* Update to 1.0.5. Check credentials on setup. Standalone test.

* Fix formating.

* Remove period. Rename _ variables.

* Fix rebase merge

* Add rebase requirement

* Improve Mock Patching.

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment