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

Add edl21 component for SML-based smart meters #27962

Merged
merged 19 commits into from Mar 5, 2020
Merged

Add edl21 component for SML-based smart meters #27962

merged 19 commits into from Mar 5, 2020

Conversation

mtdcr
Copy link
Contributor

@mtdcr mtdcr commented Oct 20, 2019

Description:

This PR integrates EDL21 compatible smart meters as used in Germany. These smart meters use SML protocol messages to transmit various „OBIS“ values.

Pull request with documentation for home-assistant.io: home-assistant/home-assistant.io#11325

Example entry for configuration.yaml:

sensor:
  - platform: edl21
  - serial_port: socket://10.2.3.4:7750

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.
  • I have followed the development checklist

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

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@mtdcr
Copy link
Contributor Author

mtdcr commented Nov 27, 2019

Rebased and updated pysml dependency from 0.0.1 to 0.0.2.

@DavidMStraub
Copy link
Contributor

Thanks for this very nice component! I tested it and it works very well! For the documentation that is still to be added, I suggest to mention also

  • which hardware (IR diode) is known to work
  • how to set up ser2net to transmit serial over the network (thanks for helping me with this)

A useful feature in my opinion would be to customize the update interval. For instance, my smart meter reports the power to 0.01 W and energy to 0.1 mWh precision every second, which is really excessive. Every 10 or 30 seconds would be more than enough for me (and fill the history less). I think this could be achieved easily by adding a scan_interval config option and adding a homeassistant.util.Throttle decorator to the EDL21Entity.update_telegram method.

@mtdcr
Copy link
Contributor Author

mtdcr commented Dec 1, 2019

  • which hardware (IR diode) is known to work

Already done, see home-assistant/home-assistant.io#11325.

  • how to set up ser2net to transmit serial over the network (thanks for helping me with this)

I'm not sure about this. It is a feature of pyserial in general and applies to many more components supported by Home Assistant.

Every 10 or 30 seconds would be more than enough for me (and fill the history less). I think this could be achieved easily by adding a scan_interval config option and adding a homeassistant.util.Throttle decorator to the EDL21Entity.update_telegram method.

I'll look into it.

@DavidMStraub
Copy link
Contributor

FYI, I tested adding

from datetime import timedelta
from homeassistant.util import Throttle

MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=30)

# ...

    @Throttle(MIN_TIME_BETWEEN_UPDATES)
    def update_telegram(self, telegram: dict) -> bool:
        # ...

and it works as expected. When using a config option for the update interval (rather than hard-coding it), I guess one cannot use the decorator but can patch the method after instantiating the EDL21Entity.

@mtdcr
Copy link
Contributor Author

mtdcr commented Dec 7, 2019

Thank you, David! Would you mind creating a PR on my branch, to preserve your authorship?

I'd prefer 60 seconds, however, because that would match the interval used by the volkszaehler component. dsmr doesn't do any throttling, AFAICS. Are there any other smart meter components in HA?

@codecov

This comment has been minimized.

homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
Dev automation moved this from Needs review to Review in progress Feb 16, 2020
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Looks great. One final comment 🎉

homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Show resolved Hide resolved
homeassistant/components/edl21/sensor.py Outdated Show resolved Hide resolved
@mtdcr
Copy link
Contributor Author

mtdcr commented Feb 18, 2020

Thank you for your reviews! I hope to have addressed everything by now.

Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

Just a question.

self._last_update = utcnow()
self._state_attrs = {
"status": "status",
"valTime": "val_time",
Copy link
Member

Choose a reason for hiding this comment

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

What kind of time is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an optional timestamp which indicates when the measurement was performed. My device doesn't use it, though.

Copy link
Member

Choose a reason for hiding this comment

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

Ok 👍

Absolute timestamps are ok and they should be UTC.

Dev automation moved this from Review in progress to Reviewer approved Feb 18, 2020
@balloob balloob merged commit 2316f7a into home-assistant:dev Mar 5, 2020
Dev automation moved this from Reviewer approved to Done Mar 5, 2020
@lock lock bot locked and limited conversation to collaborators Mar 10, 2020
@mtdcr mtdcr deleted the edl21 branch April 10, 2021 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants