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

Igor Poboiko igor.poboiko at gmail.com
Mon Mar 14 21:17:50 CET 2011 


> On March 14, 2011, 7:06 p.m., Diego Casella wrote:
> > Ok, sorry again for my late reply :(
> > Services are working great, however, I think you should refactor the way the 'mixer' DataEngine works, because it doesn't completely performs what it is supposed to.
> > Let me explain better: when you invoke the "mixer" DataEngine, you get only the audio cards names, and nothing more. You have to query() the DataEngine, passing one of those names, to receive further infos about the specific control of that audio card (this is what I, and I think other people does, expect when using this DataEngine).
> > And this is kinda bad because, since the DataEngine doesn't automatically updates when the volume changes level/state (see my previous comment), I have to call a query() every N milliseconds and then, for each control, check is something has changed and do a manual update of the plasmoid if needed.
> > Long story short: instead of returning a "Mixers" datasource, with key set to "Mixers" again, you should return a datasource with all the MIXER_ID's and, for each of them, all their detailed infos (volume and mute state included), so we can get all we need in one shot :)
> > (You could run "plasmaengineexplorer" and watch a couple of engines, such as 'org.kde.activities', 'hotplug' or 'tasks' to see what I mean)
> > 
> > Note that this will fix also the update() issue because, with the current implementation, the plasmoid won't update unless the value of one of the "Mixers" keys changes (since it contains only the audio card names, it means we will be notified of changes only if an audio card has been plugged/removed).
> > 
> > Anyways, these are my two cent: Aaron, what do you think about it?

I thought that the 'dataUpdated' signal is emitted every time data changes (even from inside the dataengine - in our case, I update the mute/volume by DBus signals from inside the dataengine, and it updates automatically in plasmaengineexplorer), and there is no need to poll the dataengine.
The other problem is that actually the plasmoid needs information only about few controls (one or a bit more sliders in plasmoid). So is there a need to set ALL the controls (by default)?
And again, what is the difference between adding these sources by default and querying it, for example, on plasmoid start? I guess I misunderstood something, but don't you anyway need to poll these sources to get all changing data?
And one more issue - there are three types of datasources: one basic (called "Mixers", provides info about available Mixers/soundcards), some for mixers/soundcards (which provides information about its controls) and some for controls (which provides information about all its state - volume level, mute, etc). Should I add all these sources? And should I add it automatically add all sources for plugged devices-sources?

Huh, I asked so many questions.. The things you are talking about are easy-to-fix, I maybe just don't understand correctly what you need :)


- Igor


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


On March 8, 2011, 7:01 p.m., Igor Poboiko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6587/
> -----------------------------------------------------------
> 
> (Updated March 8, 2011, 7:01 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/gui/kmixdockwidget.cpp 1223808 
>   /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/20110314/479d120d/attachment.htm

More information about the Plasma-devel mailing list