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

New Linky sensor #20535

Closed
wants to merge 26 commits into from
Closed

New Linky sensor #20535

wants to merge 26 commits into from

Conversation

grea09
Copy link
Contributor

@grea09 grea09 commented Jan 28, 2019

Description:

I have coded a new sensor with more informations in attributes and possibly a behaviour change. The code has been done from the original custom component from @Pirionfr. I also fixed a bug in the implementation of the python library used for retrieving data.

This is done to be able to eventually create a lovelace card for energy consumption.
An example of what it could be like: (from @Imbuzi)

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

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: linky
    username: !secret linky_username
    password: !secret linky_password
    peak_hours:
     - ["07:00","14:00"]
     - ["17:00","02:00"]
    peak_hours_cost: 0.1761
    offpeak_hours_cost: 0.1291

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).
  • 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/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
@grea09
Copy link
Contributor Author

grea09 commented Jan 28, 2019

I will update the documentation once this is considered ok.

@frenck
Copy link
Member

frenck commented Jan 28, 2019

Until that times comes, I'm adding the docs-missing label.

homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Show resolved Hide resolved
homeassistant/components/sensor/linky.py Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/linky.py Outdated Show resolved Hide resolved
@grea09
Copy link
Contributor Author

grea09 commented Feb 22, 2019

I was wanting to actually redo this even better to use it with the utility metter. Can someone tell me if it would be bad if I access the database from here ? I need to add a lot of lines in it…

self._peak_hours_cost = peak_hours_cost
self._offpeak_hours_cost = offpeak_hours_cost
self._unit = KILOWATT_HOUR
self._icon = "mdi:flash"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this a constant

password = config[CONF_PASSWORD]
timeout = config[CONF_TIMEOUT]
"""Set up the Linky sensor."""
username = config.get(CONF_USERNAME)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use [] syntax for required and optional with default keys.

self._state = None
self._attributes = {}
self.update()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this outside the init()

_LOGGER.debug("Start of update of linky data")
self._lk.update()
if not self._lk.success:
self._state = STATE_UNAVAILABLE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set this to None. Instead add another instance variable named _available to the sensor and expose it with the property available. Don't forget to mark available = True when the update is successful.

@property
def available(self):
    """Return the availablility of this sensor."""
   return self._available

self._username = username
self._password = password
self._timeout = timeout
self.client = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not create the client here? Regardless it should not be {} but rather None if it doesn't exist.

@rohankapoorcom
Copy link
Member

Please don't forget to run script/gen_requirements_all.py

else:
self._attributes["monthly_evolution"] = (
1 - ((self._lk.monthly[0][CONSUMPTION]) /
(self._lk.compare_month))) * 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is small error in the formula. It should be:

self._attributes["monthly_evolution"] = (
                self._lk.monthly[0][CONSUMPTION] / self._lk.compare_month - 1
            ) * 100

@andrewsayre
Copy link
Member

@grea09 Are you still working on this? It will need to be rebased and updated to match the latest architectural changes.

self._peak_hours = peak_hours
self._peak_hours_cost = peak_hours_cost
self._offpeak_hours_cost = offpeak_hours_cost
self._unit = KILOWATT_HOUR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use homeassistant.const.ENERGY_KILO_WATT_HOUR

else d[CONSUMPTION]
for d in self._lk.halfhourly]) / 2
# From kW for 30 minutes to kWh
self._attributes["peak_offpeak_percent"] = ((self._attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some users share on the forum to have divison by Zero here. Perhaps add some protections.

@grea09
Copy link
Contributor Author

grea09 commented May 7, 2019

I am planning to get 5 students starting on this at the end of the week. I will make sure they share their progress.

@grea09
Copy link
Contributor Author

grea09 commented May 7, 2019

Also I want to get an architectural change in HA core to extend access to history from components. This would impact the recorder and will probably take a lot of discussions to get accepted.

@Quentame
Copy link
Member

Quentame commented May 7, 2019

I've created a simple PR to add more Linky sensors : #23726

I think you can take a look at it and add your work at the linky_today AND linky_yesterday sensors (DAILY scale).

@grea09
Copy link
Contributor Author

grea09 commented May 7, 2019

I've created a simple PR to add more Linky sensors : #23726

I think you can take a look at it and add your work at the linky_today AND linky_yesterday sensors (DAILY scale).

I was more wanting to have the sensor compatible with the utility metter. The issue is that there is no such thing as "today" with Enedis. You can have the consumption of the previous day around 8 o clock in France. So all the history is incorrect. I don't think adding more sensors will help that but it can help making the data clearer. I am personally more convinced to have a "realtime" consumption based on hopefully future API by Enedis or by Bayessian inference in the meantime. Also I would prefer to actually populate the history of the sensor like if it was counting the consumption all along (hence the core change).

I think it would be interesting to think this through. I will make sure my students get in touch with you once they start the work on this.

@Quentame
Copy link
Member

Quentame commented May 8, 2019

Hi @grea09 !

Indeed, hours are yesterday's hours and today isn't updated at all, haven't seen that the first time. So I removed it from my PR #23726.

I agree that realtime API would be nice, but you are talking about High-Tech in France men ... always two step behind 😉

I was more wanting to have the sensor compatible with the utility metter

As I am new in Python and HA dev, I don't know what are you talking about the utility metter 😬 sorry.

Also I would prefer to actually populate the history of the sensor like if it was counting the consumption all along (hence the core change).

As I understand well, making some kind of sum of every day consomption ?
Yeah you can do that too !
But I personally don't feel the need of it, and don't want to charge my DB and proc for that.

I think it would be interesting to think this through. I will make sure my students get in touch with you once they start the work on this.

If you want too, feel free to contact me.

@Quentame
Copy link
Member

Quentame commented May 8, 2019

And as Enedis update Linky's data every day at 8AM, why not update the component only around that time ?

@grea09
Copy link
Contributor Author

grea09 commented May 10, 2019

As I am new in Python and HA dev, I don't know what are you talking about the utility metter sorry.
Here https://www.home-assistant.io/components/utility_meter/

And as Enedis update Linky's data every day at 8AM, why not update the component only around that time ?

They are VERY unreliable. They get something like 50% downtime. They unofficially also have maintenance downtime durring the night. But yes it can be done to update once every halfhour around that time and stop for the day once it got a response.

return

self._state = self._lk.daily[1][CONSUMPTION]
self._attributes["halfhourly"] = [d[CONSUMPTION]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't do these calculations as part of the integration, instead it should be made a standalone integration, like the previous mentioned utility meter.

@balloob
Copy link
Member

balloob commented Jun 6, 2019

Home Assistant integrations should only represent the data from the source as-is. Things like summing up or calculating costs etc should happen in integrations that consume these raw-data-representing-integrations.

This PR is conflating concerns and so should not be merged.

@balloob balloob closed this Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants