Skip to content

Add dlna_dmr component#5474

Merged
MartinHjelmare merged 9 commits intohome-assistant:nextfrom
StevenLooman:next
Aug 5, 2018
Merged

Add dlna_dmr component#5474
MartinHjelmare merged 9 commits intohome-assistant:nextfrom
StevenLooman:next

Conversation

@StevenLooman
Copy link
Copy Markdown
Contributor

@StevenLooman StevenLooman commented Jun 1, 2018

Description:

Pull request in home-assistant (if applicable): home-assistant/core#14749

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the standards.

@frenck frenck added the new-integration This PR adds documentation for a new Home Assistant integration label Jun 25, 2018
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Looks good overall @StevenLooman!
Nevertheless, left you 2 comments, could you take a look? 👍


Please note that some devices, such as Samsung TVs, are rather picky about the source used to play from. The TTS service might not work in combination with these devices. If the play_media service does not work, please try playing from a DLNA/DMS (such as [MiniDLNA](https://sourceforge.net/projects/minidlna/)).

Configuration variables:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use the configuration tags, for more information please see:
https://home-assistant.io/developers/documentation/create_page/#configuration

media_player:
- platform: dlna_dmr
url: http://192.168.0.10:9197/dmr
name: "TV living room"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We try to keep all configuration samples minimal. Thus, no optional requirement in the default sample. This helps a user to get started quickly by copy-&-paste the sample without worrying about optional parameters which they most likely not need. If required, insert a full configuration sample later that covers special setups or alike.

@StevenLooman
Copy link
Copy Markdown
Contributor Author

Thank you for the review, will fix it soon!

@frenck frenck added in-progress This PR/Issue is currently being worked on next This PR goes into the next branch has-parent This PR has a parent PR in another repo labels Jun 28, 2018
@frenck frenck added ready-for-review This PR needs to be reviewed and removed in-progress This PR/Issue is currently being worked on labels Jun 30, 2018
@ghost ghost assigned frenck Jul 8, 2018
frenck
frenck previously approved these changes Jul 8, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Jul 8, 2018

Thanks for the updates @StevenLooman!
✅ Approved. Can be merged as soon as the parent PR gets merged.

@frenck frenck added awaits-parent Awaits the merge of an parent PR and removed ready-for-review This PR needs to be reviewed labels Jul 8, 2018
@frenck frenck removed their assignment Jul 13, 2018
logo: dlna.png
ha_category: Media Player
featured: false
ha_release: 0.74
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

0.76


Please note that some devices, such as Samsung TVs, are rather picky about the source used to play from. The TTS service might not work in combination with these devices. If the play_media service does not work, please try playing from a DLNA/DMS (such as [MiniDLNA](https://sourceforge.net/projects/minidlna/)).

## {% linkable_title Configuration %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Move this above the paragraph where we talk about the example config.

url: http://192.168.0.10:9197/dmr
```

## {% linkable_title Configuration %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant that this header should be moved above the example paragraph.

@MartinHjelmare MartinHjelmare merged commit 6c4cbb7 into home-assistant:next Aug 5, 2018
@ghost ghost removed the awaits-parent Awaits the merge of an parent PR label Aug 5, 2018
@StevenLooman
Copy link
Copy Markdown
Contributor Author

Great, thanks! Please be sure to tag me in any future issues when any problems turn up.

dbrowndan pushed a commit to dbrowndan/home-assistant.io that referenced this pull request Aug 8, 2018
* Add dlna_dmr component

* Remove no longer supported features

* Add warning about picky devices

* Add UDN to configuration; requested changes from review

* ✏️ Tweaks

* ⬆️ ha_release to 0.74

* Update media_player.dlna_dmr.markdown

* Changes after review by @MartinHjelmare

* Update media_player.dlna_dmr.markdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-parent This PR has a parent PR in another repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants