D13931: [KCM] Add speaker placement test

This revision was automatically updated to reflect the committed changes. Closed by commit R115:ed0c58639d12: [KCM] Add speaker placement test (authored by nicolasfella). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13931?vs=37743=37753#toc REPOSITORY R115 Plasma Audio Volume Applet

D13931: [KCM] Add speaker placement test

nicolasfella added a comment. Thanks for bearing with me! :) REPOSITORY R115 Plasma Audio Volume Applet BRANCH speakertest REVISION DETAIL https://phabricator.kde.org/D13931 To: nicolasfella, drosca Cc: ngraham, #vdg, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed,

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37743. nicolasfella added a comment. - Use brute force approach REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13931?vs=37735=37743 BRANCH speakertest REVISION DETAIL

D13931: [KCM] Add speaker placement test

drosca added inline comments. INLINE COMMENTS > nicolasfella wrote in Advanced.qml:117 > When I remove this line and switch profile e.g. from Stereo to 5.1 the UI > doesn't get updated. I'm no QML expert but it looks like a change to > sinkmodel does not result in reevaluation of data(). I'm

D13931: [KCM] Add speaker placement test

nicolasfella added inline comments. INLINE COMMENTS > drosca wrote in Advanced.qml:117 > What is this doing? > > onDataChanged will be triggered only when some property of data in model > changes, and in that case you overwritten the binding that is set in grid, so > grid.pulseObject will no

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37735. nicolasfella marked 9 inline comments as done. nicolasfella added a comment. - Fix unref'ing - Save context in variable - Fix * placement - Fix QML style REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE

D13931: [KCM] Add speaker placement test

drosca added inline comments. INLINE COMMENTS > canberracontext.cpp:27 > > -class Sink : public Device > +CanberraContext * CanberraContext::s_context = nullptr; > + `*` > canberracontext.cpp:29 > + > +CanberraContext * CanberraContext::instance() > { `*` > canberracontext.cpp:51 > >

D13931: [KCM] Add speaker placement test

nicolasfella added inline comments. INLINE COMMENTS > drosca wrote in Advanced.qml:132 > Make it into one property, without `index` and `role`, either with pasting > the code inline or moving it to separate function. Also it probably should be > `readonly` property. I couldn't make it

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37660. nicolasfella added a comment. - Fix context ref'ing in sink REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13931?vs=37659=37660 BRANCH speakertest REVISION DETAIL

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37659. nicolasfella added a comment. - Variable initialization and pointer style - Wait for sound being played before unref'ing context - Merge properties - Remove log - Fix unused parameter warning REPOSITORY R115 Plasma Audio Volume

D13931: [KCM] Add speaker placement test

drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Looks much better now. INLINE COMMENTS > canberracontext.h:44 > private: > -ca_context *m_context = nullptr; > +ca_context *m_canberra; > +int m_references;

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37656. nicolasfella added a comment. - refcount CanberraContext REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13931?vs=37602=37656 BRANCH speakertest REVISION DETAIL

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37602. nicolasfella added a comment. - Remove comment - Remove empty destructor - Add else if REPOSITORY R115 Plasma Audio Volume Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13931?vs=37601=37602 BRANCH speakertest

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37601. nicolasfella added a comment. Simplify model access Thanks for your help! Now, when I switch profiles I get error messages that pulseObject is null, however it seems to work fine. REPOSITORY R115 Plasma Audio Volume Applet

D13931: [KCM] Add speaker placement test

nicolasfella added a comment. In D13931#290548 , @drosca wrote: > In D13931#290544 , @nicolasfella wrote: > > > I can get the PulseObject, that is not the problem, the problem is that I can't

D13931: [KCM] Add speaker placement test

drosca added a comment. In D13931#290544 , @nicolasfella wrote: > I can get the PulseObject, that is not the problem, the problem is that I can't access the model outside of a delegate. model[index] does not work because QAbstractItemModel

D13931: [KCM] Add speaker placement test

nicolasfella added inline comments. INLINE COMMENTS > drosca wrote in volumefeedback.cpp:36 > What if there is multiple VolumeFeedback instances? It will crash then. > You should either move canberra context to Context class (should probably be > fine) or implement the refcounting of

D13931: [KCM] Add speaker placement test

nicolasfella added a comment. In D13931#290535 , @drosca wrote: > And the whole ComboBox/SinkModel code needs to be changed. You should be able to get the PulseObject from the model just with QML code (see invokables in

D13931: [KCM] Add speaker placement test

drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. And the whole ComboBox/SinkModel code needs to be changed. You should be able to get the PulseObject from the model just with QML code (see invokables in

D13931: [KCM] Add speaker placement test

nicolasfella added a comment. The ComboBox part is a little hacky because I couldn't figure out a nice way to access the model data from outside a delegate REPOSITORY R115 Plasma Audio Volume Applet REVISION DETAIL https://phabricator.kde.org/D13931 To: nicolasfella, drosca Cc:

D13931: [KCM] Add speaker placement test

nicolasfella updated this revision to Diff 37534. nicolasfella marked 2 inline comments as done. nicolasfella edited the summary of this revision. nicolasfella added a comment. - Fix indentation in QML - Add i18n calls - Use QLatin1String - Coding style - Switch statement indentation

D13931: [KCM] Add speaker placement test

drosca requested changes to this revision. drosca added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > Advanced.qml:112 > + > +delegate: > + So this will show this grid as many times as you have sinks below each other and scrollable in ListView

D13931: [KCM] Add speaker placement test

nicolasfella created this revision. nicolasfella added a reviewer: drosca. Restricted Application added a project: Plasma. Restricted Application edited subscribers, added: plasma-devel; removed: Plasma. nicolasfella requested review of this revision. REVISION SUMMARY An equivalent to the