search

JamesBarwell / rpi-gpio.js

Control Raspberry Pi GPIO pins with node.js
MIT License
657 stars 116 forks source link

setPollFrequency() #7

Closed aleksipirttimaa closed 10 years ago

aleksipirttimaa commented 10 years ago

I added function to change the poll interval this should help a little with the issues of the events, interrupt driven solution may be better tough, I just needed this working quick and this was easier.

JamesBarwell commented 10 years ago

Thanks for the patch. But I don't understand why in your second commit, you have changed the argument order so that the options object gets passed in as the third argument. The API documentation states that options is the second argument: http://nodejs.org/api/fs.html#fs_fs_watchfile_filename_options_listener

I'll bring this into my own branch for some tidy up + add and fix the tests, so I don't need the pull request modifying - I'm just wondering if I'm missing something.

aleksipirttimaa commented 10 years ago

I did this in more rush than I should have. I found the documentation confusing, it might have changed or just is othervise confusing. I'll try my best, tomorrow.

aleksipirttimaa commented 10 years ago

After reading code it is the second argument that should hold the options object. I were wrong and I will feel ashamed. I tought I had issues with it tough.

JamesBarwell commented 10 years ago

Hi, that's fine, I'm happy with what you've provided. I don't have my Raspberry Pi set up at the moment so if you could confirm that it does work for you when the options are passed in second then that would be useful - I can just throw away the other commit (which I think you may have had to do to get the tests to pass?).

I've pulled your code into my own branch to make some more amendments before merging it in. I've got a few more patches that I hope to release shortly, then I will merge your work in after.

By the way, what polling frequency are you setting it to? I'm interested to know how well it picks up changes with this method. I would like to investigate the use of interrupts in the future but as you say, this is a good interim fix.

aleksipirttimaa commented 10 years ago

I changed the function arguments the right way around and it won't 'npm test', with this output ... 31 passing (55ms) 1 failing

1) rpi-gpio pin value change "before each" hook: Uncaught TypeError: object is not a function at onSetupComplete (/Users/axu/Desktop/rpi-gpio.js/test/main.js:464:17) at /Users/axu/Desktop/rpi-gpio.js/rpi-gpio.js:279:24 at /Users/axu/Desktop/rpi-gpio.js/node_modules/sinon/lib/sinon/behavior.js:113:26 at process._tickCallback (node.js:419:13) ...

When I used my own code with the correct setPollFrequency() implementation it just repeated the 'change' with same input multiple times. I also noticed that the 'change' event wouldn't react to only the first change, the rest were ignored. Odd was also that the change would happen sometimes even without my input on the gpio. (-Later testing makes me question the fs.watchFile implementation)

When I tried your latest code 0.5.0. I got the same behavior as i described, without the repeating of the 'change' event. The little testing I had done with 0.4.3 seems to be indicating the same behavior, maybe its just something on my setup? img_0575 'change' event repeating

JamesBarwell commented 10 years ago

I've merged your first commit (cheers!) + some tidy up into master and published v0.4.1. I went back to the watchFile documentation and added the modified time check as per their recommendation. This may help with the repeated event issue.

I'll close this now. If the repeated event problem persists then feel free to open a new issue specifically for that. Though I don't have time to debug with the physical hardware right now - so hopefully someone can work it out and give me a pull request.