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

2011-08-22 Thread Christian Esken
Am Samstag, 20. August 2011, 14:36:40 schrieb Mark Gaiser:
 
  On Aug. 20, 2011, 1:22 a.m., Mark Gaiser wrote:
   Hi,
   
   I was just trying to do the same thing with kmix and wasted ~6 hours on 
that (or even more) just to find that is was already here but never committed. 
So how are we on this? Can this be committed?
   
   Regards,
   Mark
 
 Sorry, it is already committed. I was looking for a commit hook message or a 
Ship it! message.. Guess it was silently committed. It can already be 
found in the current 4.7 codebase.
 Sorry for the noise.

Hi Mark,

thanks for caring. No problem about the noise. :-)

I just looked at the review request and saw it is labeled with This change 
has been marked as submitted.. Should be enough, I would say. 
http://techbase.kde.org/Development/Review_Board#Closing_a_review_request is 
not too specific about how to properly close a review request, but I guess a 
simple close is enough.

 Christian

 
 
 - Mark
 
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/#review10358
 ---
 
 
 On April 7, 2011, 8:40 a.m., Igor Poboiko wrote:
  
  ---
  This is an automatically generated e-mail. To reply, visit:
  http://svn.reviewboard.kde.org/r/6587/
  ---
  
  (Updated April 7, 2011, 8:40 a.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/core/mixdevice.h 1225808 
/trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
/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 1225808 
/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 

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

2011-08-20 Thread Mark Gaiser


 On Aug. 20, 2011, 1:22 a.m., Mark Gaiser wrote:
  Hi,
  
  I was just trying to do the same thing with kmix and wasted ~6 hours on 
  that (or even more) just to find that is was already here but never 
  committed. So how are we on this? Can this be committed?
  
  Regards,
  Mark

Sorry, it is already committed. I was looking for a commit hook message or a 
Ship it! message.. Guess it was silently committed. It can already be found 
in the current 4.7 codebase.
Sorry for the noise.


- Mark


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


On April 7, 2011, 8:40 a.m., Igor Poboiko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/
 ---
 
 (Updated April 7, 2011, 8:40 a.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/core/mixdevice.h 1225808 
   /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
   /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
   /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 1225808 
   /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 1225808 
 
 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.
 

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

2011-08-19 Thread Mark Gaiser

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


Hi,

I was just trying to do the same thing with kmix and wasted ~6 hours on that 
(or even more) just to find that is was already here but never committed. So 
how are we on this? Can this be committed?

Regards,
Mark

- Mark


On April 7, 2011, 8:40 a.m., Igor Poboiko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/
 ---
 
 (Updated April 7, 2011, 8:40 a.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/core/mixdevice.h 1225808 
   /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
   /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
   /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 1225808 
   /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 1225808 
 
 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 

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

2011-04-07 Thread Igor Poboiko

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

(Updated April 7, 2011, 8:40 a.m.)


Review request for Plasma and Diego Casella.


Changes
---

Changed QDBusObjectPath everywhere to QString, since not all software supports 
DBus OBJECT_PATH.


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 (updated)
-

  /trunk/KDE/kdemultimedia/kmix/core/mixdevice.h 1225808 
  /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
  /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
  /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 1225808 
  /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 1225808 

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 

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

2011-03-26 Thread Igor Poboiko

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

(Updated March 26, 2011, 12:15 p.m.)


Review request for Plasma and Diego Casella.


Changes
---

 - Fixed crash when called removeSource() in dataengine destructor (thanks to 
Diego for report :)
 - Added some data sources such as Can Be Muted (shows can this particular 
control be muted), Icon (QIcon - an icon for control), Controls Icons Names 
(list of QStrings - names of controls icons in mixer)
 - Added DBus properties and methods: currentMasterMixer, currentMasterControl, 
preferredMasterMixer, preferredMasterControl, setCurrentMaster(), 
setPreferredMaster() (does obvious things) and canMute() (for controls)

I didn't add ability to change and work with preferred/current master in 
dataengine because I didn't find any way to monitor its changes (in DBus part)
Comments are welcome :)


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 (updated)
-

  /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
  /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixdevice.h 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
  /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
  /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 1225808 
  /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 1225808 

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 

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

2011-03-17 Thread Diego Casella


 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?
 
 Igor Poboiko wrote:
 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 :)
 
 Diego Casella wrote:
 Err, I was referencing the controls with the same index of the audio card 
 ( i.e. 'ALSA::HDA_Intel:1' and thus 'Master:1', instead of 'Master:0'), my 
 bad :(
 So, forget what I said :)
 Anyways, one more observation: if we want to provide a complete 
 replacement of the old KMix applet, the plasmoid should be able to provide a 
 'widget' to select/change which channel we are currently operating with 
 (master, pcm, speakers ..). In other words, we need a Plasma Service to 
 change the current master channel, and one more entry in the dataengine to 
 identify which channel is currently being controlled.
 
 Igor Poboiko wrote:
 Yep, I thought about it. My idea was that the control shown in plasmoid 
 should be independent from master mixer in KMix. I mean, we just show one (or 
 more) control(s) and we don't care about KMix's master control (we have own 
 settings about visible controls, etc).
 But now I'm not sure it is good idea (since, for example, shortcuts are 
 assigned to master control) But anyway - KMix is running during the all 
 session, so I can provide information about master mixer/control in 
 dataengine and if user want to change it we can just call KMix to show its 
 window and show its settings for selecting master mixer/control :)
 What do you think about it?

I'd like to show this option without calling KMix directly, for consistency and 
visual appearance, so it will be awesome if you can include a service call to 
set it, and an entry in the dataengine which tells us which control we are 
using :)
Uh, an other observation again: since some controls doesn't have a mute state 
(like PCM or Mic Boost), I think you should also add an other entry, for 
each control, which tells us whether the control can be muted or not.


