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

Import native mac bindings from notjosh/bleno-mac. #13

Merged
merged 4 commits into from
Sep 3, 2020

Conversation

ptx2
Copy link

@ptx2 ptx2 commented Aug 9, 2020

Fix issues on macOS 10.14+ where advertising does not work. The code is from notjosh/bleno-mac and is based on Timeular/noble-mac which has already been added to abandonware/noble.

@ptx2
Copy link
Author

ptx2 commented Aug 9, 2020

WIP / seeking advice / feedback. Potentially fixes #10 and other issues.

I ran into an issue where advertising wasn't working on macOS 10.14.

Through this ticket noble#404 I found https://github.com/notjosh/bleno-mac which includes native macOS bindings for bleno, based on the noble bindings from Timeular/noble-mac that were imported into abandonware/noble in this commit: abandonware/noble@556f5fb about 18 months ago.

I have tested this briefly for my use-case https://github.com/ptx2/gymnasticon on macOS 10.13 and 10.14 and it seems to work.

I have not tested any of the beacon functionality or any other versions of macOS as these two are all I have available right now.

I figured I'd open this to see if it's appropriate to pursue merging these bindings at all and start a discussion re: next steps.

Thanks for keeping this library alive!

@rzr
Copy link

rzr commented Aug 9, 2020

I am not against merging this but I'll like that commit message to be updated why var is prefered over const ?

@ptx2
Copy link
Author

ptx2 commented Aug 9, 2020

Thanks for the quick reply.

I don't actually prefer var ;) but const was causing the build to fail due to the .jshintrc settings.

I could put it back to const and update the jshintrc to "esversion": 6 and package.json engines to "node": ">=6" if that'd be preferred?

@rzr
Copy link

rzr commented Aug 10, 2020

yes it looks to me a better option

@ptx2
Copy link
Author

ptx2 commented Aug 10, 2020

I was looking more closely at these bindings vs the noble ones. It looks like this will only compile on macOS 10.13+ whereas noble has some #ifdefs to support earlier versions of macOS. I guess bleno could just fall back on the previous bindings if there is something blocking there.

I also found some applications which don't work with these bindings.

I'll try to look into resolving both of those this week.

Copy link

@rzr rzr left a comment

Choose a reason for hiding this comment

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

LGTM

@rzr rzr merged commit e5daab4 into abandonware:master Sep 3, 2020
@ptx2 ptx2 deleted the native-macos-bindings branch October 22, 2020 11:08
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.

2 participants