D16082: Notify headphone being plugged in on some hardware

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> main.qml:173
> +            // For some hardware, headphones are a port of the build-in audio sink
> +            var text = defaultSink.ports[defaultSink.activePortIndex].description;
> +            var icon;

Please cache `defaultSink.ports[defaultSink.activePortIndex]` in a variable.

Also, can this be out of bounds or otherwise throw an error?

> main.qml:175
> +            var icon;
> +            if (defaultSink.ports[defaultSink.activePortIndex].name == "analog-output-headphones") {
> +                icon = Icon.formFactorIcon("headphone");

I would prefer if hardcoded values like these are put into a helper function or `formFactorIcon` augmented to take those into account also

> main.qml:181
> +
> +            if (!icon || icon == "") {
> +                icon = Icon.formFactorIcon(defaultSink.formFactor);

`if (!icon) {` is sufficient

> main.qml:187
>              if (!icon) {
> +                text = defaultSink.description;
>                  // Show "muted" icon for Dummy output

The `text =` assignments seem to serve no purpose, ie. the end result would be no different than having it down where it was

> pulseaudio.cpp:264
>      connect(sink, &Sink::stateChanged, this, &SinkModel::updatePreferredSink);
> +    connect(sink, &Sink::activePortIndexChanged, this, [this]() {
> +        Q_EMIT defaultSinkChanged();

You can connect a signal to a signal:

  connect(sink, &Sink::activePortIndexChanged, this, &Sink::defaultSinkChanged);

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D16082

To: thsurrel, #plasma, #vdg, drosca
Cc: broulik, nicolasfella, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20181009/892abdc1/attachment-0001.html>