[v5,1/4] bluetooth: use consistent profile names

Submitted by James Bottomley on Sept. 21, 2017, 7:26 p.m.

Details

Message ID 1506021989.3848.67.camel@HansenPartnership.com
State New
Headers show
Series "use bluetooth HFP in pulseaudio native backend when available" ( rev: 4 3 2 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

James Bottomley Sept. 21, 2017, 7:26 p.m.
The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
names using profile_function instead of randomly made up names.  Fix
this with the transformation:

PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> PA_BLUETOOTH_PROFILE_HSP_HS
PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> PA_BLUETOOTH_PROFILE_HFP_AG

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

v4: update for PA 11.0
---
 src/modules/bluetooth/backend-native.c       | 28 +++++++--------
 src/modules/bluetooth/backend-ofono.c        |  4 +--
 src/modules/bluetooth/bluez5-util.c          | 10 +++---
 src/modules/bluetooth/bluez5-util.h          |  4 +--
 src/modules/bluetooth/module-bluez5-device.c | 54 ++++++++++++++--------------
 5 files changed, 50 insertions(+), 50 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/bluetooth/backend-native.c b/src/modules/bluetooth/backend-native.c
index 0f0104d8..f2009bfd 100644
--- a/src/modules/bluetooth/backend-native.c
+++ b/src/modules/bluetooth/backend-native.c
@@ -449,7 +449,7 @@  static void set_speaker_gain(pa_bluetooth_transport *t, uint16_t gain) {
     /* If we are in the AG role, we send a command to the head set to change
      * the speaker gain. In the HS role, source and sink are swapped, so
      * in this case we notify the AG that the microphone gain has changed */
-    if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT) {
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
         len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
         pa_log_debug("RFCOMM >> +VGS=%d", gain);
     } else {
@@ -476,7 +476,7 @@  static void set_microphone_gain(pa_bluetooth_transport *t, uint16_t gain) {
     /* If we are in the AG role, we send a command to the head set to change
      * the microphone gain. In the HS role, source and sink are swapped, so
      * in this case we notify the AG that the speaker gain has changed */
-    if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT) {
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS) {
         len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
         pa_log_debug("RFCOMM >> +VGM=%d", gain);
     } else {
@@ -509,9 +509,9 @@  static DBusMessage *profile_new_connection(DBusConnection *conn, DBusMessage *m,
 
     handler = dbus_message_get_path(m);
     if (pa_streq(handler, HSP_AG_PROFILE)) {
-        p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+        p = PA_BLUETOOTH_PROFILE_HSP_HS;
     } else if (pa_streq(handler, HSP_HS_PROFILE)) {
-        p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+        p = PA_BLUETOOTH_PROFILE_HFP_AG;
     } else {
         pa_log_error("Invalid handler");
         goto fail;
@@ -626,11 +626,11 @@  static void profile_init(pa_bluetooth_backend *b, pa_bluetooth_profile_t profile
     pa_assert(b);
 
     switch (profile) {
-        case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+        case PA_BLUETOOTH_PROFILE_HSP_HS:
             object_name = HSP_AG_PROFILE;
             uuid = PA_BLUETOOTH_UUID_HSP_AG;
             break;
-        case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+        case PA_BLUETOOTH_PROFILE_HFP_AG:
             object_name = HSP_HS_PROFILE;
             uuid = PA_BLUETOOTH_UUID_HSP_HS;
             break;
@@ -647,10 +647,10 @@  static void profile_done(pa_bluetooth_backend *b, pa_bluetooth_profile_t profile
     pa_assert(b);
 
     switch (profile) {
-        case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+        case PA_BLUETOOTH_PROFILE_HSP_HS:
             dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection), HSP_AG_PROFILE);
             break;
-        case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+        case PA_BLUETOOTH_PROFILE_HFP_AG:
             dbus_connection_unregister_object_path(pa_dbus_connection_get(b->connection), HSP_HS_PROFILE);
             break;
         default:
@@ -665,9 +665,9 @@  void pa_bluetooth_native_backend_enable_hs_role(pa_bluetooth_backend *native_bac
        return;
 
    if (enable_hs_role)
-       profile_init(native_backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+       profile_init(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
    else
-       profile_done(native_backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+       profile_done(native_backend, PA_BLUETOOTH_PROFILE_HFP_AG);
 
    native_backend->enable_hs_role = enable_hs_role;
 }
@@ -693,8 +693,8 @@  pa_bluetooth_backend *pa_bluetooth_native_backend_new(pa_core *c, pa_bluetooth_d
     backend->enable_hs_role = enable_hs_role;
 
     if (enable_hs_role)
-       profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
-    profile_init(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+       profile_init(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
+    profile_init(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
 
     return backend;
 }
@@ -705,8 +705,8 @@  void pa_bluetooth_native_backend_free(pa_bluetooth_backend *backend) {
     pa_dbus_free_pending_list(&backend->pending);
 
     if (backend->enable_hs_role)
-       profile_done(backend, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
-    profile_done(backend, PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT);
+       profile_done(backend, PA_BLUETOOTH_PROFILE_HFP_AG);
+    profile_done(backend, PA_BLUETOOTH_PROFILE_HSP_HS);
 
     pa_dbus_connection_unref(backend->connection);
 
diff --git a/src/modules/bluetooth/backend-ofono.c b/src/modules/bluetooth/backend-ofono.c
index 2c51497f..85b9c477 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -223,7 +223,7 @@  static void hf_audio_agent_card_found(pa_bluetooth_backend *backend, const char
     const char *key, *value;
     struct hf_audio_card *card;
     pa_bluetooth_device *d;
-    pa_bluetooth_profile_t p = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+    pa_bluetooth_profile_t p = PA_BLUETOOTH_PROFILE_HFP_AG;
 
     pa_assert(backend);
     pa_assert(path);
@@ -257,7 +257,7 @@  static void hf_audio_agent_card_found(pa_bluetooth_backend *backend, const char
             card->local_address = pa_xstrdup(value);
         } else if (pa_streq(key, "Type")) {
             if (pa_streq(value, "gateway"))
-                p = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+                p = PA_BLUETOOTH_PROFILE_HSP_HS;
         }
 
         pa_log_debug("%s: %s", key, value);
diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
index c9283232..4470f2ef 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -174,10 +174,10 @@  static bool device_supports_profile(pa_bluetooth_device *device, pa_bluetooth_pr
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
         case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
-        case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+        case PA_BLUETOOTH_PROFILE_HSP_HS:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS)
                 || !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HFP_HF);
-        case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+        case PA_BLUETOOTH_PROFILE_HFP_AG:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_AG)
                 || !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HFP_AG);
         case PA_BLUETOOTH_PROFILE_OFF:
@@ -1018,7 +1018,7 @@  void pa_bluetooth_discovery_set_ofono_running(pa_bluetooth_discovery *y, bool is
         pa_bluetooth_device *d;
 
         PA_HASHMAP_FOREACH(d, y->devices, state) {
-            if (device_supports_profile(d, PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)) {
+            if (device_supports_profile(d, PA_BLUETOOTH_PROFILE_HFP_AG)) {
                 DBusMessage *m;
 
                 pa_assert_se(m = dbus_message_new_method_call(BLUEZ_SERVICE, d->path, "org.bluez.Device1", "Disconnect"));
@@ -1304,9 +1304,9 @@  const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
             return "a2dp_sink";
         case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
             return "a2dp_source";
-        case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+        case PA_BLUETOOTH_PROFILE_HSP_HS:
             return "headset_head_unit";
-        case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+        case PA_BLUETOOTH_PROFILE_HFP_AG:
             return "headset_audio_gateway";
         case PA_BLUETOOTH_PROFILE_OFF:
             return "off";
diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
index a3e7bf3d..84c0c3f1 100644
--- a/src/modules/bluetooth/bluez5-util.h
+++ b/src/modules/bluetooth/bluez5-util.h
@@ -46,8 +46,8 @@  typedef enum pa_bluetooth_hook {
 typedef enum profile {
     PA_BLUETOOTH_PROFILE_A2DP_SINK,
     PA_BLUETOOTH_PROFILE_A2DP_SOURCE,
-    PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
-    PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
+    PA_BLUETOOTH_PROFILE_HSP_HS,
+    PA_BLUETOOTH_PROFILE_HFP_AG,
     PA_BLUETOOTH_PROFILE_OFF
 } pa_bluetooth_profile_t;
 #define PA_BLUETOOTH_PROFILE_COUNT PA_BLUETOOTH_PROFILE_OFF
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index 530207a2..d076fbad 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -257,8 +257,8 @@  static int sco_process_render(struct userdata *u) {
     pa_memchunk memchunk;
 
     pa_assert(u);
-    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ||
-                u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HSP_HS ||
+                u->profile == PA_BLUETOOTH_PROFILE_HFP_AG);
     pa_assert(u->sink);
 
     pa_sink_render_full(u->sink, u->write_block_size, &memchunk);
@@ -317,8 +317,8 @@  static int sco_process_push(struct userdata *u) {
     pa_usec_t tstamp = 0;
 
     pa_assert(u);
-    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ||
-                u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY);
+    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HSP_HS ||
+                u->profile == PA_BLUETOOTH_PROFILE_HFP_AG);
     pa_assert(u->source);
     pa_assert(u->read_smoother);
 
@@ -784,7 +784,7 @@  static void transport_release(struct userdata *u) {
 
 /* Run from I/O thread */
 static void transport_config_mtu(struct userdata *u) {
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS || u->profile == PA_BLUETOOTH_PROFILE_HFP_AG) {
         u->read_block_size = u->read_link_mtu;
         u->write_block_size = u->write_link_mtu;
 
@@ -981,7 +981,7 @@  static void source_set_volume_cb(pa_source *s) {
     pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume);
 
     /* Set soft volume when in headset role */
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
+    if (u->profile == PA_BLUETOOTH_PROFILE_HFP_AG)
         pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, volume);
 
     /* If we are in the AG role, we send a command to the head set to change
@@ -1004,7 +1004,7 @@  static int add_source(struct userdata *u) {
     data.namereg_fail = false;
     pa_proplist_sets(data.proplist, "bluetooth.protocol", pa_bluetooth_profile_to_string(u->profile));
     pa_source_new_data_set_sample_spec(&data, &u->sample_spec);
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS)
         pa_proplist_sets(data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
 
     connect_ports(u, &data, PA_DIRECTION_INPUT);
@@ -1012,10 +1012,10 @@  static int add_source(struct userdata *u) {
     if (!u->transport_acquired)
         switch (u->profile) {
             case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
-            case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+            case PA_BLUETOOTH_PROFILE_HFP_AG:
                 data.suspend_cause = PA_SUSPEND_USER;
                 break;
-            case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+            case PA_BLUETOOTH_PROFILE_HSP_HS:
                 /* u->stream_fd contains the error returned by the last transport_acquire()
                  * EAGAIN means we are waiting for a NewConnection signal */
                 if (u->stream_fd == -EAGAIN)
@@ -1039,7 +1039,7 @@  static int add_source(struct userdata *u) {
     u->source->userdata = u;
     u->source->parent.process_msg = source_process_msg;
 
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS || u->profile == PA_BLUETOOTH_PROFILE_HFP_AG) {
         pa_source_set_set_volume_callback(u->source, source_set_volume_cb);
         u->source->n_volume_steps = 16;
     }
@@ -1151,7 +1151,7 @@  static void sink_set_volume_cb(pa_sink *s) {
     pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume);
 
     /* Set soft volume when in headset role */
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
+    if (u->profile == PA_BLUETOOTH_PROFILE_HFP_AG)
         pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, volume);
 
     /* If we are in the AG role, we send a command to the head set to change
@@ -1174,17 +1174,17 @@  static int add_sink(struct userdata *u) {
     data.namereg_fail = false;
     pa_proplist_sets(data.proplist, "bluetooth.protocol", pa_bluetooth_profile_to_string(u->profile));
     pa_sink_new_data_set_sample_spec(&data, &u->sample_spec);
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS)
         pa_proplist_sets(data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
 
     connect_ports(u, &data, PA_DIRECTION_OUTPUT);
 
     if (!u->transport_acquired)
         switch (u->profile) {
-            case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+            case PA_BLUETOOTH_PROFILE_HFP_AG:
                 data.suspend_cause = PA_SUSPEND_USER;
                 break;
-            case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+            case PA_BLUETOOTH_PROFILE_HSP_HS:
                 /* u->stream_fd contains the error returned by the last transport_acquire()
                  * EAGAIN means we are waiting for a NewConnection signal */
                 if (u->stream_fd == -EAGAIN)
@@ -1210,7 +1210,7 @@  static int add_sink(struct userdata *u) {
     u->sink->userdata = u;
     u->sink->parent.process_msg = sink_process_msg;
 
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS || u->profile == PA_BLUETOOTH_PROFILE_HFP_AG) {
         pa_sink_set_set_volume_callback(u->sink, sink_set_volume_cb);
         u->sink->n_volume_steps = 16;
     }
@@ -1219,7 +1219,7 @@  static int add_sink(struct userdata *u) {
 
 /* Run from main thread */
 static void transport_config(struct userdata *u) {
-    if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
+    if (u->profile == PA_BLUETOOTH_PROFILE_HSP_HS || u->profile == PA_BLUETOOTH_PROFILE_HFP_AG) {
         u->sample_spec.format = PA_SAMPLE_S16LE;
         u->sample_spec.channels = 1;
         u->sample_spec.rate = 8000;
@@ -1349,7 +1349,7 @@  static int setup_transport(struct userdata *u) {
 
     u->transport = t;
 
-    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
+    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HFP_AG)
         transport_acquire(u, true); /* In case of error, the sink/sources will be created suspended */
     else {
         int transport_error;
@@ -1369,8 +1369,8 @@  static pa_direction_t get_profile_direction(pa_bluetooth_profile_t p) {
     static const pa_direction_t profile_direction[] = {
         [PA_BLUETOOTH_PROFILE_A2DP_SINK] = PA_DIRECTION_OUTPUT,
         [PA_BLUETOOTH_PROFILE_A2DP_SOURCE] = PA_DIRECTION_INPUT,
-        [PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
-        [PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
+        [PA_BLUETOOTH_PROFILE_HSP_HS] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
+        [PA_BLUETOOTH_PROFILE_HFP_AG] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
         [PA_BLUETOOTH_PROFILE_OFF] = 0
     };
 
@@ -1620,7 +1620,7 @@  static int start_thread(struct userdata *u) {
 
         /* If we are in the headset role, the sink should not become default
          * unless there is no other sound device available. */
-        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
+        if (u->profile == PA_BLUETOOTH_PROFILE_HFP_AG)
             u->sink->priority = 1500;
 
         pa_sink_put(u->sink);
@@ -1636,7 +1636,7 @@  static int start_thread(struct userdata *u) {
         /* If we are in the headset role or the device is an a2dp source,
          * the source should not become default unless there is no other
          * sound device available. */
-        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE)
+        if (u->profile == PA_BLUETOOTH_PROFILE_HFP_AG || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE)
             u->source->priority = 1500;
 
         pa_source_put(u->source);
@@ -1873,7 +1873,7 @@  static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
         p = PA_CARD_PROFILE_DATA(cp);
         break;
 
-    case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
+    case PA_BLUETOOTH_PROFILE_HSP_HS:
         cp = pa_card_profile_new(name, _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 20;
         cp->n_sinks = 1;
@@ -1886,7 +1886,7 @@  static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
         p = PA_CARD_PROFILE_DATA(cp);
         break;
 
-    case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
+    case PA_BLUETOOTH_PROFILE_HFP_AG:
         cp = pa_card_profile_new(name, _("Headset Audio Gateway (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 20;
         cp->n_sinks = 1;
@@ -1961,9 +1961,9 @@  static int uuid_to_profile(const char *uuid, pa_bluetooth_profile_t *_r) {
     else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
         *_r = PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
     else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_HS) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF))
-        *_r = PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT;
+        *_r = PA_BLUETOOTH_PROFILE_HSP_HS;
     else if (pa_streq(uuid, PA_BLUETOOTH_UUID_HSP_AG) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_AG))
-        *_r = PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY;
+        *_r = PA_BLUETOOTH_PROFILE_HFP_AG;
     else
         return -PA_ERR_INVALID;
 
@@ -2174,7 +2174,7 @@  static pa_hook_result_t transport_speaker_gain_changed_cb(pa_bluetooth_discovery
         volume++;
 
     pa_cvolume_set(&v, u->sample_spec.channels, volume);
-    if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS)
         pa_sink_volume_changed(u->sink, &v);
     else
         pa_sink_set_volume(u->sink, &v, true, true);
@@ -2202,7 +2202,7 @@  static pa_hook_result_t transport_microphone_gain_changed_cb(pa_bluetooth_discov
 
     pa_cvolume_set(&v, u->sample_spec.channels, volume);
 
-    if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
+    if (t->profile == PA_BLUETOOTH_PROFILE_HSP_HS)
         pa_source_volume_changed(u->source, &v);
     else
         pa_source_set_volume(u->source, &v, true, true);

Comments

On 21.09.2017 21:26, James Bottomley wrote:
> The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
> names using profile_function instead of randomly made up names.  Fix
> this with the transformation:
>
> PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT -> PA_BLUETOOTH_PROFILE_HSP_HS
> PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY -> PA_BLUETOOTH_PROFILE_HFP_AG
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>
It looks to me like the conversion is not completely correct. In fact,
the native backend (in your notation) currently implements

PA_BLUETOOTH_PROFILE_HSP_HS
PA_BLUETOOTH_PROFILE_HSP_AG

while the ofono backend implements

PA_BLUETOOTH_PROFILE_HFP_HF
PA_BLUETOOTH_PROFILE_HFP_AG

Would it collide with your later patches to distinguish between the
different roles/profiles already in your first patch? As far as I can
see, the real separation of HSP/HFP is mainly done in the native
backend.

Regards
              Georg
On Sun, 2017-09-24 at 15:07 +0200, Georg Chini wrote:
> On 21.09.2017 21:26, James Bottomley wrote:
> > 
> > The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
> > names using profile_function instead of randomly made up
> > names.  Fix
> > this with the transformation:
> > 
> > PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ->
> > PA_BLUETOOTH_PROFILE_HSP_HS
> > PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY ->
> > PA_BLUETOOTH_PROFILE_HFP_AG
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om>
> > 
> It looks to me like the conversion is not completely correct. In
> fact,
> the native backend (in your notation) currently implements
> 
> PA_BLUETOOTH_PROFILE_HSP_HS
> PA_BLUETOOTH_PROFILE_HSP_AG
> 
> while the ofono backend implements
> 
> PA_BLUETOOTH_PROFILE_HFP_HF
> PA_BLUETOOTH_PROFILE_HFP_AG

So your problem is that the current ofono backend actually
uses PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT which gets translated to 
PA_BLUETOOTH_PROFILE_HSP_HS but you think that's a bug in the current
code because it should be HFP instead?

> Would it collide with your later patches to distinguish between the
> different roles/profiles already in your first patch? As far as I can
> see, the real separation of HSP/HFP is mainly done in the native
> backend.

Well, yes, because this patch is designed as an obvious constant rename
which is easy to review.  If I start adding logic changes, it defeats
the purpose of the patch being a simple rename.

Before the patch that follows this, there is no equivalent
of PA_BLUETOOTH_PROFILE_HFP_HF, so I don't really want to introduce one
here before I add the patch that gives it meaning.  I agree this
profile should already have existed in the ofono backend, but it
doesn't.

James
On 25.09.2017 15:31, James Bottomley wrote:
> On Sun, 2017-09-24 at 15:07 +0200, Georg Chini wrote:
>> On 21.09.2017 21:26, James Bottomley wrote:
>>> The PA_BLUETOOTH_PROFILE names should mirror the PA_BLUETOOTH_UUID
>>> names using profile_function instead of randomly made up
>>> names.  Fix
>>> this with the transformation:
>>>
>>> PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ->
>>> PA_BLUETOOTH_PROFILE_HSP_HS
>>> PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY ->
>>> PA_BLUETOOTH_PROFILE_HFP_AG
>>>
>>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
>>> om>
>>>
>> It looks to me like the conversion is not completely correct. In
>> fact,
>> the native backend (in your notation) currently implements
>>
>> PA_BLUETOOTH_PROFILE_HSP_HS
>> PA_BLUETOOTH_PROFILE_HSP_AG
>>
>> while the ofono backend implements
>>
>> PA_BLUETOOTH_PROFILE_HFP_HF
>> PA_BLUETOOTH_PROFILE_HFP_AG
> So your problem is that the current ofono backend actually
> uses PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT which gets translated to
> PA_BLUETOOTH_PROFILE_HSP_HS but you think that's a bug in the current
> code because it should be HFP instead?

I would not call it a bug. It just would be nice when in the end
all the names would be correct. It is a bit strange to have
PA_BLUETOOTH_PROFILE_HSP_HS in the ofono backend when
it actually implements PA_BLUETOOTH_PROFILE_HFP_HF.
The same applies for the AG role and the native backend.
The native backend implements PA_BLUETOOTH_PROFILE_HSP_AG,
not HFP.

>
>> Would it collide with your later patches to distinguish between the
>> different roles/profiles already in your first patch? As far as I can
>> see, the real separation of HSP/HFP is mainly done in the native
>> backend.
> Well, yes, because this patch is designed as an obvious constant rename
> which is easy to review.  If I start adding logic changes, it defeats
> the purpose of the patch being a simple rename.

I understand the purpose to keep it simple. Maybe you could just
split the two AG roles in that patch and add a remark in the commit
message that you will do the separation of the HS/HF role in the
next patch? This would still require some code changes but they
would be minimal.

>
> Before the patch that follows this, there is no equivalent
> of PA_BLUETOOTH_PROFILE_HFP_HF, so I don't really want to introduce one
> here before I add the patch that gives it meaning.  I agree this
> profile should already have existed in the ofono backend, but it
> doesn't.

After introducing PA_BLUETOOTH_PROFILE_HFP_HF, it should then
be also used in the ofono backend.

>
> James
On Mon, 2017-09-25 at 15:49 +0200, Georg Chini wrote:
> On 25.09.2017 15:31, James Bottomley wrote:
> > 
> > On Sun, 2017-09-24 at 15:07 +0200, Georg Chini wrote:
> > > 
> > > On 21.09.2017 21:26, James Bottomley wrote:
> > > > 
> > > > The PA_BLUETOOTH_PROFILE names should mirror the
> > > > PA_BLUETOOTH_UUID names using profile_function instead of
> > > > randomly made up names.  Fix this with the transformation:
> > > > 
> > > > PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT ->
> > > > PA_BLUETOOTH_PROFILE_HSP_HS
> > > > PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY ->
> > > > PA_BLUETOOTH_PROFILE_HFP_AG
> > > > 
> > > > Signed-off-by: James Bottomley
> > > > <James.Bottomley@HansenPartnership.com>
> > > > 
> > > It looks to me like the conversion is not completely correct. In
> > > fact, the native backend (in your notation) currently implements
> > > 
> > > PA_BLUETOOTH_PROFILE_HSP_HS
> > > PA_BLUETOOTH_PROFILE_HSP_AG
> > > 
> > > while the ofono backend implements
> > > 
> > > PA_BLUETOOTH_PROFILE_HFP_HF
> > > PA_BLUETOOTH_PROFILE_HFP_AG
> > So your problem is that the current ofono backend actually
> > uses PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT which gets translated
> > to PA_BLUETOOTH_PROFILE_HSP_HS but you think that's a bug in the
> > current code because it should be HFP instead?
> 
> I would not call it a bug. It just would be nice when in the end
> all the names would be correct. It is a bit strange to have
> PA_BLUETOOTH_PROFILE_HSP_HS in the ofono backend when
> it actually implements PA_BLUETOOTH_PROFILE_HFP_HF.
> The same applies for the AG role and the native backend.
> The native backend implements PA_BLUETOOTH_PROFILE_HSP_AG,
> not HFP.

I really think, if this has to be done, it should be done as a separate
patch to keep this patch simple and obvious.  Even better, why don't
you decide what should happen to ofono and send the patch to the list.
 I can keep it in this series under your authorship if necessary, that
way it would be applied when this series is?

> > > Would it collide with your later patches to distinguish between
> > > the different roles/profiles already in your first patch? As far
> > > as I can see, the real separation of HSP/HFP is mainly done in
> > > the native backend.
> > Well, yes, because this patch is designed as an obvious constant
> > rename which is easy to review.  If I start adding logic changes,
> > it defeats the purpose of the patch being a simple rename.
> 
> I understand the purpose to keep it simple. Maybe you could just
> split the two AG roles in that patch and add a remark in the commit
> message that you will do the separation of the HS/HF role in the
> next patch? This would still require some code changes but they
> would be minimal.
>  
> > Before the patch that follows this, there is no equivalent
> > of PA_BLUETOOTH_PROFILE_HFP_HF, so I don't really want to introduce
> > one here before I add the patch that gives it meaning.  I agree
> > this profile should already have existed in the ofono backend, but
> > it doesn't.
> 
> After introducing PA_BLUETOOTH_PROFILE_HFP_HF, it should then
> be also used in the ofono backend.

So right at the moment, I believe from the logic that ofono will
continue to work as it does today even with these patches applied.  My
problem with making any changes to the ofono backend is that I've never
been able to get it working, so I have no means of testing it.  Since
you have it all working, could you test out the changes you want and
send them yourself?  That way there'd be no cockups.

Regards,

James