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 zeroconf discovery support to Brother Printer integration #30959
Conversation
return self.async_abort(reason="connection_error") | ||
|
||
try: | ||
if "Brother" not in user_input["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.
this is very weird code. Just do something like this pleaes:
if "Brother" not in user_input.get("name"):
Also, do you want to check that it starts with, or is "Two Brothers on the fourth floor" ok too ?
# pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 | ||
self.context.update( | ||
{ | ||
CONF_HOST: host, |
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 don't set the host in context
. If it's for using the flow, just store it on self
"""Handle a flow initiated by zeroconf.""" | ||
try: | ||
# pylint: disable=no-member # https://github.com/PyCQA/pylint/issues/3167 | ||
brother = Brother(self.context.get(CONF_HOST)) |
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.
Why are we making this connection again? We should just keep this object around from async_step_zeroconf
?
if user_input is not None: | ||
|
||
# Check if already configured | ||
await self.async_set_unique_id(brother.serial.lower()) |
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.
You already did this in the previous step .
} | ||
) | ||
return self.async_show_form( | ||
step_id="zeroconf_confirm", |
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.
Don't render forms for other steps. Instead return await self.async_step_zeroconf_confirm()
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.
Looks good!
Breaking Change:
None
Description:
This PR adds zeroconf discovery support to Brother Printer integration.
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
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: