Skip to content

Fix Synology NAS discovered multiple times#35094

Merged
balloob merged 2 commits intohome-assistant:devfrom
Quentame:synology_dsm/fix-discovery
May 3, 2020
Merged

Fix Synology NAS discovered multiple times#35094
balloob merged 2 commits intohome-assistant:devfrom
Quentame:synology_dsm/fix-discovery

Conversation

@Quentame
Copy link
Copy Markdown
Member

@Quentame Quentame commented May 2, 2020

Proposed change

Adding all possible NAS MAC address to the config entry to abort the SSDP step if the serial (which is a MAC) is in configured MACs.

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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

Comment thread homeassistant/components/synology_dsm/__init__.py Outdated
@Quentame
Copy link
Copy Markdown
Member Author

Quentame commented May 2, 2020

I could not test it on my network @bdraco

May you ? (with 2 eth connected to your NAS)

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 2, 2020

Added for debugging

_mac_already_configured
....
        _LOGGER.debug("Checking mac: %s, existing macs: %s", mac, existing_macs)

The mac that comes from discovery has no -s

2020-05-02 23:32:35 DEBUG (MainThread) [custom_components.synology_dsm.config_flow] Checking mac: 001132BE0C99, existing macs: ['00-11-32-BE-0C-99', '00-11-32-BE-0C-9A']

Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes so I see the ping its ready again

@Quentame Quentame force-pushed the synology_dsm/fix-discovery branch from 9ba51fa to 25132e2 Compare May 3, 2020 03:21
@Quentame Quentame requested a review from bdraco May 3, 2020 03:23
@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 3, 2020

Restarted. The discovery dupe is gone.

Deleted and restarting again to make sure discovery still works

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 3, 2020

Confirmed discovery still works.

One more restart and then I'm update production

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 3, 2020

All good, not rediscovered when setup from discovery and restarted

Production update went well.

Now doing a configure as localhost and restart to make sure it doesn't appear in discovery

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 3, 2020

Confirmed not rediscovered when setup as 127.0.0.1 and discovery comes from the network ip

Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job on this!

@balloob balloob merged commit 661656e into home-assistant:dev May 3, 2020
@Quentame Quentame deleted the synology_dsm/fix-discovery branch May 3, 2020 09:00
@balloob balloob mentioned this pull request May 5, 2020
@lock lock Bot locked and limited conversation to collaborators May 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Synology DSM discovery: KeyError: 'host' Synology DSM discovery: flow unique_id issue? Synology DSM discovery has found 2 separate copies of my NAS

4 participants