Review Request: Rework KMix DBus API and add KMix plasma dataengine

[Igor Poboiko]

> On March 7, 2011, 11:54 p.m., Christian Esken wrote:
> > /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp, line 987
> > <http://svn.reviewboard.kde.org/r/6587/diff/2/?file=45477#file45477line987>
> >
> >     I agree that "mixer->toggleMute(md->id());" was quite ugly, and it is good that it was changed. But "md->toggleMute()" would be the really elegant solution. Is there was a real reason behind it? Can/should it e changed to md->toggleMute()?

Just only because this method is used only here (and was used in DBus). But since I split all DBus part from Mixer code, I thought there is no need for this method. It can be changed to toggleMute() method in MixDevice class, and I'll do it (it looks just nicer, I agree :) )


- Igor


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6587/#review9969
-----------------------------------------------------------


On March 6, 2011, 7:24 p.m., Igor Poboiko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6587/
> -----------------------------------------------------------
> 
> (Updated March 6, 2011, 7:24 p.m.)
> 
> 
> Review request for Plasma and Diego Casella.
> 
> 
> Summary
> -------
> 
> This patch reworks KMix DBus API and adds a plasma dataengine+service as a frontend to information provided by DBus.
> New DBus structure is:
>  - /Mixers
> used to get some global information, such as available mixers list and global master mixer
>  - /Mixers/MIXER_ID
> used to get information about mixer with id=MIXER_ID. It provides such information as list of available controls, name of this mixer, id, etc
>  - /Mixers/MIXER_ID/CONTROL_ID
> used to get and set information about control. Such information as volume level, mute, name of control, etc.
> It also adds a DBus signals which are emitted when new mixer/control appears, or volume level changes.
> It also splits all dbus-related code to separate class, DBus{KMix,Mixer,Control}Wrapper.
> 
> The Plasma Dataengine:
> By default, the only available source is "KMix". It provides information global information about KMix: is KMix running, and list of available mixers. (its IDs)
> Source for every mixer is called by it's ID (for example, "ALSA::HDA_Intel:1"). This source provides such information about current Mixer as: it's readable name, is it opened, its balance and list of available controls. It also adds basic sources for every control, which provides only information about its readable name
> Sources for controls are called by 'MIXER_ID/CONTROL_ID' (for example, "ALSA::HDA_Intel:1/Master:0"). If you request this source, it will provide such information as its readable name, is it muted and its volume level (which are updates automatically, using DBus signals).
> There is a service available for controls sources. It provides such methods as setVolume() and setMute().
> 
> It doesn't close bug 171287, but it becomes one step closer to its solving :)
> 
> And, I'm not very familiar with CMake, but it would be great idea to make plasma part optional.
> 
> 
> This addresses bug 171287.
>     https://bugs.kde.org/show_bug.cgi?id=171287
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1223808 
>   /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1223808 
>   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.h 1223808 
>   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1223808 
>   /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1223808 
>   /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1223808 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.control.xml PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixer.xml PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixset.xml PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/CMakeLists.txt PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixer.operations PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerservice.h PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerservice.cpp PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/plasma/engine/plasma-engine-mixer.desktop PRE-CREATION 
>   /trunk/KDE/kdemultimedia/kmix/tests/CMakeLists.txt 1223808 
> 
> Diff: http://svn.reviewboard.kde.org/r/6587/diff
> 
> 
> Testing
> -------
> 
> KMix from KDE SC 4.6.0 compiles ok with this patch, and patch applies to current trunk.
> Tested on system with one card and with ALSA backend, so I don't know is plasma dataengine works correctly with plugging/unplugging mixers (but it should).
> All DBus methods/properties works fine, signals are emitted, volume can be set using DBus methods.
> 
> Plasma dataengine was tested using plasmaengineexplorer. 
> All works fine except the one thing. When I request an source for Mixer, it also adds soucres for controls. And then when I request source for already available Control, it doesn't react anyhow. But when I set "Update every % ms", and click "Reqeust", it works fine. If I request a source for control BEFORE requesting the source for mixer, all works fine too (without setting "Update every % ms"). I don't know is it a plasmaengineexplorer bug, or my.
> 
> 
> Thanks,
> 
> Igor
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110308/e52b2b4e/attachment.htm