On 02/19/2013 10:33 AM, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>
> Avoid using strings only to represent form factors in the bluetooth-util
> API and instead use a new dedicated enum type: pa_bt_form_factor_t.
Everything looks good on this one IMO.
> ---
> src/modules/bluetooth/bluetooth-util.c | 53 ++++++++++++++++++-------
> src/modules/bluetooth/bluetooth-util.h | 15 ++++++-
> src/modules/bluetooth/module-bluetooth-device.c | 8 ++--
> 3 files changed, 58 insertions(+), 18 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index 688fee0..faf1c41 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -1798,26 +1798,26 @@ pa_hook* pa_bluetooth_discovery_hook(pa_bluetooth_discovery *y, pa_bluetooth_hoo
> return &y->hooks[hook];
> }
>
> -const char*pa_bluetooth_get_form_factor(uint32_t class) {
> +pa_bt_form_factor_t pa_bluetooth_get_form_factor(uint32_t class) {
> unsigned i;
> - const char *r;
> -
> - static const char * const table[] = {
> - [1] = "headset",
> - [2] = "hands-free",
> - [4] = "microphone",
> - [5] = "speaker",
> - [6] = "headphone",
> - [7] = "portable",
> - [8] = "car",
> - [10] = "hifi"
> + pa_bt_form_factor_t r;
> +
> + static const pa_bt_form_factor_t table[] = {
> + [1] = PA_BT_FORM_FACTOR_HEADSET,
> + [2] = PA_BT_FORM_FACTOR_HANDSFREE,
> + [4] = PA_BT_FORM_FACTOR_MICROPHONE,
> + [5] = PA_BT_FORM_FACTOR_SPEAKER,
> + [6] = PA_BT_FORM_FACTOR_HEADPHONE,
> + [7] = PA_BT_FORM_FACTOR_PORTABLE,
> + [8] = PA_BT_FORM_FACTOR_CAR,
> + [10] = PA_BT_FORM_FACTOR_HIFI
> };
>
> if (((class >> 8) & 31) != 4)
> - return NULL;
> + return PA_BT_FORM_FACTOR_UNKNOWN;
>
> if ((i = (class >> 2) & 63) >= PA_ELEMENTSOF(table))
> - r = NULL;
> + r = PA_BT_FORM_FACTOR_UNKNOWN;
> else
> r = table[i];
>
> @@ -1827,6 +1827,31 @@ const char*pa_bluetooth_get_form_factor(uint32_t class) {
> return r;
> }
>
> +const char *pa_bt_form_factor_to_string(pa_bt_form_factor_t ff) {
> + switch (ff) {
> + case PA_BT_FORM_FACTOR_UNKNOWN:
> + return "unknown";
> + case PA_BT_FORM_FACTOR_HEADSET:
> + return "headset";
> + case PA_BT_FORM_FACTOR_HANDSFREE:
> + return "hands-free";
> + case PA_BT_FORM_FACTOR_MICROPHONE:
> + return "microphone";
> + case PA_BT_FORM_FACTOR_SPEAKER:
> + return "speaker";
> + case PA_BT_FORM_FACTOR_HEADPHONE:
> + return "headphone";
> + case PA_BT_FORM_FACTOR_PORTABLE:
> + return "portable";
> + case PA_BT_FORM_FACTOR_CAR:
> + return "car";
> + case PA_BT_FORM_FACTOR_HIFI:
> + return "hifi";
> + }
> +
> + pa_assert_not_reached();
> +}
> +
> char *pa_bluetooth_cleanup_name(const char *name) {
> char *t, *s, *d;
> bool space = false;
> diff --git a/src/modules/bluetooth/bluetooth-util.h b/src/modules/bluetooth/bluetooth-util.h
> index 6423f88..b59255e 100644
> --- a/src/modules/bluetooth/bluetooth-util.h
> +++ b/src/modules/bluetooth/bluetooth-util.h
> @@ -154,7 +154,20 @@ void pa_bluetooth_transport_set_speaker_gain(pa_bluetooth_transport *t, uint16_t
>
> pa_hook* pa_bluetooth_discovery_hook(pa_bluetooth_discovery *y, pa_bluetooth_hook_t hook);
>
> -const char* pa_bluetooth_get_form_factor(uint32_t class);
> +typedef enum pa_bt_form_factor {
> + PA_BT_FORM_FACTOR_UNKNOWN,
> + PA_BT_FORM_FACTOR_HEADSET,
> + PA_BT_FORM_FACTOR_HANDSFREE,
> + PA_BT_FORM_FACTOR_MICROPHONE,
> + PA_BT_FORM_FACTOR_SPEAKER,
> + PA_BT_FORM_FACTOR_HEADPHONE,
> + PA_BT_FORM_FACTOR_PORTABLE,
> + PA_BT_FORM_FACTOR_CAR,
> + PA_BT_FORM_FACTOR_HIFI,
> +} pa_bt_form_factor_t;
> +
> +pa_bt_form_factor_t pa_bluetooth_get_form_factor(uint32_t class);
> +const char *pa_bt_form_factor_to_string(pa_bt_form_factor_t ff);
>
> char *pa_bluetooth_cleanup_name(const char *name);
>
> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
> index e04780b..9dc0cb3 100644
> --- a/src/modules/bluetooth/module-bluetooth-device.c
> +++ b/src/modules/bluetooth/module-bluetooth-device.c
> @@ -2147,7 +2147,7 @@ static int add_card(struct userdata *u) {
> bool b;
> pa_card_profile *p;
> enum profile *d;
> - const char *ff;
> + pa_bt_form_factor_t ff;
> char *n;
> const char *default_profile;
> const pa_bluetooth_device *device = u->device;
> @@ -2167,8 +2167,10 @@ static int add_card(struct userdata *u) {
> pa_proplist_sets(data.proplist, PA_PROP_DEVICE_API, "bluez");
> pa_proplist_sets(data.proplist, PA_PROP_DEVICE_CLASS, "sound");
> pa_proplist_sets(data.proplist, PA_PROP_DEVICE_BUS, "bluetooth");
> - if ((ff = pa_bluetooth_get_form_factor(device->class)))
> - pa_proplist_sets(data.proplist, PA_PROP_DEVICE_FORM_FACTOR, ff);
> +
> + if ((ff = pa_bluetooth_get_form_factor(device->class)) != PA_BT_FORM_FACTOR_UNKNOWN)
> + pa_proplist_sets(data.proplist, PA_PROP_DEVICE_FORM_FACTOR, pa_bt_form_factor_to_string(ff));
> +
> pa_proplist_sets(data.proplist, "bluez.path", device->path);
> pa_proplist_setf(data.proplist, "bluez.class", "0x%06x", (unsigned) device->class);
> pa_proplist_sets(data.proplist, "bluez.name", device->name);
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
