Skip to content
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

Add option to pass HCI options when creating a device #28

Merged
merged 1 commit into from
Oct 2, 2018

Conversation

michal-narajowski
Copy link

This can be useful when we need to use specific HCI device ID.

@michal-narajowski
Copy link
Author

@moogle19 The Travis error is some issue with the script, I think. Could you review this patch?

@ccollins476ad
Copy link

The change looks good to me.

@moogle19
Copy link

@michal-narajowski
No, it's not a script issue. The problem is that hci.Options is not available for darwin and because of that, all darwin builds fail.
hci.Option is only available under linux, darwin uses device.Option.
I think the best way to use Option for both platforms would be an Interface (like we have for Client, Device, ...) which is implemented by hci and device.

@michal-narajowski michal-narajowski force-pushed the device-hci-opts branch 5 times, most recently from 579bd85 to 83953ac Compare September 3, 2018 08:21
@michal-narajowski
Copy link
Author

@moogle19 Silly of me, I didn't notice the compilation error. Does it look better now?

@moogle19
Copy link

moogle19 commented Sep 3, 2018

Looks good but I think examples/lib/dev/option is not the right directory for that.
Please put the option.go file into the root folder.

This can be useful when we need to use specific HCI device ID.
@michal-narajowski
Copy link
Author

Changed as requested

@rymanluk
Copy link

rymanluk commented Oct 2, 2018

I tested it and it looks good to me.

Copy link

@rymanluk rymanluk left a comment

Choose a reason for hiding this comment

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

Are there any comments to resolve here or we can merge it?

rymanluk added a commit to rymanluk/mynewt-newtmgr that referenced this pull request Oct 2, 2018
@moogle19 moogle19 merged commit e78417b into go-ble:master Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants