Add Stookwijzer

[fwestenberg]

Breaking change

Proposed change

New integration: Stookwijzer.nu.

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)
• 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 #
• This PR is related to issue:
• Link to documentation pull request: Add Stookwijzer home-assistant.io#25384

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• 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.

[frenck]

Removed request for review, as these comments hasn't been resolved:

• Add Stookwijzer #84435 (comment)
• Add Stookwijzer #84435 (comment)

[fwestenberg]

@frenck I believe this PR is ready now!

[fwestenberg]

@frenck last item on the list. Without unique_id, the integration seems not to work. Is this really a problem, having lat/lon as unique_id? Docs suggest this also.

[frenck]

Without unique_id, the integration seems not to work. Is this really a problem, having lat/lon as unique_id?

That unique ID I'm suggesting to remove, it the unique ID for the config entry. It isn't directly used or needed and complete option. It should not influence the integration in any way (it is only used to prevent duplicates, for which lat/long is not really a fit).

This specific identifier is generally used for e.g., a device serial number, account ID, mac addresses, those kinds of things.

I'm happy to make/test and push the change in a working fashion, if you are ok with that?

../Frenck

[fwestenberg]

Without unique_id, the integration seems not to work. Is this really a problem, having lat/lon as unique_id?

That unique ID I'm suggesting to remove, it the unique ID for the config entry. It isn't directly used or needed and complete option. It should not influence the integration in any way (it is only used to prevent duplicates, for which lat/long is not really a fit).

This specific identifier is generally used for e.g., a device serial number, account ID, mac addresses, those kinds of things.

I'm happy to make/test and push the change in a working fashion, if you are ok with that?

../Frenck

Yes, this would really help!

[frenck]

I'll dive right into it 👍

[frenck]

@fwestenberg I've pushed a change containing:

• Removal of unique ID from config flow
• Cleanup of tests because of the above
• Adjusts the unique ID used for the sensor entity
• Changed the title of the resulting config entry to just "Stookwijzer". This way, the frontend will merge the title & config entry title. A user can always rename it if they like. Besides, the previously used coordinates are too big to display anyways.
• Added description in the config flow user step (so the user knows what to do with the map)
• Fixed translations of data in the user step
• Cleaned up translation strings
• Small formatting changes

Let me know what you think.

../Frenck

[fwestenberg]

Hi Frenck, your changes look to me! Thanks for your effort!

[frenck]

Added one more thing that I noticed missing in the final tests. Since it is using the enum device class, it means it also should pass the possible options the state can be in.

This allows, e.g., the automation and scripts editors to provide the end-user with a list of states to choose from:

If you are ok with all these changes (including this last one), please let me know. I'll finalize the adding of this integration at that point.

../Frenck

[fwestenberg]

Really nice you've added this as well. I'm satisfied!

[frenck]

Alright, thanks @fwestenberg 👍

../Frenck