[pulseaudio-discuss] [PATCH 1/3] backend-native: add support for the HSP Headset role

[Tanu Kaskinen]

On Thu, 2017-03-02 at 17:04 +0100, Georg Chini wrote:
>  #define HSP_AG_PROFILE "/Profile/HSPAGProfile"
> +#define HSP_HS_PROFILE "/Profile/HSPHSProfile"
> +
> +/* RFCOMM channel for HSP headset role */
> +#define HSP_HS_DEFAULT_CHANNEL  3

The comment still doesn't explain why 3 is a good default.

I realize you might not know, since this was originally written by Wim.
I added Wim to Cc -- Wim, can you explain why 3 is the best channel? It
looks like a deliberate choice.

> @@ -249,22 +373,38 @@ static void rfcomm_io_callback(pa_mainloop_api *io, pa_io_event *e, int fd, pa_i
>          buf[len] = 0;
>          pa_log_debug("RFCOMM << %s", buf);
>  
> -        if (sscanf(buf, "AT+VGS=%d", &gain) == 1) {
> -          t->speaker_gain = gain;
> -          pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery, PA_BLUETOOTH_HOOK_TRANSPORT_SPEAKER_GAIN_CHANGED), t);
> -        } else if (sscanf(buf, "AT+VGM=%d", &gain) == 1) {
> -          t->microphone_gain = gain;
> -          pa_hook_fire(pa_bluetooth_discovery_hook(t->device->discovery, PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED), t);
> +        /* There are only four HSP AT commands:
> +         * AT+VGS=value: value between 0 and 15, sets the speaker gain in AG role or notifies the
> +         * AG that the speaker gain has changed in HS role.
> +         * AT+VGM=value: value between 0 and 15, sets the microphone gain in AG role or notifies
> +         * the AG that the microphone gain has changed.

I think +VGS/+VGM should be explained separately from AT+VGS/AT+VGM.

> @@ -295,17 +440,25 @@ static void set_speaker_gain(pa_bluetooth_transport *t, uint16_t gain) {
>  
>      t->speaker_gain = gain;
>  
> -    len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
> -    pa_log_debug("RFCOMM >> +VGS=%d", 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) {
> +        len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
> +        pa_log_debug("RFCOMM >> +VGS=%d", gain);
> +    } else {
> +        len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
> +        pa_log_debug("RFCOMM >> +VGM=%d", gain);

AT+VGM should be used here.

> @@ -314,10 +467,18 @@ static void set_microphone_gain(pa_bluetooth_transport *t, uint16_t gain) {
>  
>      t->microphone_gain = gain;
>  
> -    len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
> -    pa_log_debug("RFCOMM >> +VGM=%d", 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) {
> +        len = sprintf(buf, "\r\n+VGM=%d\r\n", gain);
> +        pa_log_debug("RFCOMM >> +VGM=%d", gain);
> +    } else {
> +        len = sprintf(buf, "\r\n+VGS=%d\r\n", gain);
> +        pa_log_debug("RFCOMM >> +VGS=%d", gain);

AT+VGS should be used here.

-- 
Tanu

https://www.patreon.com/tanuk