[PATCH 1/2] android/gatt: Refactor characteristic notify function

[PATCH 1/2] android/gatt: Refactor characteristic notify function

[Jakub Tyszkowski]

This removes redundant status from function parameter list, as sent
status is different than success only when there is no characteristic.
---
 android/gatt.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index d7c7c93..cad7e1a 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -951,13 +951,12 @@ static void handle_client_get_included_service(const void *buf, uint16_t len)
 
 static void send_client_char_notify(const struct characteristic *ch,
 					int32_t conn_id,
-					const struct service *service,
-					uint8_t status)
+					const struct service *service)
 {
 	struct hal_ev_gatt_client_get_characteristic ev;
 
 	memset(&ev, 0, sizeof(ev));
-	ev.status = status;
+	ev.status = ch ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;
 
 	if (ch) {
 		ev.char_prop = ch->ch.properties;
@@ -1026,11 +1025,9 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
 
 	if (!queue_isempty(data->service->chars))
 		send_client_char_notify(queue_peek_head(data->service->chars),
-						data->conn_id, data->service,
-						HAL_STATUS_SUCCESS);
+						data->conn_id, data->service);
 	else
-		send_client_char_notify(NULL, data->conn_id, data->service,
-							HAL_STATUS_FAILED);
+		send_client_char_notify(NULL, data->conn_id, data->service);
 
 	free(data);
 }
@@ -1126,12 +1123,7 @@ static void handle_client_get_characteristic(const void *buf, uint16_t len)
 	else
 		ch = queue_peek_head(srvc->chars);
 
-	if (ch)
-		send_client_char_notify(ch, dev->conn_id, srvc,
-							HAL_STATUS_SUCCESS);
-	else
-		send_client_char_notify(NULL, dev->conn_id, srvc,
-							HAL_STATUS_FAILED);
+	send_client_char_notify(ch, dev->conn_id, srvc);
 
 	status = HAL_STATUS_SUCCESS;
 
-- 
1.9.0

[PATCH 2/2] android/gatt: Remove redundant queue check

[Jakub Tyszkowski]

It is safe to just call queue_peak_head on empty queue. Now its also
safe to pass NULL to send_client_char_notify as status will be set
inside it.
---
 android/gatt.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/android/gatt.c b/android/gatt.c
index cad7e1a..dcc533c 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -1023,11 +1023,8 @@ static void discover_char_cb(uint8_t status, GSList *characteristics,
 
 	cache_all_srvc_chars(characteristics, data->service->chars);
 
-	if (!queue_isempty(data->service->chars))
-		send_client_char_notify(queue_peek_head(data->service->chars),
-						data->conn_id, data->service);
-	else
-		send_client_char_notify(NULL, data->conn_id, data->service);
+	send_client_char_notify(queue_peek_head(data->service->chars),
+	 					data->conn_id, data->service);
 
 	free(data);
 }
-- 
1.9.0

Re: [PATCH 1/2] android/gatt: Refactor characteristic notify function

[Szymon Janc]

Hi Jakub,

On Wednesday 26 March 2014 19:23:35 Jakub Tyszkowski wrote:
> This removes redundant status from function parameter list, as sent
> status is different than success only when there is no characteristic.
> ---
>  android/gatt.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/android/gatt.c b/android/gatt.c
> index d7c7c93..cad7e1a 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -951,13 +951,12 @@ static void handle_client_get_included_service(const
> void *buf, uint16_t len)
> 
>  static void send_client_char_notify(const struct characteristic *ch,
>  					int32_t conn_id,
> -					const struct service *service,
> -					uint8_t status)
> +					const struct service *service)
>  {
>  	struct hal_ev_gatt_client_get_characteristic ev;
> 
>  	memset(&ev, 0, sizeof(ev));
> -	ev.status = status;
> +	ev.status = ch ? HAL_STATUS_SUCCESS : HAL_STATUS_FAILED;
> 
>  	if (ch) {
>  		ev.char_prop = ch->ch.properties;
> @@ -1026,11 +1025,9 @@ static void discover_char_cb(uint8_t status, GSList
> *characteristics,
> 
>  	if (!queue_isempty(data->service->chars))
>  		send_client_char_notify(queue_peek_head(data->service->chars),
> -						data->conn_id, data->service,
> -						HAL_STATUS_SUCCESS);
> +						data->conn_id, data->service);
>  	else
> -		send_client_char_notify(NULL, data->conn_id, data->service,
> -							HAL_STATUS_FAILED);
> +		send_client_char_notify(NULL, data->conn_id, data->service);
> 
>  	free(data);
>  }
> @@ -1126,12 +1123,7 @@ static void handle_client_get_characteristic(const
> void *buf, uint16_t len) else
>  		ch = queue_peek_head(srvc->chars);
> 
> -	if (ch)
> -		send_client_char_notify(ch, dev->conn_id, srvc,
> -							HAL_STATUS_SUCCESS);
> -	else
> -		send_client_char_notify(NULL, dev->conn_id, srvc,
> -							HAL_STATUS_FAILED);
> +	send_client_char_notify(ch, dev->conn_id, srvc);
> 
>  	status = HAL_STATUS_SUCCESS;

Both patches applied. Thanks.
Although I had to manually fix whitespace issue:

.git/rebase-apply/patch:19: space before tab in indent.
                                                data->conn_id, data->service);
fatal: 1 line adds whitespace errors.

so please pay attention for this in future.

-- 
Szymon K. Janc
szymon.janc@gmail.com