Fix 'Audio Setup'
AbandonedPublic

Authored by sgoth on Mar 24 2020, 2:47 PM.

Details

Reviewers
None
Summary

kcm_phonon doesn't exist since plasma 5.17 anymore.
Pulseaudio enabled builds depend on 'kcm_pulseaudio' from plasma-pa,
others use 'phononsettings'.

See D22616

Diff Detail

Repository
R345 KMix
Branch
audiosetup_kcm (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24153
Build 24171: arc lint + arc unit
sgoth requested review of this revision.Mar 24 2020, 2:47 PM
sgoth created this revision.
sgoth added a comment.Mar 24 2020, 2:53 PM

I'm aware that the proposed change is still far from optimal as the error handling doesn't work for missing kcm modules and still may result in silent errors for the user.
If you think rewriting that to verify the kcm actual exists is the way to go, please say so. I was reluctant to add the KF5Service dependency to enumerate kcms (at least i see kcmshell uses that).

Same check should imo also be added for phononsettings. This is shipped with phonon package, but kmix only indirectly pulls in that via knotifications, where it is optional.

sgoth added a comment.Mar 24 2020, 4:10 PM

If phononsettings is missing/uncallable the message box inf KMixWindow::forkExec is shown providing at least some feedback.
For the kcmshell5 call, KProcess::startDetached is able to start the process - but that fails internally. I don't see a way to fix that in this 'fire and forget' style besides validating kcm_pulseaudio is available.

In downstream I've simply removed that menu item for now.

sgoth added a comment.Apr 14 2020, 6:42 PM

Yeah, i consider it the better option.
Depending on kcm_pulseaudio which comes with plasma-pa is rather moot as plasma-pa disables kmix.

sgoth abandoned this revision.Apr 14 2020, 6:43 PM