On 2 February 2016 at 14:39, Pali Rohár <pali.rohar at gmail.com> wrote: > On Tuesday 02 February 2016 14:24:07 Arun Raghavan wrote: >> > > > @@ -35,16 +35,18 @@ >> > > > >> > > > #include "module-bluetooth-policy-symdef.h" >> > > > >> > > > -PA_MODULE_AUTHOR("Frédéric Dalleau"); >> > > > -PA_MODULE_DESCRIPTION("When a bluetooth sink or source is added, >> > > > load module-loopback"); >> > > > +PA_MODULE_AUTHOR("Frédéric Dalleau, Pali Rohár"); >> > > > +PA_MODULE_DESCRIPTION("Automatically switch between bluetooth >> > > > hsp >> > > > and a2dp profiles and automatically load module-loopback when a >> > > > bluetooth sink (ag) or source (ag, a2dp_source) is added"); >> > > >> > > I'd just rewrite this to be "Policy module to make using bluetooth >> > > devices out-of-the-box easier". >> > >> > Personally I'm fine with both descriptions so its up to other >> > pulseaudio >> > reviewers/maintainers. >> >> I prefer the shorter name in the description, so let's go with that. > > Ok. > >> > > > PA_MODULE_VERSION(PACKAGE_VERSION); >> > > > PA_MODULE_LOAD_ONCE(true); >> > > > PA_MODULE_USAGE( >> > > > + "switch=<Switch between hsp and a2dp profile?> " >> > > >> > > Maybe call this "autoswitch". >> > >> > "autoswitch" or "auto_switch"? >> >> I'm not a huge fan of the underscores in our modarg names, but I'm fine >> with either. > > Ok. > >> > > > + /* Flag this card for revert */ >> > > > + pa_proplist_sets(new_data->proplist, "bluez-revert", >> > > > "true"); >> > > > + return PA_HOOK_OK; >> > > >> > > What is the reasoning behind having this property? >> > >> > For each card I need to know if card needs to be "reverted" in >> > future. >> >> Is this for a case where a device is manually kept in the hsp profile >> and you don't want to touch that? > > Yes. When user chose manually to use hsp profile. > >> > > In general, I prefer the module doing its book-keeping locally >> > > rather >> > > than (ab)using the proplist for state. >> > >> > Ok, what to use then? How to store for each private boolean flag >> > needed >> > just by this module? >> >> Usually you'd do something like have a hashmap in the module's >> userdata, and then you could add your pa_card to that hashmap if it >> needs a revert (and remove when you're done with the revert). The >> userdata is available in all the hook callbacks. > > But then I need to deal with removing card (e.g. somebody unplug usb > bluetooth adapter when voice call via hsp profile is active)... And > maybe other scenarios? Yes. You just need to use PA_CORE_HOOK_CARD_UNLINK. -- Arun