-
Notifications
You must be signed in to change notification settings - Fork 162
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.
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
Maybe fix for a couple of bugs in write descriptors: #74
base: master
Are you sure you want to change the base?
Conversation
1) In the read_write example, in a write to descriptor, the buffer is never written with the value to write. 2) In /bluez/gattlib_read_write.c the sizeof the pointer is used rather than the length of the buffer when writing the descriptor I think there may still be problems in the read write example with regard to the type of the value you wish to write. In my case it is a uint8_t. I used the example here https://github.com/sandeepmistry/arduino-BLEPeripheral/blob/master/examples/led/led.ino Anyway with the version in my branch I can now write the value correctly, but probbaly needs work to be generic.
@@ -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'))) { |
There was a problem hiding this comment.
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);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
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); |
There was a problem hiding this comment.
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));
There was a problem hiding this comment.
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.
0f6ab98
to
860f7f5
Compare
In the read_write example, in a write to descriptor, the buffer is never written with the value to write.
In /bluez/gattlib_read_write.c the sizeof the pointer is used rather than the length of the buffer when writing the descriptor
I think there may still be problems in the read write example with regard to the type of the value you wish to write.
In my case it is a uint8_t. I used the example here
https://github.com/sandeepmistry/arduino-BLEPeripheral/blob/master/examples/led/led.ino
Anyway with the version in my branch I can now write the value correctly, but probably needs work to be generic.