Skip to content

Maybe fix for a couple of bugs in write descriptors: #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kwikius wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Maybe fix for a couple of bugs in write descriptors: #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions bluez/gattlib_read_write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ int gattlib_write_char_by_uuid(gatt_connection_t* connection, uuid_t* uuid, cons
fprintf(stderr, "Fail to find handle for UUID.\n");
return ret;
}

return gattlib_write_char_by_handle(connection, handle, buffer, sizeof(buffer));
return gattlib_write_char_by_handle(connection, handle, buffer, buffer_len);
}

int gattlib_notification_start(gatt_connection_t* connection, const uuid_t* uuid) {
Expand Down
10 changes: 6 additions & 4 deletions examples/read_write/read_write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include <assert.h>
#include <stdio.h>
#include <stdlib.h>

#include "gattlib.h"

typedef enum { READ, WRITE} operation_t;
Expand Down Expand Up @@ -53,7 +52,7 @@ int main(int argc, char *argv[]) {
} else if ((strcmp(argv[2], "write") == 0) && (argc == 5)) {
g_operation = WRITE;

if ((strlen(argv[4]) >= 2) && (argv[4][0] == '0') && (argv[4][0] == 'x')) {
if ((strlen(argv[4]) >= 2) && (argv[4][0] == '0') && ((argv[4][1] == 'x') || (argv[4][1] == 'X'))) {

@oliviermartin oliviermartin Oct 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding this issue. But would actually in case of hexadecimal we should reverse the condition such as:

if ((strlen(argv[4]) >= 2) && (argv[4][0] == '0') && ((argv[4][1] == 'x') || (argv[4][1] == 'X'))) {
	value_data = strtol(argv[4], NULL, 16);
} else {
	value_data = strtol(argv[4], NULL, 0);
}

@kwikius kwikius Oct 18, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

value_data = strtol(argv[4], NULL, 0);
} else {
value_data = strtol(argv[4], NULL, 16);
Expand All @@ -69,7 +68,7 @@ int main(int argc, char *argv[]) {
return 1;
}

connection = gattlib_connect(NULL, argv[1], BDADDR_LE_PUBLIC, BT_SEC_LOW, 0, 0);
connection = gattlib_connect(NULL, argv[1], BDADDR_LE_RANDOM, BT_SEC_LOW, 0, 0);
if (connection == NULL) {
fprintf(stderr, "Fail to connect to the bluetooth device.\n");
return 1;
Expand All @@ -85,8 +84,11 @@ int main(int argc, char *argv[]) {
printf("%02x ", buffer[i]);
printf("\n");
} else {
ret = gattlib_write_char_by_uuid(connection, &g_uuid, buffer, sizeof(buffer));
buffer[0] = (uint8_t) value_data;
buffer[1] = '\0';
ret = gattlib_write_char_by_uuid(connection, &g_uuid,buffer,1);

@oliviermartin oliviermartin Oct 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value_data is initialized with value_data = strtol(argv[4], NULL, 0); and strtol returns a long int. Would not be the correct code simply being:

ret = gattlib_write_char_by_uuid(connection, &g_uuid, value_data, sizeof(value_data));

@kwikius kwikius Oct 18, 2018 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note First: I am not suggesting that my code is suitable to be merged.

The type of the data should be the same as the type of the characteristic at the remote peripheral "server" end, shouldnt it? In my remote "server" end , the characteristic is an 8 bit type. Bluez seems to know this since passing more than 1 byte into bluez in my application is silently ignored and the value is not changed. (Note it is some time since I worked on it so I will have to go back and check the behaviour and get back to you)

Bluez seems to want the type to be presented as an array of some 8 bit type that can be converted back at the remote end. That would suggest some sort of define as to the type (or just size) of data being sent, which would be modifiable by the user for their own purposes.

assert(ret == 0);
printf("Write UUID completed: ");
}

gattlib_disconnect(connection);
Expand Down