core
core copied to clipboard
Huawei LTE SSDP improvements
Proposed change
Various fixes and improvements to Huawei LTE SSDP discovery, see individual commits for details.
Type of change
- [ ] Dependency upgrade
- [x] 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:
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] 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:
- [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 runningpython3 -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.
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!
Code owner commands
Code owners of huawei_lte can trigger bot actions by commenting:
@home-assistant closeCloses the issue.@home-assistant rename Awesome new titleChange the title of the issue.@home-assistant reopenReopen the issue.@home-assistant unassign huawei_lteRemoves the current integration label and assignees on the issue, add the integration domain after the command.
Good questions indeed.
Re the wantedness/allowlist: To be honest, I have no clue, and I have no access to devices we would not want. There used to be the huawei_router integration that dealt with some other than mobile/LTE devices, so I suppose we might encounter some. Anyway, I would be fine with just starting from a clean slate and dropping the current list, and seeing if we get any reports. (Aside, it's not a new whitelist -- one existed already, but was simpler.) I'm kind of indifferent here, if any of you is leaning towards keeping/removing, let's do that, just let me know.
Re the services: the semi-recent device I can access at the moment exposes LANDevice:1, WANPPPConnection:1, WANConnectionDevice:1, WANDevice:1, and InternetGatewayDevice:1. None of those sounds any better or specific than the IGD we currently use, but if anyone knows better or has different opinions, do speak up.
Another older dongle has services WANIPConnection:1, WANConnectionDevice:1, WANCommonInterfaceConfig:1, WANDevice:1, Layer3Forwarding:1, InternetGatewayDevice:1. Its friendly name is not something that we would have found as "ours" before or after the changes in 4bd2e288c8c667e776e541b6bf6398454dcdd19a (it's just "Huawei Internet Gateway Device"). Dropping the additional match would make that one discoverable via SSDP.
Guess we could replace the friendlyname match with a probe at an endpoint that according to our best guess is known to be available without authentication in ~all devices if we're worried about false positives. If you have a better feeling about this approach than just removing the match, let me know and I'll have a look at that.
maybe "Huawei" should be added to the regexp to let that another dongle be discovered, too?
To me the regexp exists to filter out other Huawei(/SoyeaLink) devices besides the LTE'y ones, and adding "Huawei" there would more or less defeat the purpose. In that sense I would not add it, but then again, as said, I would be open to removing the regexp match altogether and see where that takes us. As you noted, the worst case scenario wouldn't be that much of a biggie, the user has the option to ignore false positives.
Found a thinko with regards to unique id handling, addressed in c431a1f6de75e4b9fe876ff8766665daddff3132.
Ahh, gotcha. Your call, both options are fine in my opinion.
I'm leaning towards removing it, done so in f12e37828ac387560b8ab7bba102001896828672. Going to merge this ~tomorrow unless there are objections.