search

lsongdev / node-bluetooth

:large_blue_diamond:😬Bluetooth serial port communication for Node.js
https://npmjs.org/node-bluetooth
Other
197 stars 56 forks source link

Use upstream implementation of native bindings #44

Open eelcocramer opened 4 years ago

eelcocramer commented 4 years ago

I notice that a lot of your current problems have already been fixed in the upstream module, which I maintain, and where you copied the sources from.

This PR makes you module depend on the upstream module so you are always using the latest sources and allow you to focus on maintaining your own javascript bindings.

eelcocramer commented 4 years ago

Will most likely fix #40 and #45

mdsaddam commented 4 years ago

@eelcocramer any time lines on fixing issue #45 ?

eelcocramer commented 4 years ago

May also fix #48.

frankjoke commented 4 years ago

@eelcocramer , can you fork this repo to your account and do the changes there so it can be used? I work on different linux systems (Example Debian buster10 or Ubuntu) for ioBroker and used the library in the past, but now I need to test apps also on node V12 and it does not work anymore there.

eelcocramer commented 4 years ago

@frankjoke the forked repo is already there. This PR originates from there. You can find the link below the title where it says eelcocramer:master but I've also added it here for your convenience ;-)

frankjoke commented 4 years ago

Ok, I tried this and could not load it, got ` node-bluetooth@1.2.6 install /home/pi/ioBroker.radar2/node_modules/node-bluetooth

node node_modules/copyfiles/copyfiles -f node_modules/bluetooth-serial-port/build/Release/BluetoothSerialPort.node Release internal/modules/cjs/loader.js:960 throw err; ^ Error: Cannot find module '/home/pi/ioBroker.radar2/node_modules/node-bluetooth/node_modules/copyfiles/copyfiles' at Function.Module._resolveFilename (internal/modules/cjs/loader.js:957:15) at Function.Module._load (internal/modules/cjs/loader.js:840:27) at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12) at internal/main/run_main_module.js:18:47 { code: 'MODULE_NOT_FOUND', requireStack: [] } if I link it directly. I believe that you use a module copyfiles which is not loaded automatically, instead integrating directly your bindings into the code. Did you create an npm package from it which could be included? npm init -scope=@eelcocamer npm publish --access public In this way we could include it as @eelcocramer/node-bluetooth in our package.json with normal npm handling. You would need to change the installation instruction in that way as well.

p.s.: I am not in in writing/managing nodejs code with bindings and C++ therefore I doi not kinow exactly what is needed there. I am mainly writin web apps or home automation adapters and tools.

eelcocramer commented 4 years ago

Well, I'm not going to publish it. But you can fork it and publish it yourself if you need it.

frankjoke commented 4 years ago

I tried this and managed after a view changes to your install script to get a module which could be installed on multiple systems. Youi can find it here https://github.com/frankjoke/node-bluetooth and it's npm is @frankjoke/node-bluetooth. but: It does not work! On one syste I get a crash of node if I start .scan() and on the other one scan is never returning and never finding anything. It seems that the the bindings were maybe not tested on many systems.

rzr commented 3 years ago

Feedback also welcome at

https://github.com/song940/node-bluetooth/issues/55#

eelcocramer commented 3 years ago

For the convenience of folks that use the javascript patterns used in @song940's module I've updated my PR to use the experimental C sources from my fix branch. So you can now use my fork of his repo to verify if these fixes work.

I don't think the inquiry functions work but for some folks reading and writing may be fine.

Feedback is appreciated!