Disable creating port mappings from UI, add discovery from component by StevenLooman · Pull Request #18565 · home-assistant/core

StevenLooman · 2018-11-18T21:46:23Z

Description:

Disable creating port mappings from UI (see #17937)
Add discovery from component (see #17400)
Upgrade to async_upnp_client==0.13.3 to support async discovery
Possible fix for #18066: SSL being required when talking to non-SSL-enabled services

Related issue (if applicable):
fixes #17937, fixes #17400

Pull request in home-assistant.io with documentation (if applicable): -

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.

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

Documentation added/updated in home-assistant.io

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

New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

StevenLooman · 2018-11-18T21:48:16Z

@dgomes

StevenLooman · 2018-11-19T05:47:26Z

Please do not merge. I'm not happy with the state of the flow.

frenck · 2018-11-19T20:53:20Z

Please do not merge. I'm not happy with the state of the flow.

Please add WIP: to the title in that case...

homeassistant/components/upnp/__init__.py

homeassistant/components/upnp/config_flow.py

StevenLooman · 2018-11-24T16:24:09Z

@dgomes Please review. Any comment is welcome.

The component has been rewritten to use the DiscoveryFlowHandler, simplifying things for the component itself. Also, now the component creates a device in the device registry, which the sensors are using/reporting.

dgomes

LGTM!

homeassistant/components/upnp/__init__.py

homeassistant/components/media_player/dlna_dmr.py

homeassistant/components/upnp/device.py

balloob · 2018-11-27T20:06:45Z

I think that we need to login to Lokalise to remove the no longer needed translation keys. Just removing the files won't help as they are not the source of truth.

StevenLooman · 2018-11-28T20:20:55Z

I think that we need to login to Lokalise to remove the no longer needed translation keys. Just removing the files won't help as they are not the source of truth.

I don't get what you're trying to say.

I removed the files as these were:

incomplete for most languages, and
no longer up to date, and
Not added by me. I.e., new (and better, given these were obsolete/not up to date) translations are added by others.

balloob · 2018-11-28T20:35:35Z

Changing the translation files has no effect at all. We just run script/translations_download and they will be back, because Lokalise is the source of truth.

balloob · 2018-11-28T20:36:14Z

And because the are no longer up to date because you changed the keys, you have to log in to Lokalise and update them there

StevenLooman · 2018-11-28T20:58:47Z

Ok, thank you for explaining. I've joined the backend project. It seems that I cannot change any keys in Localise.
I will add (new) translations for the component when possible.

StevenLooman · 2018-11-28T20:58:56Z

Wrong button...

dgomes · 2018-12-05T21:25:31Z

@StevenLooman you need to remove the translations from the PR in order for this PR to be merged

StevenLooman · 2018-12-06T05:43:31Z

Done!

StevenLooman · 2018-12-07T20:06:52Z

Minor fix to async_upnp_client.

dgomes · 2018-12-07T22:23:44Z

@StevenLooman make sure you remove the commits that remove the translation files

StevenLooman · 2018-12-09T13:08:31Z

Can we restart travis? @dgomes
It seems something unrelated has failed.

MartinHjelmare · 2018-12-09T13:27:31Z

Restarted the job.

balloob · 2018-12-10T14:56:07Z

This PR now removes translations instead of not changing them in this PR.

StevenLooman · 2018-12-10T19:13:36Z

I must have mis-read @dgomes' comment. I can only remove the commit which deleted the translations, not which altered them as it contains more changes.

Is this mergable now?

StevenLooman · 2018-12-10T19:42:17Z

I've re-applied all the changes. No translations are touched.

StevenLooman · 2018-12-12T21:48:16Z

Anything I can do?

balloob · 2018-12-14T11:59:23Z

homeassistant/components/sensor/upnp.py

+                (DOMAIN_UPNP, self.unique_id)
+            },
+            'name': self.name,
+            'via_hub': (DOMAIN_UPNP, self._device.udn),


I see that you register a via_hub here but that device is not created anywhere ?

It is created in the upnp component itself

StevenLooman · 2018-12-21T17:36:18Z

Thanks!

…ome-assistant#18565) * Disable creating port mappings from UI, add discovery from component * Remove unused constant * Upgrade to async_upnp_client==0.13.6 and use manufacturer from device * Upgrade to async_upnp_client==0.13.7

homeassistant added cla-signed integration: discovery integration: upnp platform: media_player.dlna_dmr labels Nov 18, 2018

ghost added the in progress label Nov 18, 2018

MartinHjelmare changed the title ~~Disable creating port mappings from UI, add discovery from component~~ WIP: Disable creating port mappings from UI, add discovery from component Nov 20, 2018

MartinHjelmare reviewed Nov 20, 2018

View reviewed changes

homeassistant/components/upnp/__init__.py Outdated Show resolved Hide resolved

homeassistant/components/upnp/config_flow.py Outdated Show resolved Hide resolved

StevenLooman requested a review from a team as a code owner November 24, 2018 16:22

StevenLooman changed the title ~~WIP: Disable creating port mappings from UI, add discovery from component~~ Disable creating port mappings from UI, add discovery from component Nov 24, 2018

dgomes approved these changes Nov 24, 2018

View reviewed changes

homeassistant/components/upnp/__init__.py Outdated Show resolved Hide resolved

balloob reviewed Nov 27, 2018

View reviewed changes

homeassistant/components/media_player/dlna_dmr.py Outdated Show resolved Hide resolved

balloob reviewed Nov 27, 2018

View reviewed changes

homeassistant/components/upnp/device.py Outdated Show resolved Hide resolved

StevenLooman closed this Nov 28, 2018

ghost removed the in progress label Nov 28, 2018

StevenLooman reopened this Nov 28, 2018

ghost added the in progress label Nov 28, 2018

ammarfaizi2 approved these changes Dec 1, 2018

View reviewed changes

StevenLooman force-pushed the igd_discovery branch from 460ed89 to ca9b1e6 Compare December 10, 2018 19:12

Disable creating port mappings from UI, add discovery from component

4a06064

StevenLooman force-pushed the igd_discovery branch from 4fbe059 to 4a06064 Compare December 10, 2018 19:41

Remove unused constant

b992771

balloob reviewed Dec 14, 2018

View reviewed changes

Upgrade to async_upnp_client==0.13.6 and use manufacturer from device

65d9709

StevenLooman force-pushed the igd_discovery branch from 8426a51 to 65d9709 Compare December 14, 2018 13:37

Upgrade to async_upnp_client==0.13.7

22f80ef

dgomes merged commit 501b3f9 into home-assistant:dev Dec 21, 2018

ghost removed the in progress label Dec 21, 2018

StevenLooman deleted the igd_discovery branch January 8, 2019 19:20

balloob mentioned this pull request Jan 10, 2019

0.85.0 #19897

Merged

ghost added integration: dlna_dmr and removed platform: media_player.dlna_dmr labels Mar 21, 2019

Uh oh!

Conversation

StevenLooman commented Nov 18, 2018 • edited by MartinHjelmare Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Checklist:

Uh oh!

StevenLooman commented Nov 18, 2018

Uh oh!

StevenLooman commented Nov 19, 2018

Uh oh!

frenck commented Nov 19, 2018

Uh oh!

Uh oh!

Uh oh!

StevenLooman commented Nov 24, 2018

Uh oh!

dgomes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

balloob commented Nov 27, 2018

Uh oh!

StevenLooman commented Nov 28, 2018

Uh oh!

balloob commented Nov 28, 2018

Uh oh!

balloob commented Nov 28, 2018

Uh oh!

StevenLooman commented Nov 28, 2018

Uh oh!

StevenLooman commented Nov 28, 2018

Uh oh!

dgomes commented Dec 5, 2018

Uh oh!

StevenLooman commented Dec 6, 2018

Uh oh!

StevenLooman commented Dec 7, 2018

Uh oh!

dgomes commented Dec 7, 2018

Uh oh!

StevenLooman commented Dec 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinHjelmare commented Dec 9, 2018

Uh oh!

balloob commented Dec 10, 2018

Uh oh!

StevenLooman commented Dec 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StevenLooman commented Dec 10, 2018

Uh oh!

StevenLooman commented Dec 12, 2018

Uh oh!

balloob Dec 14, 2018

Choose a reason for hiding this comment

Uh oh!

StevenLooman Dec 14, 2018

Choose a reason for hiding this comment

Uh oh!

StevenLooman commented Dec 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

StevenLooman commented Nov 18, 2018 •

edited by MartinHjelmare

Loading

StevenLooman commented Dec 9, 2018 •

edited

Loading

StevenLooman commented Dec 10, 2018 •

edited

Loading