core icon indicating copy to clipboard operation
core copied to clipboard

Note SoyeaLink support in Huawei LTE comments

Open scop opened this issue 3 years ago • 7 comments

Proposed change

The Huawei LTE integration works out of the box with at least some SoyeaLink devices, note that in comments.

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)
  • [x] 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:

Checklist

  • [ ] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [x] There is no commented out code in this PR.
  • [ ] I have followed the development checklist
  • [x] 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:

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.
  • [ ] 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.

The integration reached or maintains the following Integration Quality Scale:

  • [ ] No score or internal
  • [ ] 🥈 Silver
  • [ ] 🥇 Gold
  • [ ] 🏆 Platinum

To help with the load of incoming pull requests:

scop avatar Oct 08 '22 08:10 scop

Hey there @fphammerle, mind taking a look at this pull request as it has been labeled with an integration (huawei_lte) you are listed as a code owner for? Thanks!

homeassistant avatar Oct 08 '22 08:10 homeassistant

I'm not sure this PR is useful (especially re: your comment about "at least some SoyeaLink devices"). What about using supported brands in the manifest?

bachya avatar Oct 08 '22 17:10 bachya

"At least some" can be said about Huawei devices as well, I wouldn't read too much into it.

Supported brands in the manifest I can see being useful in addition to this PR. The related instructions hint that addition of a logo in the brands repo could be a requirement for its use. If that's so, I wouldn't hold this PR back for it as I wasn't able to locate SoyeaLink logo usage guidelines and thus getting it in the brands repo might take some time.

scop avatar Oct 09 '22 07:10 scop

If that's so, I wouldn't hold this PR back

Well, we should hold it back, as stuffing multiple brands in the integration name isn't really nice either.

frenck avatar Oct 10 '22 15:10 frenck

Well, we should hold it back, as stuffing multiple brands in the integration name isn't really nice either.

This comment seems to confirm that the addition of a logo is actually a requirement for using supported_brands? But I'd hope we can be crystal clear about it, so: If it is a requirement, I think we should clarify that in https://developers.home-assistant.io/docs/creating_integration_manifest/#supported-brands -- it currently says "should" (not "must" or the like). If it's not a requirement, could you confirm that and I'll revise this to use supported_brands instead, without the logo for the time being?

scop avatar Oct 11 '22 18:10 scop

Removed changes to user visible strings. Comments are not affected by the other mechanisms, so I thought we might as well apply changes to them as they're ready here.

scop avatar Nov 06 '22 09:11 scop

Virtual integration does not provide any of the info that is added here. But it's quite clear that spending more time on this isn't going to be worth it, closing.

scop avatar Nov 20 '22 08:11 scop