- Diego


---
This is an 

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

2011-03-15 Thread Diego Casella


 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?
 
 Igor Poboiko wrote:
 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 :)

Err, I was referencing the controls with the same index of the audio card ( 
i.e. 'ALSA::HDA_Intel:1' and thus 'Master:1', instead of 'Master:0'), my bad :(
So, forget what I said :)
Anyways, one more observation: if we want to provide a complete replacement of 
the old KMix applet, the plasmoid should be able to provide a 'widget' to 
select/change which channel we are currently operating with (master, pcm, 
speakers ..). In other words, we need a Plasma Service to change the current 
master channel, and one more entry in the dataengine to identify which channel 
is currently being controlled.


- Diego


---
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.
 

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

2011-03-15 Thread Igor Poboiko


 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?
 
 Igor Poboiko wrote:
 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 :)
 
 Diego Casella wrote:
 Err, I was referencing the controls with the same index of the audio card 
 ( i.e. 'ALSA::HDA_Intel:1' and thus 'Master:1', instead of 'Master:0'), my 
 bad :(
 So, forget what I said :)
 Anyways, one more observation: if we want to provide a complete 
 replacement of the old KMix applet, the plasmoid should be able to provide a 
 'widget' to select/change which channel we are currently operating with 
 (master, pcm, speakers ..). In other words, we need a Plasma Service to 
 change the current master channel, and one more entry in the dataengine to 
 identify which channel is currently being controlled.

Yep, I thought about it. My idea was that the control shown in plasmoid should 
be independent from master mixer in KMix. I mean, we just show one (or more) 
control(s) and we don't care about KMix's master control (we have own settings 
about visible controls, etc).
But now I'm not sure it is good idea (since, for example, shortcuts are 
assigned to master control) But anyway - KMix is running during the all 
session, so I can provide information about master mixer/control in dataengine 
and if user want to change it we can just call KMix to show its window and show 
its settings for selecting master mixer/control :)
What do you think about it?


- 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 

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

2011-03-14 Thread Diego Casella

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


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?

- Diego


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 
   

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

2011-03-14 Thread Igor Poboiko


 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 

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

2011-03-08 Thread 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: 

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

2011-03-08 Thread Igor Poboiko

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

(Updated March 8, 2011, 8:38 a.m.)


Review request for Plasma and Diego Casella.


Changes
---

Added toggleMute method to MixDevice class and switched to it everywhere where 
we need to toggle mute.


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 (updated)
-

  /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 

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

2011-03-08 Thread Diego Casella

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


Sorry for the late reply Igor, I was kinda busy these days. I've tried the 
dataengine within a test plasmoid I did, and it works great :)
I can get and query all the channels, volume changes are correctly notified if 
an external application modifies the current level,
and works fine with the multimedia keys too.
I couldn't test the Plasma Services yet because I can't invoke them within 
javascript; as soon as I get I reply from plasma ml I will try everything.
Now, a little bit tricky part (Aaron, correct me if I'm wrong): if I write the 
following code

dataEngine(mixer).connectSource(Mixers, plasmoid)

instead of

dataEngine(mixer).connectSource(Mixers, plasmoid, 500)

I would expect to be notified of volume changes only when a volume change 
actually takes place, instead of requesting the dataengine's data every 500ms.
In your current implementation this is not happening, and I'd _really_ like to 
see this feature implemented.

- Diego


On March 8, 2011, 8:38 a.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, 8:38 a.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 
   

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

2011-03-08 Thread Igor Poboiko

---
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.


Changes
---

Fixed a typo that caused crash when new control is plugged (I tested with 
PulseAudio backend and starting Amarok). Now it looks like works fine.


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 (updated)
-

  /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 % 

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

2011-03-07 Thread Aaron Seigo

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


the DataEngine part looks good now.. nice work :)

i'll leave the review of the kmix parts to the kmix devs, though ...

- Aaron


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 

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

2011-03-07 Thread Christian Esken

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



/trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11182

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()?



/trunk/KDE/kdemultimedia/kmix/core/mixer.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11183

It is good to see some cleanups here [controlChangedForwareder()] - thanks 
a lot.


- Christian


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 

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

2011-03-05 Thread Aaron Seigo

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


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.

this is because the data for the control (or at least, the readableName) is set 
in updateMixerData. then, when it is later requested the DataEngine sees it 
exists already and so does not call sourceRequestEvent. since there is no poll 
(time) request, updateSourceEvent isn't called either. when there is a poll 
time set, then updateSourceEvent is (eventually) called and the data is 
updated. the fix is to not set any data on the control until such time as a 
visualization requests it. you can set up the behind-the-scenes objects, but 
leave the setData calls for the control out of updateMixerData.

there are some memory leaks that need closing as well.

i also recommend, for stylistic reasons, using natural language labels rather 
than camelCaseAsIfItsAFunctionName labels. e.g.: Controls, Readable Name, 
etc.


/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11146

this is wrong. if the name is KMix then the source created _must_ be 
KMix, but getKMixData creates/sets running and mixers

my suggestion: change this to if (name == mixers)



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11145

does this matter? if mixers is empty, shouldn't that be enough?



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11147

where is this deleted?



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11148

looks like a good candidate for a QHash rather than a QList



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11149

perhaps another opportunity for a hash.



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11151

another good candidate for a hash



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11150

where is this deleted?



/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11152

no data for the control should be set here. this belongs in 
updateControlData


- Aaron


On March 5, 2011, 7:56 a.m., Igor Poboiko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/
 ---
 
 (Updated March 5, 2011, 7:56 a.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.