Skip to content

Add available and state valid property to PulseAudio#33393

Closed
breiti wants to merge 1 commit intohome-assistant:devfrom
breiti:pulse-na
Closed

Add available and state valid property to PulseAudio#33393
breiti wants to merge 1 commit intohome-assistant:devfrom
breiti:pulse-na

Conversation

@breiti
Copy link
Copy Markdown
Contributor

@breiti breiti commented Mar 29, 2020

Proposed change

Until the first response from the PulseAudio server is received, all switches are reported "off", which is not necessarily the correct representation. This change marks the switch as "not available" until the first valid list of loaded modules

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

@springstan springstan changed the title Mark PulseAudio switch not available Add available property to PulseAudio Mar 30, 2020
@springstan springstan changed the title Add available property to PulseAudio Add available and state valid property to PulseAudio Mar 30, 2020
Comment thread homeassistant/components/pulseaudio_loopback/switch.py Outdated
Comment thread homeassistant/components/pulseaudio_loopback/switch.py Outdated
Comment thread homeassistant/components/pulseaudio_loopback/switch.py Outdated
Comment thread homeassistant/components/pulseaudio_loopback/switch.py Outdated
Comment thread homeassistant/components/pulseaudio_loopback/switch.py Outdated
@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Apr 3, 2020

You need to use a library to speak with PulseAudio, use: https://github.com/mk-fg/python-pulse-control based on: https://developers.home-assistant.io/docs/creating_component_code_review#4-communication-with-devicesservices

This library is supported by Home Assistant because we use also PulseAudio in the background.

@breiti
Copy link
Copy Markdown
Contributor Author

breiti commented Apr 3, 2020

While I understand the need for an external library, please note that this PR does not add any new library code but only makes minor changes to already existing code in Home Assistant. Migrating to a library is definitely useful, but this should be a different PR.

Until the first response from the PulseAudio server is received, all
switches are reported "off", which is not necessarily the correct
representation.

This change marks the switch as "not available" until the first valid
list of loaded modules is received and solves home-assistant#32016.
@breiti
Copy link
Copy Markdown
Contributor Author

breiti commented Apr 12, 2020

@pvizeli: As you suggested, I started to work on replacing the internal library with the pulsectl library. Everything works fine, but when I try to use this on my production system (Home Assistant on Ubuntu/Docker) it seems like the required libpulse.so.0 file is not installed in this Docker container?

@breiti
Copy link
Copy Markdown
Contributor Author

breiti commented Apr 13, 2020

After installing libpulse inside the container (apk add libpulse), it seems like this library does not support remote connections. The same code is working fine inside the Debian based dev container...

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare 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 to keep track of PR status.

As stated above, please extract the device specific interface code and data parsing to a standalone library published on PyPI.

https://developers.home-assistant.io/docs/creating_component_code_review#4-communication-with-devicesservices

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Apr 29, 2020

Is added to docker now

@breiti
Copy link
Copy Markdown
Contributor Author

breiti commented Apr 30, 2020

Extracting library in #34965, making this PR obsolete.

@breiti breiti closed this Apr 30, 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.

pulseaudio: Multiple loopbacks for same switch

5 participants