8000 Netgear fix port and device model beeing overwritten by starkillerOG · Pull Request #57277 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Netgear fix port and device model beeing 8000 overwritten #57277

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 8, 2021
Merged

Netgear fix port and device model beeing overwritten #57277

merged 3 commits into from
Oct 8, 2021

Conversation

starkillerOG
Copy link
Contributor
@starkillerOG starkillerOG commented Oct 7, 2021 •
edited
Loading

Breaking change

Proposed change

  • Do not overwrite the port through SSDP discovery once the integration is setup to allow for manual configuration if SSDP finds the wrong port (on Orbi models)
  • Add debug log for the SSDP discovery to debug the port on Orbi models
  • Do not overwrite the device model if it is none, if another integration already supplied the model info with the same mac identification this Netgear integration would overwirte it and make is None again. This fixes this. (the model info is not always provided by the router).

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

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:

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.

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:

@starkillerOG starkillerOG requested a review from Quentame as a code owner October 7, 2021 20:25
@probot-home-assistant
Copy link

Hey there @hacf-fr, @Quentame, mind taking a look at this pull request as it has been labeled with an integration (netgear) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@starkillerOG starkillerOG mentioned this pull request Oct 7, 2021
Merged
22 tasks
@starkillerOG
Copy link
Contributor Author
starkillerOG commented Oct 7, 2021 •
edited
Loading

@MartinHjelmare I tested this and it does not break anything.
Not sure if it will fix the orbi models since I don't have one and can't test, but at least it is a step in the right direction and I think it will fix it.
Can you merge this?

@MartinHjelmare
Copy link
Member

Please rebase and only add the fix commits to a clean branch. Thanks!

@starkillerOG @MartinHjelmare
netgear fixes
b4f88e8 
Netgear add ssid, conn_ap_mac and allow_or_block sensors

Update homeassistant/components/netgear/sensor.py

Update homeassistant/components/netgear/sensor.py

Update homeassistant/components/netgear/sensor.py

remove defaults

remove allow/block

fix black

add debug to ssdp discovery

do not overwrite model if it is None

The model of the device would be overwritten to None if it was already known by another integration with the same mac connection.
The router does not always provide model information

fix isort

Do not update port of already setup router

fix if attribute is not available

Co-Authored-By: Martin Hjelmare <marhje52@gmail.com>
@starkillerOG
Copy link
Contributor Author
starkillerOG commented Oct 7, 2021 •
edited
Loading

@MartinHjelmare I rebased and squased the commits, hope this is alright.

MartinHjelmare
MartinHjelmare reviewed Oct 7, 2021
View reviewed changes
MartinHjelmare
MartinHjelmare requested changes Oct 8, 2021
View reviewed changes
MartinHjelmare
MartinHjelmare reviewed Oct 8, 2021
View reviewed changes
@starkillerOG @MartinHjelmare
Update homeassistant/components/netgear/router.py
5483a24 
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@MartinHjelmare MartinHjelmare added this to the 2021.10.2 milestone Oct 8, 2021
@MartinHjelmare
Copy link
Member

Do we need to wait for user testing?

@balloob
Copy link
Member
balloob commented Oct 8, 2021 •
edited
Loading

This change looks good. Specially default_name is a good one 😬

I don't think we need to wait for user testing, as it looks like it should work.

@balloob balloob merged commit 830e2bc into home-assistant:dev Oct 8, 2021
balloob pushed a commit that referenced this pull request Oct 8, 2021
@starkillerOG @MartinHjelmare @balloob
Netgear fix port and device model beeing overwritten (#57277)
c6d506c 
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@balloob balloob mentioned this pull request Oct 8, 2021
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@balloob balloob balloob approved these changes

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@Quentame Quentame Awaiting requested review from Quentame Quentame is a code owner

Assignees
No one assigned
Labels
bugfix by-code-owner cherry-picked cla-signed Hacktoberfest integration: netgear small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
2021.10.2
Development

Successfully merging this pull request may close these issues.
4 participants
@starkillerOG @MartinHjelmare @balloob @homeassistant
317B
0