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

Add linky sensor #16468

Merged
merged 4 commits into from Sep 24, 2018

Conversation

Projects
None yet
5 participants
@tiste
Contributor

tiste commented Sep 6, 2018

Description:

It adds a component (sensor) showing the last day consumption of your home from the Linky electric meter.

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: linky
    username: !secret linky_username
    password: !secret linky_password

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.

@wafflebot wafflebot bot added the in progress label Sep 6, 2018

@tiste tiste force-pushed the tiste:dev branch from 2ba0c27 to ae5c30d Sep 6, 2018

@tiste tiste force-pushed the tiste:dev branch from ae5c30d to 0ee4242 Sep 6, 2018

@tiste tiste force-pushed the tiste:dev branch from 0ee4242 to f97142f Sep 6, 2018

@tiste tiste force-pushed the tiste:dev branch from f97142f to 0614dc2 Sep 6, 2018

@tiste tiste referenced this pull request Sep 7, 2018

Closed

Add linky component page #6214

2 of 2 tasks complete
@fabaff

Would be nice to use and update release of pylinky when
Pirionfr/pyLinky#3 was addressed.

.coveragerc Outdated
@@ -180,6 +180,8 @@ omit =
homeassistant/components/lametric.py
homeassistant/components/*/lametric.py
homeassistant/components/*/linky.py

This comment has been minimized.

@fabaff

fabaff Sep 7, 2018

Member

Please move it to the others sensors below.

password = config.get(CONF_PASSWORD)
from pylinky.client import LinkyClient
client = LinkyClient(username, password)

This comment has been minimized.

@fabaff

fabaff Sep 7, 2018

Member

Please check if the credentials are valid. If not, make the platform setup fail with a log entry. Otherwise will the users end-up with a platform which is not working in their instance.

This comment has been minimized.

@tiste

tiste Sep 7, 2018

Contributor

Actually, the constructor is doing nothing except storing credentials.
fetch_data() is the only method that login the user and load its data, that's the reason why it's inside the try/catch block.

This comment has been minimized.

@fabaff

fabaff Sep 8, 2018

Member

You should make the first call in setup_platform in this case to ensure that the credentials are valid.

_LOGGER.debug(json.dumps(self._client.get_data(), indent=2))
if self._client.get_data():

This comment has been minimized.

@fabaff

fabaff Sep 7, 2018

Member

Handle this in the try-except block. Return as early as possible.

This comment has been minimized.

@tiste

tiste Sep 7, 2018

Contributor

get_data() is a getter, not an API call (fetch_data() is). Are you sure to want to put it into the try/catch block?

This comment has been minimized.

@fabaff

fabaff Sep 8, 2018

Member

If fetch_data() fails then it doesn't make sense to go on with update().

@fabaff fabaff referenced this pull request Sep 7, 2018

Closed

Timeouts #3

@tiste tiste force-pushed the tiste:dev branch from 0614dc2 to ba0e529 Sep 7, 2018

REQUIREMENTS = ['pylinky==0.1.5']
_LOGGER = logging.getLogger(__name__)
DOMAIN = 'linky'

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 8, 2018

Member

Remove this. This platform belongs to the sensor domain.

})
def setup_platform(hass, config, add_devices, discovery_info=None):

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 8, 2018

Member

Rename add_devices to add_entities.

def setup_platform(hass, config, add_devices, discovery_info=None):
"""Configure the platform and add the Linky sensor."""
username = config.get(CONF_USERNAME)

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 8, 2018

Member

Don't use dict.get use dict[key] for required config keys.

"""Fetch new state data for the sensor."""
try:
self._client.fetch_data()
except BaseException as exp:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 8, 2018

Member

Please only except the exception we expect.

# get the last past day data
self._state = self._client.get_data()['daily'][-2]['conso']
else:
self._state = 0

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 8, 2018

Member

Unknown state should be represented with None.

@tiste tiste force-pushed the tiste:dev branch from ba0e529 to 39ccfc8 Sep 17, 2018

@tiste tiste force-pushed the tiste:dev branch from 39ccfc8 to f81b6a3 Sep 17, 2018

@tiste tiste force-pushed the tiste:dev branch from f81b6a3 to aedd6bb Sep 17, 2018

@tiste tiste force-pushed the tiste:dev branch from aedd6bb to 0d01b57 Sep 17, 2018

@tiste tiste force-pushed the tiste:dev branch from 0d01b57 to 475367d Sep 17, 2018

@tiste tiste force-pushed the tiste:dev branch from 475367d to d13dacb Sep 17, 2018

tiste added some commits Apr 16, 2018

@tiste tiste force-pushed the tiste:dev branch from d13dacb to 33adc55 Sep 18, 2018

timeout = config[CONF_TIMEOUT]
from pylinky.client import LinkyClient
client = LinkyClient(username, password, None, timeout)

This comment has been minimized.

@fabaff

fabaff Sep 20, 2018

Member

If the credentials are wrong make the platform setup fail.

This comment has been minimized.

@tiste

tiste Sep 20, 2018

Contributor

Did you mean "valid" data?
Otherwise, there is no public login function. Everything is done in the fetch_data function.

This comment has been minimized.

@fabaff

fabaff Sep 21, 2018

Member

You need to make the first request to the API during the platform setup and go for the PyLinkyError.

except BaseException as exp:
_LOGGER.error(exp)
_LOGGER.debug(json.dumps(self._client.get_data(), indent=2))

This comment has been minimized.

@fabaff

fabaff Sep 20, 2018

Member

Again, return if you can't retrieve the data. The debug message will be empty, thus it will not be useful.

except PyLinkyError as exp:
_LOGGER.error(exp)
return
except BaseException as exp:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 23, 2018

Member

Remove this.

This comment has been minimized.

@tiste

tiste Sep 23, 2018

Contributor

Don't know if your read a bit about pylinky package, but most of the errors are not handled within PyLinkyError error. So it won't be handled.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 23, 2018

Member

We should only catch specific expected errors, and not use broad except. If there is a specific error that we expect we can add an except block for that.

except PyLinkyError as exp:
_LOGGER.error(exp)
return
except BaseException as exp:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Sep 23, 2018

Member

Remove this.

@tiste tiste force-pushed the tiste:dev branch from e5771a2 to 2fe187d Sep 23, 2018

@tiste tiste referenced this pull request Sep 23, 2018

Merged

Add linky component page #6346

2 of 2 tasks complete
@fabaff

fabaff approved these changes Sep 24, 2018

@fabaff fabaff merged commit 4fd2f77 into home-assistant:dev Sep 24, 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 remained the same at 93.559%
Details

@wafflebot wafflebot bot removed the in progress label Sep 24, 2018

@balloob balloob referenced this pull request Sep 28, 2018

Merged

0.79.0 #16940

@totoroyamada

This comment has been minimized.

totoroyamada commented Oct 3, 2018

Hi, I get login errors although my username and password are ok (i can connect to the Enedis site). Is there any special operation I have to do ? I tried to write my username and pwd without quotes, between quotes, between simple quotes, I even tried several combinations (username between quotes, pwd without etc...) but it doesn't change anything. Am I missing something ?
Thanks !

@MartinHjelmare

This comment has been minimized.

Member

MartinHjelmare commented Oct 3, 2018

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Oct 3, 2018

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