-
-
Notifications
You must be signed in to change notification settings - Fork 35k
Convert MetOffice to use UI for configuration #34900
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
Convert MetOffice to use UI for configuration #34900
Conversation
- Sensor and Weather platforms now use config_flow with unique_ids - MetOfficeData object is now async-protected - Integration uses DataUpdateCoordinator and so refreshes correctly - Test cases written to test config_flow and entity behaviours
…gative cases, as well as the positive cases.
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.
FYI, these comments are based on my experience in writing my own weather integration with config flow. I do not claim an authoritative understanding.
- Not all sensor entities are now enabled by default - Changes to the test cases to only therefore test sensor entities that *are* enabled
- Extend test coverage by adding a case that interrupts connection
I haven't looked in detail at your tests, but it seems you are testing all the way down to the platforms and entities. You can probably remove those files from .coveragerc if so. |
I am, yes. I had great fun getting to grips with mock requests and fixed date times, to ensure that the data always matched :) - I'll update .coveragerc as you suggest. |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
But as far as I'm concerned it's ready to go?
|
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.
Mostly just some clean up needed. Some changes around sensor with device classes though.
- Sensor and Weather platforms now use config_flow with unique_ids - MetOfficeData object is now async-protected - Integration uses DataUpdateCoordinator and so refreshes correctly - Test cases written to test config_flow and entity behaviours
…gative cases, as well as the positive cases.
- Not all sensor entities are now enabled by default - Changes to the test cases to only therefore test sensor entities that *are* enabled
- Extend test coverage by adding a case that interrupts connection
…/core into metoffice_config_flow
Did you commit changes without pushing them? Many of my comments have been resolved but have not been addressed in the code. |
D'oh - on it. |
Found an example to work from, then just had to realise to add a proper unique_id for the test location, so the next entry would be refused.
As per review suggestion Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
As per review suggestion Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Serves me right for not going through the proper steps...
With pointers from @MartinHjelmare, finally found the right way to patching datetime.now so as to be able to freeze time for datapoint API to return known weather data for testing.
Thanks to @MartinHjelmare (again) for this very neat way of Mock'ing into datetime.now directly on the patch declaration.
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.
Great!
Please add a breaking change paragraph where we briefly describe what changed and what the user needs to do to cope with the breaking change. |
Done. Thank you! (How do I go about un-"draft"ing the dependent PR that adds forecast data and daily update mode? Presumably I start by merging in all these updates, then...?) |
Yeah, rebase on dev branch after this PR has been merged. |
A question here, since I haven't been able to find the answer in the documents. The documents mention the entities that are enabled by default; but how do I enable additional non-default endpoints? Specifically I am looking for |
Ah, sorry!
If you look in the configuration / integrations / MetOffice you get the
list of current enabled entities...
[image: image.png]
Click on the downward triangle (the filter button) in the top-right of the
view, and you can choose which entities are displayed...
[image: image.png]
From there, you can tick to select the ones you want, then click the button
at the top of the list, to enable the selected entities...
[image: image.png]
Hopefully that should do it for you (on this, and any other integrations
that may start doing this, to you).
…--Ian
On Wed, 8 Jul 2020 at 20:24, Timo ***@***.***> wrote:
A question here, since I haven't been able to find the answer in the
documents. The documents mention the entities that are enabled by default;
but how do I enable additional non-default endpoints?
Specifically I am looking for visibility_distance and humidity.
Previously we set these up under monitored_conditions, but I cannot find
where to set this.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#34900 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQU6A4UXGFHWG4G5K3BHHTR2TBXVANCNFSM4MUEFYYA>
.
|
Oh wow! Apologies. I had no idea! I was looking through it and it didn’t make sense I saw 13 entities, and not all of them after clicking on them.
Thanks again for walking me through that; everything is working again!
… On 8 Jul 2020, at 22:27, Ian Harcombe ***@***.***> wrote:
Ah, sorry!
If you look in the configuration / integrations / MetOffice you get the
list of current enabled entities...
[image: image.png]
Click on the downward triangle (the filter button) in the top-right of the
view, and you can choose which entities are displayed...
[image: image.png]
From there, you can tick to select the ones you want, then click the button
at the top of the list, to enable the selected entities...
[image: image.png]
Hopefully that should do it for you (on this, and any other integrations
that may start doing this, to you).
--Ian
On Wed, 8 Jul 2020 at 20:24, Timo ***@***.***> wrote:
> A question here, since I haven't been able to find the answer in the
> documents. The documents mention the entities that are enabled by default;
> but how do I enable additional non-default endpoints?
>
> Specifically I am looking for visibility_distance and humidity.
> Previously we set these up under monitored_conditions, but I cannot find
> where to set this.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#34900 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAQU6A4UXGFHWG4G5K3BHHTR2TBXVANCNFSM4MUEFYYA>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Breaking change
As this moves the config for the MetOffice weather integration into the UI from the YAML, you will need to ensure you keep a copy of the DataPoint API key handy to re-enable the integration in your server. The location being forecast is, by default, taken as the location of the server, so again if you were forecasting other locations you will need to have those GPS co-ordinates to hand, to re-enter them when configuring new integrations through the UI.
Proposed change
In line with the request made following PR 34642 (link below), this change moves the MetOffice weather integration away from YAML to use the UI (config_flow) configuration.
Type of change
Additional information
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: