-
Notifications
You must be signed in to change notification settings - Fork 175
gattc: use GetUUID() to allow for bare metal use of short UUID. #14
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
Conversation
|
This PR is intended to address the comments from #11 |
aykevl
left a comment
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.
GetUUID works, but Effective Go suggests leaving out the Get prefix: https://golang.org/doc/effective_go.html#Getters
To make that possible, perhaps you can unexport the UUID field (renaming it to uuid) or rename it to something else such as UnderlyingUUID?
6fc6226 to
16c01a9
Compare
|
That was what I was trying originally but I did not immediately figure out how to handle the naming conflict you mentioned, which is why I took an "easier" way out. I realized I can do what I wanted using a type alias, which I have just now changed and pushed to this branch. |
|
This is the function to convert from |
|
I think I have made all of the changes requested but am unable to test them due to #15 |
|
You could use any development from Nordic to test these changes. But I will also try to test it. |
aykevl
left a comment
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.
Unfortunately this doesn't work correctly on a SoftDevice:
$ tinygo gdb -target=pca10056-s140v7 ./examples/scanner/
[...]
(gdb) bt
#0 runtime.nilPanic () at /home/ayke/src/github.com/tinygo-org/tinygo/src/runtime/panic.go:32
#1 0x0002713e in (tinygo.org/x/bluetooth.Addresser).Set ()
It appears that the MAC address is being set on a nil Address.
Seems like this problem exists on |
6a7c098 to
b07ce67
Compare
…t UUIDs Signed-off-by: deadprogram <ron@hybridgroup.com>
Signed-off-by: Ron Evans <ron@hybridgroup.com>
Signed-off-by: Ron Evans <ron@hybridgroup.com>
…service and characteristic discovery Signed-off-by: deadprogram <ron@hybridgroup.com>
…stics Signed-off-by: deadprogram <ron@hybridgroup.com>
Signed-off-by: deadprogram <ron@hybridgroup.com>
|
Made a great deal of progress here. Now I am testing and discovering some differences between the results being returned by Linux vs. the SoftDevice. vs. my current code on SD: First thing I notice is that the short UUIDs on SD do not look like what I was expecting. Is that an error? In any case, also why are more services being returned? And likewise with the number of characteristics per service? I think I am getting pretty close here. |
Signed-off-by: deadprogram <ron@hybridgroup.com>
8103703 to
7d120e5
Compare
|
Found the error in converting short UUID to UUID, and pushed a commit to correct that. Now, here is my debug output: |
Signed-off-by: deadprogram <ron@hybridgroup.com>
Signed-off-by: deadprogram <ron@hybridgroup.com>
|
OK, now after testing, I am pretty confident that the SD version is working exactly as it is supposed to with my latest commits: Note this is the "heartbeat" example. |
aykevl
left a comment
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.
Looks good, except that I discovered some unsafe code. Other than that it looks good to me.
…ting short UUID Signed-off-by: deadprogram <ron@hybridgroup.com>
|
Great! Now we can discover services and characteristics from SoftDevice chips. |
This PR adds a GetUUID() accessor to both
DeviceServiceandDeviceCharacteristicto simplify use on bare metal with short UUIDs.