* [PATCH] gatt-database: Return meaningful ecodes for ccc write
@ 2015-08-10 12:37 Gowtham Anandha Babu
2015-08-11 8:31 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Gowtham Anandha Babu @ 2015-08-10 12:37 UTC (permalink / raw)
To: linux-bluetooth; +Cc: bharat.panda, Gowtham Anandha Babu
Removed generic ATT protocol error codes and added
Common Profile and Service Error Codes.
---
src/gatt-database.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/gatt-database.c b/src/gatt-database.c
index 69a814d..defe329 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -1783,22 +1783,16 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
return 0;
}
- /*
- * TODO: All of the errors below should fall into the so called
- * "Application Error" range. Since there is no well defined error for
- * these, we return a generic ATT protocol error for now.
- */
-
if (chrc->ntfy_cnt == UINT_MAX) {
/* Maximum number of per-device CCC descriptors configured */
- return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+ return BT_ERROR_OUT_OF_RANGE;
}
/* Don't support undefined CCC values yet */
if (value > 2 ||
(value == 1 && !(chrc->props & BT_GATT_CHRC_PROP_NOTIFY)) ||
(value == 2 && !(chrc->props & BT_GATT_CHRC_PROP_INDICATE)))
- return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+ return BT_ERROR_CCC_IMPROPERLY_CONFIGURED;
/*
* Always call StartNotify for an incoming enable and ignore the return
@@ -1807,7 +1801,7 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
if (g_dbus_proxy_method_call(chrc->proxy,
"StartNotify", NULL, NULL,
NULL, NULL) == FALSE)
- return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
+ return BT_ERROR_ALREADY_IN_PROGRESS;
__sync_fetch_and_add(&chrc->ntfy_cnt, 1);
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] gatt-database: Return meaningful ecodes for ccc write
2015-08-10 12:37 [PATCH] gatt-database: Return meaningful ecodes for ccc write Gowtham Anandha Babu
@ 2015-08-11 8:31 ` Luiz Augusto von Dentz
2015-08-11 8:45 ` Gowtham Anandha Babu
0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2015-08-11 8:31 UTC (permalink / raw)
To: Gowtham Anandha Babu; +Cc: linux-bluetooth@vger.kernel.org, Bharat Panda
Hi Gowtham,
On Mon, Aug 10, 2015 at 3:37 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Removed generic ATT protocol error codes and added
> Common Profile and Service Error Codes.
> ---
> src/gatt-database.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 69a814d..defe329 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -1783,22 +1783,16 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
> return 0;
> }
>
> - /*
> - * TODO: All of the errors below should fall into the so called
> - * "Application Error" range. Since there is no well defined error for
> - * these, we return a generic ATT protocol error for now.
> - */
> -
> if (chrc->ntfy_cnt == UINT_MAX) {
> /* Maximum number of per-device CCC descriptors configured */
> - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> + return BT_ERROR_OUT_OF_RANGE;
This one above Im not complete sure it is the proper error to return,
I would consider BT_ATT_ERROR_INSUFFICIENT_RESOURCES more appropriated
here.
> }
>
> /* Don't support undefined CCC values yet */
> if (value > 2 ||
> (value == 1 && !(chrc->props & BT_GATT_CHRC_PROP_NOTIFY)) ||
> (value == 2 && !(chrc->props & BT_GATT_CHRC_PROP_INDICATE)))
> - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> + return BT_ERROR_CCC_IMPROPERLY_CONFIGURED;
>
> /*
> * Always call StartNotify for an incoming enable and ignore the return
> @@ -1807,7 +1801,7 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
> if (g_dbus_proxy_method_call(chrc->proxy,
> "StartNotify", NULL, NULL,
> NULL, NULL) == FALSE)
> - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> + return BT_ERROR_ALREADY_IN_PROGRESS;
This one should probably return BT_ATT_ERROR_UNLIKELY instead,
BT_ERROR_ALREADY_IN_PROGRESS shall only be used if there is a request
ongoing which is not the case here.
> __sync_fetch_and_add(&chrc->ntfy_cnt, 1);
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] gatt-database: Return meaningful ecodes for ccc write
2015-08-11 8:31 ` Luiz Augusto von Dentz
@ 2015-08-11 8:45 ` Gowtham Anandha Babu
0 siblings, 0 replies; 3+ messages in thread
From: Gowtham Anandha Babu @ 2015-08-11 8:45 UTC (permalink / raw)
To: 'Luiz Augusto von Dentz'; +Cc: linux-bluetooth, 'Bharat Panda'
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Tuesday, August 11, 2015 2:02 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; Bharat Panda
> Subject: Re: [PATCH] gatt-database: Return meaningful ecodes for ccc write
>
> Hi Gowtham,
>
> On Mon, Aug 10, 2015 at 3:37 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Removed generic ATT protocol error codes and added Common Profile and
> > Service Error Codes.
> > ---
> > src/gatt-database.c | 12 +++---------
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/gatt-database.c b/src/gatt-database.c index
> > 69a814d..defe329 100644
> > --- a/src/gatt-database.c
> > +++ b/src/gatt-database.c
> > @@ -1783,22 +1783,16 @@ static uint8_t ccc_write_cb(uint16_t value, void
> *user_data)
> > return 0;
> > }
> >
> > - /*
> > - * TODO: All of the errors below should fall into the so called
> > - * "Application Error" range. Since there is no well defined error for
> > - * these, we return a generic ATT protocol error for now.
> > - */
> > -
> > if (chrc->ntfy_cnt == UINT_MAX) {
> > /* Maximum number of per-device CCC descriptors configured */
> > - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> > + return BT_ERROR_OUT_OF_RANGE;
>
> This one above Im not complete sure it is the proper error to return, I would
> consider BT_ATT_ERROR_INSUFFICIENT_RESOURCES more appropriated
> here.
>
> > }
> >
> > /* Don't support undefined CCC values yet */
> > if (value > 2 ||
> > (value == 1 && !(chrc->props & BT_GATT_CHRC_PROP_NOTIFY)) ||
> > (value == 2 && !(chrc->props &
> BT_GATT_CHRC_PROP_INDICATE)))
> > - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> > + return BT_ERROR_CCC_IMPROPERLY_CONFIGURED;
> >
> > /*
> > * Always call StartNotify for an incoming enable and ignore
> > the return @@ -1807,7 +1801,7 @@ static uint8_t ccc_write_cb(uint16_t
> value, void *user_data)
> > if (g_dbus_proxy_method_call(chrc->proxy,
> > "StartNotify", NULL, NULL,
> > NULL, NULL) == FALSE)
> > - return BT_ATT_ERROR_REQUEST_NOT_SUPPORTED;
> > + return BT_ERROR_ALREADY_IN_PROGRESS;
>
> This one should probably return BT_ATT_ERROR_UNLIKELY instead,
> BT_ERROR_ALREADY_IN_PROGRESS shall only be used if there is a request
> ongoing which is not the case here.
>
I have changed the error codes and sent v1 for the same. Please have a look at it.
> > __sync_fetch_and_add(&chrc->ntfy_cnt, 1);
> >
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-08-11 8:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-10 12:37 [PATCH] gatt-database: Return meaningful ecodes for ccc write Gowtham Anandha Babu
2015-08-11 8:31 ` Luiz Augusto von Dentz
2015-08-11 8:45 ` Gowtham Anandha Babu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.