Closed schoero closed 4 years ago
@Rogerrrrrrrs thank you for submitting this PR and all the work you have done on it, @AnthonyMyatt, @bahamut657 and thank you for your work on this PR too.
@Rogerrrrrrrs
All the new wave methods are currently methods in class Gpio. Why is this the case? Would it not be better if they were functions in global?
When the PR is installed with Node.js v13.5.0 using the command npm i https://github.com/Rogerrrrrrrs/pigpio.git#Waves
there are compile errors that need to be fixed.
```
pi@raspberrypi:~/pigpio $ npm i https://github.com/Rogerrrrrrrs/pigpio.git#Waves
> pigpio@2.0.1 install /home/pi/pigpio/node_modules/pigpio
> node-gyp rebuild
make: Entering directory '/home/pi/pigpio/node_modules/pigpio/build'
CXX(target) Release/obj.target/pigpio/src/pigpio.o
../src/pigpio.cc: In function ‘Nan::NAN_METHOD_RETURN_TYPE gpioWaveAddGeneric(Nan::NAN_METHOD_ARGS_TYPE)’:
../src/pigpio.cc:746:50: error: no matching function for call to ‘v8::Array::Get(unsigned int&)’
v8::Local
I'm in the process of reviewing the PR.
@Rogerrrrrrrs If you're not sure how to fix the compile errors that occur when installing with with Node.js v13.5.0 please let me know and I'll take a look. There are no compile errors with Node.js v8, v10 and v12 and everything functions correctly.
@fivdi Thank you, I would be happy to take your help on the compile errors.
All the new wave methods are currently methods in class Gpio. Why is this the case? Would it not be better if they were functions in global?
You are right, there is no benefit being a member of Gpio.
I agree with the requested changes and will take a look at it now.
@Rogerrrrrrrs please let me know when you think you're finished.
@fivdi I have moved the wave functions to the global level. I have updated the documentation, examples and tests. When I did that, I also had to update the return values of the functions that previously returned this
.
It also builds now without errors on node 13 thanks to your help.
I think you can review it again.
@Rogerrrrrrrs , @bahamut657, @AnthonyMyatt Thank you all very much for working on the PR.
Glad I could play a part.
This PR has now been published with pigpio@3.0.0 on npm.
@fivdi Were there any breaking changes in the 3.0.0 release besides dropping node 6? What about the 2.0.0 release?
There were no breaking changes in v2.0.0 or v3.0.0.
This pull request adds Wave functionality and resolves #38 and closes #64 and is basically a continuation of #64
It also adds ticks to the interrupt event as discussed here
The following methods where added:
The wave methods where tested using a Raspberry Pi 3B+ and verified using an oscilloscope.