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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use common strings in opentherm_gw config flow #42211
Use common strings in opentherm_gw config flow #42211
Conversation
"serial_error": "[%key:common::config_flow::error::cannot_connect%]", | ||
"timeout": "[%key:common::config_flow::error::cannot_connect%]" |
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.
If we're going to use the same error for both cases, we may as well merge them into one cannot_connect
error condition in the config flow.
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.
True. Originally I did not want to touch the code, but I'll add a commit to reduce it to a single check.
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.
Ok, commited that.
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.
And I shouldn't have commited without running the tests again, so another commit to fix the test.
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! Thanks for the work. The only small change you could additionally consider is a more general name for the error key (timeout
may be confusing for future code maintenance if it's not necessarily caused by a timeout)
Ok, changed the name of the error key. |
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 馃憤
* Use common strings in opentherm_gw config flow * Combine TimeourError and SerialException handling * Fixed test_form_connection_error * And then fix the flake8 error... * Change "timeout" to "cannot_connect" msg
Proposed change
Contribute to issue #40578
Type of change
Example entry for
configuration.yaml
:# Example configuration.yaml
Additional information
I did not convert strings
device
orid_exists
to common strings because there are no common strings that fully cover the intended message.For
device
it is uncommon that either a path (/dev/ttyS1
) or an URL can be entered.For
id_exists
, the error message describes that the given ID is the issue and not the combinatie of ID and device.Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: