home-assistant
Update stookwijzer api to atlas leefomgeving and add extra sensors
Breaking change
Stookwijzer states are updated. New possible states are: code_yellow, code_orange and code_red.
Proposed change
The API has changed, also the different advices are added (source https://iplo.nl/thema/lucht/houtstook-de-stookwijzer-en-het-stookalert/)
https://github.com/fwestenberg/stookwijzer/compare/v1.3.0...v1.4.10
To fix the issue mentioned in https://github.com/home-assistant/core/pull/104705, the pyreproj dependency is removed.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [x] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes https://github.com/home-assistant/home-assistant.io/issues/29679
- This PR is related to issue:
- Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/30092
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Ruff (
ruff format homeassistant tests) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [x] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [x] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [x] Untested files have been added to
.coveragerc.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
@joostlek Does this PR look familiar to you? Would you mind reviewing this one as well?
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Learn more about our pull request process.
![home-assistant[bot] avatar](https://avatars.githubusercontent.com/in/97978?v=4 "Published by a pub.dev verified publisher"){kind=small}
Splitting will be a problem. Because of the dependency so many had to change. And I even added a lot of new tests. We can probably take more time for this review and keep it as is?
Yes but a dependency change does not have to change the way we fetch data or the amount of sensors. Like if it was only 1 new entity, sure. But in this case this PR does too much
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. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Some more information about the Stookalert, which is also added to the Stookwijzer integration in this PR. When loading the Stookwijzer website, the following will be shown at the map in case of a Stookalert:
This Stookalert information is not displayed when there currently is no Stookalert. But this means the Stookalert integration perhaps can be migrated to the Stookwijzer integration. This way, only one integration containing all stook-related information is created.
Any progress on this issue? The stookwijzer is to look up local information on the situation, The stookalert is a province wide warning if there is smog for example and they give out a warning not to use your wood stove. I think the stookwijzer is the most important one to look up wether it is ok to use the wood stove.
If you select a location you get local information
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. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
Hi, If this integration isn't going to be usable anymore, it might as wel be removed as an integration.
This integration is working perfectly, running it at my HA for months now without any error. But I’m still waiting for it to be approved…
This integration is working perfectly, running it at my HA for months now without any error. But I’m still waiting for it to be approved…
Ah ok. Well then, hope it gets approved before next winter when the houtstook starts again :)
@fwestenberg @One35SEven For this PR to be merged it needs to be split up as requested by @joostlek February 1st.
I'd suggest to split the PR like this:
- A PR which adds the
tests/components/stookwijzer/test_init.py+ fixtures used by it - A PR which bumps the library and makes minimal changes needed to support it
- A PR which adds the data coordinator
- A PR which adds the binary sensor
This one could be very usefull in this season! would like to use it since it is much more fine grained than stookalert. Hope you guys are able to get it released.
What's left here to get this merged? I'd really use this now with the winter coming up.
@joostlek Honestly, that review is kinda waste of time. You've seen a few pushes from me + it is draft for a reason. Most of the code will look different.
I think this is rather complete now. Now I've mostly refactored it, I do agree with the other comments that this needs to be split out into multiple PRs.
So, what we first can do here, is make a PR that bump the dependency in the existing integrations and does the bare minimum to get there.
That would mean a PR with a Dependency bump, config entry migration + issue, config flow adjustments, config flow test adjustments, and probably the init tests (which test the migration of the entry + issue raised in case of failure) + diagnostics adjustments.
As follow-up PRs could be:
- Add coordinator
- Add entity description support (+ entity entry migration) for the existing sensor + sensor tests
- Add new sensors
- Add strict typing
- Add quality scale files
With this PR in place, that shouldn't be too much of a task to split out.
After that, I think there are quite a few improvements that can be made upstream in the client library as well. Mostly in terms of error handling. Right now, it just silences all the errors and returns None for those cases. While easy to use, it does cause problem as we won't be able to react on it. For example, how would we know the difference between an connection error or data just not being available? Each of these cases is handled differently in Home Assistant, but we don't have that information.
../Frenck
Entity descriptions support: https://github.com/home-assistant/core/pull/131585
Extend tests for Stookwijzer init: https://github.com/home-assistant/core/pull/131589
Enable strict typing for Stookwijzer: https://github.com/home-assistant/core/pull/131590
Add data description for Stookwijzer config flow: https://github.com/home-assistant/core/pull/131591
And the last one:
Record current IQS state for Stookwijzer: https://github.com/home-assistant/core/pull/131592
Closing this PR, as everything in here is done in the splitter-out PRs listed above.
../Frenck