Skip to content
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.

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

Add support for LG soundbars #17570

Merged
merged 3 commits into from Oct 18, 2018

Conversation

@mjg59
Copy link
Contributor

commented Oct 18, 2018

Description:

Add support for the current generation of LG soundbars as media player devices.

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

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

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.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
self._bass = 0
self._treble = 0

self._device = temescal.temescal(host, port=port, callback=self.handle_event)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Oct 18, 2018

line too long (85 > 79 characters)

Show resolved Hide resolved homeassistant/components/media_player/lg_soundbar.py
Show resolved Hide resolved homeassistant/components/media_player/lg_soundbar.py

@mjg59 mjg59 referenced this pull request Oct 18, 2018

Merged

Add documentation for the LG soundbar component #6919

2 of 2 tasks complete

@mjg59 mjg59 force-pushed the mjg59:lg_soundbar branch from ebfcdb3 to 30b2fa4 Oct 18, 2018

mjg59 added some commits Oct 15, 2018

Add LG soundbar support
We can autodiscover these, so for now let's skip any local configuration.
Currently we expose volume, source and equaliser preset - we can expose the
other volume controls and options as well if necessary, but I don't know
whether it's worth it.
Add discovery of LG devices
This is a generic discovery type that doesn't obviously contain enough
information to identify whether we're talking to a speaker system or any
other kind of device - on the other hand I haven't been able to find any
other LG devices that respond like this, so we can cross that bridge when
we get to it.

@mjg59 mjg59 force-pushed the mjg59:lg_soundbar branch from 30b2fa4 to c553808 Oct 18, 2018

@balloob balloob merged commit e2a1e21 into home-assistant:dev Oct 18, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.591%
Details

@wafflebot wafflebot bot removed the in progress label Oct 18, 2018

self._treble = 0

self._device = temescal.temescal(host, port=port,
callback=self.handle_event)

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

Callback registration should preferably be done in async_added_to_hass. We risk an error otherwise if the callback is called before the entity has been added to home assistant.


self._device = temescal.temescal(host, port=port,
callback=self.handle_event)
self.update()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

Remove this and instead call add_entities with second argument True. That will make sure that update is called during entity addition.

self._equaliser = -1
self._equalisers = []
self._mute = 0
self._rear_volume = 0

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

This isn't used.

self._equalisers = []
self._mute = 0
self._rear_volume = 0
self._rear_volume_min = 0

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

This isn't used.

self._mute = 0
self._rear_volume = 0
self._rear_volume_min = 0
self._rear_volume_max = 0

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

This isn't used.

self._rear_volume_max = 0
self._woofer_volume = 0
self._woofer_volume_min = 0
self._woofer_volume_max = 0

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

None of the woofer volumes are used.

self._woofer_volume_min = 0
self._woofer_volume_max = 0
self._bass = 0
self._treble = 0

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Oct 18, 2018

Member

Bass and treble aren't used either.

@balloob balloob referenced this pull request Oct 26, 2018

Merged

0.81 #17809

@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.