Show headset icons for devices identifying as "Headset" in Description
ClosedPublic

Authored by antlarr on Sep 29 2017, 9:16 AM.

Details

Summary

A Logitech H600 / H800 headset was showing an "unknown" icon for the player device.

These devices use the following identifiers:

The sink/player device:
currentPort.name: analog-output
currentPort.description: Analog Output H600
Description: [Wireless Headset] Analog Stereo

The source/capture device:
currentPort.name: analog-input-mic
currentPort.description: Microphone H600
Description: [Wireless Headset] Analog Mono

Test Plan

Using such a device and this patch now shows a headset icon
next to the player/capture devices

Diff Detail

Repository
R115 Plasma Audio Volume Applet
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antlarr created this revision.Sep 29 2017, 9:16 AM
Restricted Application added a project: Plasma. · View Herald TranscriptSep 29 2017, 9:16 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
This comment was removed by antlarr.
antlarr updated this revision to Diff 20082.Sep 29 2017, 9:39 AM
  • We have a headset icon, so use it for headsets
antlarr retitled this revision from Show headphone icons for devices identifying as "Headset" in Description to Show headset icons for devices identifying as "Headset" in Description.Sep 29 2017, 9:40 AM
antlarr edited the summary of this revision. (Show Details)
antlarr edited the test plan for this revision. (Show Details)
davidedmundson accepted this revision.Sep 29 2017, 9:43 AM
This revision is now accepted and ready to land.Sep 29 2017, 9:43 AM
sebas added a subscriber: sebas.Sep 29 2017, 9:43 AM

Where is the headset icon? I can't see it in breeze...

Hmm, you're right, there's no audio-headset icon in breeze, it's inherited from oxygen. Should we use it while we request an audio-headset icon for breeze? or should I revert that change and use headphones in the meantime?

sebas added a comment.Sep 29 2017, 9:47 AM

Perhaps move the conditional to the end, so it is taken if there's nothing else, that's making "surer" that nothing that currently works is changed, and at the same time, request a headset icon for breeze?

If I move it to the end, then the player device will show a headset correctly anyway since no other condition matches but for the capture device, a microphone icon will be shown instead of the headset.

sebas added a comment.Sep 29 2017, 9:51 AM

Ah okay, I guess then keep this patch in, and ask for the breeze icon. :)

Perfect, thanks, I'll do that

drosca requested changes to this revision.Sep 29 2017, 9:57 AM
drosca added a subscriber: drosca.

Isn't there a better way to test it without using Description (human readable string - usually device model name)?

This revision now requires changes to proceed.Sep 29 2017, 9:57 AM
antlarr added a comment.EditedSep 29 2017, 10:01 AM

I wrote in the summary above the contents of the string variables we could work with so that it's clear I couldn't find any other way to extract that information. If you have any other idea (any other variable to use for the match?) I'm happy to test it.

pactl list sinks
pactl list sources

If pulseaudio even differentiates between headset and headphones.

antlarr updated this revision to Diff 20090.Sep 29 2017, 12:44 PM
  • Use form_factor from device to select the icon for a profile
drosca requested changes to this revision.Sep 29 2017, 12:52 PM

Perfect solution!

But why is it in Profile when it is device property? Just remove it from Profile and leave it only in Device.

applet/contents/ui/DeviceListItem.qml
41–42

Form factor will be the same for all ports, so just use switch (FormFactor)

This revision now requires changes to proceed.Sep 29 2017, 12:52 PM
antlarr updated this revision to Diff 20094.Sep 29 2017, 2:30 PM
  • Use the formFactor attribute of the Device directly

Thanks for the hint! I didn't know I could use FormFactor directly to reference the Device's formFactor property. Indeed now it looks much better.

antlarr marked an inline comment as done.Sep 29 2017, 2:32 PM
drosca accepted this revision.Oct 2 2017, 11:52 AM
drosca added inline comments.
applet/contents/ui/DeviceListItem.qml
28–42

This check is no longer needed.

This revision is now accepted and ready to land.Oct 2 2017, 11:52 AM
antlarr updated this revision to Diff 20239.Oct 2 2017, 11:54 AM
  • Remove check for currentPort which is no longer needed
This revision was automatically updated to reflect the committed changes.

icons are added to breeze-icons

guilhermesilva added a subscriber: guilhermesilva.EditedApr 24 2018, 8:49 AM

Hi guys! Sorry to "necro-bump" this issue, but I wanted to share my opinion on this new approach of setting the device icon. From what I've been seeing in random screenshots of the plasma-pa applet, most of the times they all have the same icon: audio-card.svg. That's because, apparently, for a lot of people (myself included), the device.form_factor property is set to internal in all sinks:

$ pacmd list sinks | grep device.form_factor
device.form_factor = "internal"
device.form_factor = "internal"
device.form_factor = "internal"
device.form_factor = "internal"

So, the the device list in plasma-pa end up looking like this:

While that's not really a big deal for most people, I really think the previous "hacky" approach delivered better-looking results:

I don't know if that's an issue with PulseAudio, but for now I decided to downgrade the plasma-pa package back to version 5.11.5. I've also made an adaptation in contents/ui/DeviceListItem.qml, because my speakers port is named analog-output-lineout, so I wasn't getting the proper icon as well. Ah, I've also changed the audio-speakers-symbolic icon to just audio-speakers because I think it looks better:

diff --git a/applet/contents/ui/DeviceListItem.qml b/applet/contents/ui/DeviceListItem.qml
index 642ca02..2302160 100644
--- a/applet/contents/ui/DeviceListItem.qml
+++ b/applet/contents/ui/DeviceListItem.qml
@@ -27,8 +27,8 @@ ListItemBase {
     label: currentPort ? i18nc("label of device items", "%1 (%2)", currentPort.description, Description) : Description
     icon: {
         if (currentPort) {
-            if (currentPort.name.indexOf("speaker") != -1) {
-                return "audio-speakers-symbolic";
+            if (currentPort.name.indexOf("speaker") != -1 || currentPort.name.indexOf("lineout") != -1) {
+                return "audio-speakers";
             } else if (currentPort.name.indexOf("headphones") != -1) {
                 return "audio-headphones";
             } else if (currentPort.name.indexOf("hdmi") != -1) {

I hope you guys can reconsider the decision of relying on device.form_factor.

Thanks a lot!