Hi Daniel & Mikel,

On 10/13/2011 09:05 AM, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
> 
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
> ---
>  plugins/hfp_hf.c |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 100 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/hfp_hf.c b/plugins/hfp_hf.c
> index 1008696..d81cae9 100644
> --- a/plugins/hfp_hf.c
> +++ b/plugins/hfp_hf.c
> @@ -38,6 +38,7 @@
>  #include <ofono/plugin.h>
>  #include <ofono/log.h>
>  #include <ofono/modem.h>
> +#include <ofono/devinfo.h>
>  #include <ofono/netreg.h>
>  #include <ofono/voicecall.h>
>  #include <ofono/call-volume.h>
> @@ -46,6 +47,7 @@
>  #include <drivers/hfpmodem/slc.h>
>  
>  #include "bluetooth.h"
> +#include "drivers/atmodem/atutil.h"
>  
>  #define	BLUEZ_GATEWAY_INTERFACE		BLUEZ_SERVICE ".HandsfreeGateway"
>  
> @@ -62,10 +64,15 @@ static GHashTable *modem_hash = NULL;
>  struct hfp_data {
>  	struct hfp_slc_info info;
>  	char *handsfree_path;
> +	char *device_address;
>  	DBusMessage *slc_msg;
>  	gboolean agent_registered;
>  };
>  
> +struct devinfo_data {
> +	char *device_address;
> +};
> +

Can we actually move the devinfo implementation into drivers/hfpmodem?

>  static void hfp_debug(const char *str, void *user_data)
>  {
>  	const char *prefix = user_data;
> @@ -206,6 +213,7 @@ static int hfp_hf_probe(const char *device, const char *dev_addr,
>  	struct ofono_modem *modem;
>  	struct hfp_data *data;
>  	char buf[256];
> +	struct ofono_devinfo *info;
>  
>  	/* We already have this device in our hash, ignore */
>  	if (g_hash_table_lookup(modem_hash, device) != NULL)
> @@ -229,15 +237,27 @@ static int hfp_hf_probe(const char *device, const char *dev_addr,
>  	if (data->handsfree_path == NULL)
>  		goto free;
>  
> +	data->device_address = g_strdup(dev_addr);
> +	if (data->device_address == NULL)
> +		goto free;
> +
>  	ofono_modem_set_data(modem, data);
>  	ofono_modem_set_name(modem, alias);
>  	ofono_modem_register(modem);
>  
>  	g_hash_table_insert(modem_hash, g_strdup(device), modem);
>  
> +	info = ofono_devinfo_create(modem, 0, "hfp", data);
> +	ofono_devinfo_register(info);
> +

The _create step should be done in pre_sim, and you might want to let
the driver register itself, just like every other atom driver.

>  	return 0;
>  
>  free:
> +	if (data != NULL) {
> +		g_free(data->handsfree_path);
> +		g_free(data->device_address);
> +	}
> +
>  	g_free(data);
>  	ofono_modem_remove(modem);
>  
> @@ -354,6 +374,7 @@ static void hfp_remove(struct ofono_modem *modem)
>  
>  	g_hash_table_remove(modem_hash, data->handsfree_path);
>  
> +	g_free(data->device_address);
>  	g_free(data->handsfree_path);
>  	g_free(data);
>  
> @@ -476,6 +497,67 @@ static void hfp_post_sim(struct ofono_modem *modem)
>  	DBG("%p", modem);
>  }
>  
> +static void hfp_query_manufacturer(struct ofono_devinfo *info,
> +					ofono_devinfo_query_cb_t cb,
> +					void *data)
> +{
> +	CALLBACK_WITH_FAILURE(cb, "", data);
> +}
> +
> +static void hfp_query_model(struct ofono_devinfo *info,
> +				ofono_devinfo_query_cb_t cb,
> +				void *data)
> +{
> +	CALLBACK_WITH_FAILURE(cb, "", data);
> +}
> +
> +static void hfp_query_revision(struct ofono_devinfo *info,
> +				ofono_devinfo_query_cb_t cb,
> +				void *data)
> +{
> +	CALLBACK_WITH_FAILURE(cb, "", data);
> +}
> +

The devinfo atom should handle empty implementations of query_*
functions, so these should not be necessary.

> +static void hfp_query_serial(struct ofono_devinfo *info,
> +				ofono_devinfo_query_cb_t cb,
> +				void *data)
> +{
> +	struct devinfo_data *dev = ofono_devinfo_get_data(info);
> +	CALLBACK_WITH_SUCCESS(cb, dev->device_address, data);
> +}
> +
> +static int hfp_devinfo_probe(struct ofono_devinfo *info, unsigned int vendor,
> +				void *user)
> +{
> +	struct hfp_data *hfp_data = user;
> +
> +	struct devinfo_data *devinfo_data = g_try_new0(struct devinfo_data, 1);
> +	if (devinfo_data == NULL)
> +		return -ENOMEM;
> +
> +	devinfo_data->device_address = g_strdup(hfp_data->device_address);
> +	if (devinfo_data->device_address == NULL) {
> +		g_free(devinfo_data);
> +		return -ENOMEM;
> +	}
> +
> +	ofono_devinfo_set_data(info, devinfo_data);
> +
> +	return 0;
> +}
> +
> +static void hfp_devinfo_remove(struct ofono_devinfo *info)
> +{
> +	struct devinfo_data *data = ofono_devinfo_get_data(info);
> +
> +	ofono_devinfo_set_data(info, NULL);
> +	if (data == NULL)
> +		return;
> +
> +	g_free(data->device_address);
> +	g_free(data);
> +}
> +
>  static struct ofono_modem_driver hfp_driver = {
>  	.name		= "hfp",
>  	.probe		= hfp_probe,
> @@ -486,6 +568,16 @@ static struct ofono_modem_driver hfp_driver = {
>  	.post_sim	= hfp_post_sim,
>  };
>  
> +static struct ofono_devinfo_driver hfp_devinfo_driver = {
> +	.name			= "hfp",
> +	.probe			= hfp_devinfo_probe,
> +	.remove			= hfp_devinfo_remove,
> +	.query_manufacturer	= hfp_query_manufacturer,
> +	.query_model		= hfp_query_model,
> +	.query_revision		= hfp_query_revision,
> +	.query_serial		= hfp_query_serial
> +};
> +
>  static struct bluetooth_profile hfp_hf = {
>  	.name		= "hfp_hf",
>  	.probe		= hfp_hf_probe,
> @@ -506,8 +598,15 @@ static int hfp_init(void)
>  	if (err < 0)
>  		return err;
>  
> +	err = ofono_devinfo_driver_register(&hfp_devinfo_driver);
> +	if (err < 0) {
> +		ofono_modem_driver_unregister(&hfp_driver);
> +		return err;
> +	}
> +

Can we do this inside drivers/hfpmodem?

>  	err = bluetooth_register_uuid(HFP_AG_UUID, &hfp_hf);
>  	if (err < 0) {
> +		ofono_devinfo_driver_unregister(&hfp_devinfo_driver);
>  		ofono_modem_driver_unregister(&hfp_driver);
>  		return err;
>  	}
> @@ -521,6 +620,7 @@ static int hfp_init(void)
>  static void hfp_exit(void)
>  {
>  	bluetooth_unregister_uuid(HFP_AG_UUID);
> +	ofono_devinfo_driver_unregister(&hfp_devinfo_driver);
>  	ofono_modem_driver_unregister(&hfp_driver);
>  
>  	g_hash_table_destroy(modem_hash);

Regards,
-Denis