natevw / node-nrf

Pure-JavaScript nRF24L01 driver library
117 stars 31 forks source link

Handle stream closing. #13

Open natevw opened 10 years ago

natevw commented 10 years ago

Right now once you open a stream, it reserves a slot "forever". We need to:

VaclavSynacek commented 10 years ago

Hi, I would like this to get solved. Or alternatively if it would be possible to change pipe's address. I am not good enough to create a pull request myself, but here is what I found with a little experiment.

I'm trying to rewrite a C program based on RF24 to node and node-nrf. I have 10 arduinos talking to one RPi (some home sensors - temperature, light, etc). In order not to have more than one nRF24l01 on the RPi the arduinos are all using one shared pipe address to talk to the RPi. The RPi replies on a dedicated pipe address to each arduino. For this to work, the RPi has to recycle the nRF24l01 limited pipes.

In RF24 that is not a problem, I simply change the address of the one tx pipe being used.

In node-nrf I tried implementing the same with the following inside the 'data' event handler:

var replyPipe = radio.openPipe('tx', arduino1_address);
replyPipe.end(data_to_arduino);

This sort of works - the data gets transmitted. However after about 8 transmissions there is a warnning:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at EventEmitter.addListener (events.js:160:15)
    at EventEmitter.once (events.js:185:8)
    at /home/pi/RF24Node/node_modules/nrf/index.js:338:17
    at /home/pi/RF24Node/node_modules/nrf/index.js:79:25

But after that the data again gets delivered. So it is not like it is not working at all, it just is a bit unstable and ugly workaround.

natevw commented 10 years ago

Thanks for the feedback. It shouldn't hurt to open as many 'tx' pipes as you'd like, and I will try look into this warning soon. Enough backlog on this library that I'm planning to set some blocks aside for this as soon as I've got my dev setup back (within a week).

VaclavSynacek commented 10 years ago

And one more think: the warning is probably true. I checked on the program after a day of continuous run and it gradually eats more and more memory and processing power. After about a day running the above code every 2 or 3 seconds it goes to almost 100% CPU of Raspberry Pi (probably processing all those unclosed pipes).

natevw commented 10 years ago

Interesting, that's probably a bug then. If you need to workaround sooner, would it be possible to keep an array of TX pipes to all 10 of your devices, instead of recreating?

VaclavSynacek commented 10 years ago

Coming from the RF24 library I was quite skeptical about this suggestion. How can 10 TX pipes coexist on top of only 6 hardware pipes of nrf24l01? But you got the switching covered in the library! It works. Thanks for the advice.

BTW: The memory leak warning is still there after creating the 8th pipe for the 8th arduino, but the memory and cpu usage seem stable after a few hours. So this does not really bother me any more.

natevw commented 10 years ago

Looks like there might be a little more to do yet, but when I was finally back in the code tonight I noticed that the streams already do have a .close() method on them which does already do some cleanup

So another potential solution to the trouble you are having is to call replyPipe.close() after you end it…I should probably make this happen automatically though; I can't think of any technical reason why .end() shouldn't close for you.