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
Dsmr reader #28701
Dsmr reader #28701
Conversation
- Added DSMR Reader platform - Updated codeowners for other components I added earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments in a new PR. Thanks!
{ | ||
"domain": "dsmr_reader", | ||
"name": "DSMR Reader", | ||
"documentation": "https://www.home-assistant.io/components/dsmr_reader", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace components
with integrations
in the url.
|
||
from .definitions import DEFINITIONS | ||
|
||
_LOGGER = logging.getLogger(__name__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger isn't used.
|
||
self._name = self._definition["name"] | ||
self._unit_of_measurement = ( | ||
self._definition["unit"] if "unit" in self._definition else "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can replace the ternary operator with dict.get
. It's built for this case.
self._unit_of_measurement = self._definition.get("unit")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't use empty string for unit of measurement. If there's no unit, return None
as unit of measurement.
|
||
self.async_schedule_update_ha_state() | ||
|
||
return await mqtt.async_subscribe(self.hass, self._topic, message_received, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async_added_to_hass
shouldn't return anything. Please remove the return
and just await
here.
Description:
Add support for DSMR Reader to Home Assistant.
Related issue (if applicable): fixes #
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11156
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices: