Hi Tanu, On Mon, Mar 24, 2014 at 1:18 AM, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote: > On Wed, 2014-03-19 at 18:48 -0700, eu at felipetonello.com wrote: >> From: "Felipe F. Tonello" <eu at felipetonello.com> >> >> This module is used to change a card profile when a voice call is received >> from oFono. >> >> It also provides a functionality to avoid suspension of sink/source that >> module-suspend-on-idle does. This is usefull since most of those pcms are >> dummy just to change the codec route to the modem in the kernel driver. If you >> use this functinoality, please load this module before loading >> module-suspend-on-idle. >> >> Signed-off-by: Felipe F. Tonello <eu at felipetonello.com> >> --- >> src/Makefile.am | 12 +- >> src/modules/module-ofono-switch-on-voicecall.c | 540 +++++++++++++++++++++++++ >> 2 files changed, 550 insertions(+), 2 deletions(-) >> create mode 100644 src/modules/module-ofono-switch-on-voicecall.c > > Thanks for the patch! I don't have time to do proper review now, so I'll > add this to the patch queue, which you can see here: > http://www.freedesktop.org/wiki/Software/PulseAudio/PatchStatus/ > > I have some comments, though: > > Please follow the coding style guidelines[1]. I only glanced at the > code, but I see tabs being used for indentation and lines longer than > 128 characters. I will fix it. > > Why does this module have to be loaded before module-syspend-on-idle? > Such load order restrictions should be avoided if at all possible. IIRC this was necessary because the sink/source new callback from this module had to be called first then the one from module-suspend-on-idle. Does it make sense? > > I would like to have a shared library for handling the D-Bus > communication. The library would provide an API for dealing with oFono > from various modules, one of which could be > module-ofono-switch-on-voicecall. An example of that sort of library is > libbluez5-util.so, which implements an abstraction layer for BlueZ 5. > libbluez5-util.so implements a singleton object that is created when the > first Bluetooth module is loaded and deleted when the last Bluetooth > module is unloaded. This pattern would be good for oFono also. I'm not > requiring you to do this, if you think it's too much work (I might then > some day do the refactoring myself). If you do this, you should > coordinate that work with Jo?o Paulo Rechi Vita (CC'd), who has also > written code for interfacing with oFono. Jo?o's work has not been > reviewed yet either. Ok. I will check the possibility and answer in this thread if it worth to make a v2 with this library. Anyway, this module make very basic usage of oFono API. It looks more complex only because of D-Bus itself. Felipe