Re: [PATCH BlueZ 2/2] gatt-database : Add support for Included service

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi avichal,

On Thu, Mar 15, 2018 at 4:45 PM, avichal <avichal.a@xxxxxxxxxxx> wrote:
>
> Added support of included services in gatt server
> While registring the service new property is added
> "includes"
>
>         array{object} type  [read-only]
>         Array of object paths representing the included
>         services of this service.
>
> Signed-off-by: avichal <avichal.a@xxxxxxxxxxx>

We do not use Signed-off-by in userspace.

> ---
>  src/gatt-database.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 9a33ae7..b2e93ce 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -109,6 +109,7 @@ struct external_service {
>         uint16_t attr_cnt;
>         struct queue *chrcs;
>         struct queue *descs;
> +       struct queue *includes;
>  };
>
>  struct external_profile {
> @@ -444,12 +445,21 @@ static void desc_free(void *data)
>         free(desc);
>  }
>
> +static void inc_free(void *data)
> +{
> +       struct external_desc *inc = data;
> +
> +       free(inc);
> +}
> +

Extra line empty, please remove it.

>  static void service_free(void *data)
>  {
>         struct external_service *service = data;
>
>         queue_destroy(service->chrcs, chrc_free);
>         queue_destroy(service->descs, desc_free);
> +       queue_destroy(service->includes,inc_free );
>
>         if (service->attrib)
>                 gatt_db_remove_service(service->app->database->db,
> @@ -1533,6 +1543,7 @@ static struct external_service *create_service(struct gatt_app *app,
>         service->proxy = g_dbus_proxy_ref(proxy);
>         service->chrcs = queue_new();
>         service->descs = queue_new();
> +       service->includes = queue_new();
>
>         /* Add 1 for the service declaration */
>         if (!incr_attr_count(service, 1)) {
> @@ -1616,6 +1627,44 @@ static bool parse_uuid(GDBusProxy *proxy, bt_uuid_t *uuid)
>         return true;
>  }
>
> +
> +static bool parse_includes(GDBusProxy *proxy, struct external_service * service)
> +{
> +       DBusMessageIter iter;
> +       DBusMessageIter array;
> +       char *obj;
> +
> +       if (!g_dbus_proxy_get_property(proxy, "Includes", &iter))
> +               return false;
> +
> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY)
> +               return false;
> +
> +       dbus_message_iter_recurse(&iter, &array);
> +

Ditto.

> +       do {
> +               if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_OBJECT_PATH)
> +                       return false;
> +
> +               dbus_message_iter_get_basic(&array, &obj);
> +
> +               if(queue_push_tail(service->includes,obj)){
> +                       DBG("Included paths added\n");
> +                       incr_attr_count(service ,1);
> +               }
> +               else{
> +                       error("Failed to add Includes path in que\n");
> +                       return FALSE;
> +               }

Always use a space after if statement, e.g if (cond), also the else
should be in the same like as in the closing }, like in } else {.

> +
> +       } while (dbus_message_iter_next(&array));
> +
> +
> +       return true;
> +}
> +
> +
>  static bool parse_primary(GDBusProxy *proxy, bool *primary)
>  {
>         DBusMessageIter iter;
> @@ -2488,6 +2537,46 @@ fail:
>         gatt_db_attribute_write_result(attrib, id, BT_ATT_ERROR_UNLIKELY);
>  }
>

Again, please remove extra empty lines.

> +
> +static void include_services(void * data ,void *userdata)
> +{
> +       char *obj = (char*)data;
> +       struct external_service * service = (struct external_service *)userdata;
> +       struct gatt_db_attribute *attrib=NULL;
> +       struct external_service  *service_inc=NULL;
> +
> +       DBG("object path recieved %s,",obj);
> +
> +       service_inc = queue_find(service->app->services, match_service_by_path, obj);
> +       if(service_inc) {
> +
> +               if(!(service)->attrib)
> +                       error("service attibute not found\n");
> +

Ditto.

> +               if(!service_inc->attrib)
> +                       error("include attribute not found\n");
> +

Ditto.

> +               attrib = gatt_db_service_add_included((service)->attrib ,service_inc ->attrib);
> +               if(attrib == NULL)
> +                       error("include service attributes failed \n");
> +
> +               else
> +                       (service)->attrib = attrib;
> +
> +       }
> +       else
> +               error("include service not found\n");
> +
> +}
> +
> +static void database_add_includes(struct external_service *service)
> +{
> +       queue_foreach(service->includes,include_services,service );
> +}
> +
>  static bool database_add_chrc(struct external_service *service,
>                                                 struct external_chrc *chrc)
>  {
> @@ -2560,11 +2649,20 @@ static bool database_add_service(struct external_service *service)
>                 return false;
>         }
>

Ditto,

> +       if(!parse_includes(service->proxy,service)){
> +               error("Failed to read \"Includes\" property of service");
> +       }
> +
>         service->attrib = gatt_db_add_service(service->app->database->db, &uuid,
> -                                               primary, service->attr_cnt);
> +                                       primary, service->attr_cnt);
>         if (!service->attrib)
>                 return false;
>
> +       database_add_includes(service);
> +       DBG("ATTR count %d\n",service->attr_cnt);
> +

Ditto.

>         entry = queue_get_entries(service->chrcs);
>         while (entry) {
>                 struct external_chrc *chrc = entry->data;
> --
> 2.7.4

There are probably more code style problems that I miss, but you can
prevent this errors by having checkpatch.pl on your pre-commit and
pre-applypatch hooks like this:

exec git diff --cached | /pathto/checkpatch.pl -q --no-signoff
--ignore INITIALISED_STATIC,GLOBAL_INITIALISERS,PREFER_PACKED,SPACING,FSF_MAILING_ADDRESS,TRAILING_STATEMENTS,RETURN_VOID,,SPLIT_STRING,OPEN_BRACE,STATIC_CONST_CHAR_ARRAY,LINE_SPACING
 --show-types -

-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux