natevw / node-nrf

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

Extra, not needed, packets are received. #30

Open atomicus opened 9 years ago

atomicus commented 9 years ago

If I open the pipe: var rx = radio.openPipe('rx', 0xF0F0F0F0F1, {size: 'auto'}); for each packet send to my code I get two on('data') events:

First one is correct one - 'data' is proper Second one is the same size as first one, but filled with last character of first

Example:

Send: XXX [what was send by arduino]

-Recv: YYY [what was received by rpi]

send: 0001

-recv: 0001 -recv: 1111

send: 0002

-recv: 0002 -recv: 2222

And so on. *note: displayed data is reversed - due to #28

Calling: var rx = radio.openPipe('rx', 0xF0F0F0F0F1, {size: '4'}); makes problem disappear, but then ACK does not work as in #29

So, no way to use lib...

natevw commented 9 years ago

Hmm, sorry you are having this trouble. I have not used manual packet size much, but was supposed to be implemented. Could you set radio._debug = true before you call any methods on it, and then post a sample log to https://gist.github.com/?

atomicus commented 9 years ago

Sorry you had to wait, here it is: https://gist.github.com/atomicus/88ff3b8ece5b26f75d18 Take a look at line 1948 - theres a proper "101" message received, later on 1960, theres malformed, doubled "11111" received.

This happens also on line 2485->2497.

Just ctrl+f for numbers 101,102,103... and you will find it.

natevw commented 9 years ago

Thanks, and no worries on the delay. This afternoon I am getting my hardware back together for better followup and future development the backlog of issues here.

Looking at the logs [with the big disclaimer that I'm a bit rusty while having been "away"] there's a strong smell of race condition, which seems a good guess especially since you are using software poling instead of hardware IRQ.

The snippet of the log you pointed out:

IRQ. { RX_P_NO: 1, TX_DS: 0, MAX_RT: 0, RX_DR: 1 }
execCommand R_RX_PL_WID 1
_checkStatus, irq = false checking = false
execCommand [ 'R_REGISTER', 7 ] 1
 - exec read: <Buffer 42 05>
execCommand R_RX_PAYLOAD 5
 - exec read: <Buffer 42 42>
gotStates { RX_P_NO: 1, TX_DS: 0, MAX_RT: 0, RX_DR: 1 } null
IRQ. { RX_P_NO: 1, TX_DS: 0, MAX_RT: 0, RX_DR: 1 }
execCommand R_RX_PL_WID 1
_checkStatus, irq = false checking = false
execCommand [ 'R_REGISTER', 7 ] 1
 - exec read: <Buffer 42 20 20 31 30 31>
setStates { RX_DR: true, TX_DS: false, MAX_RT: false }
 - exec read: <Buffer 4e 05>
execCommand R_RX_PAYLOAD 5
 - exec read: <Buffer 4e 4e>
gotStates { RX_P_NO: 7, TX_DS: 0, MAX_RT: 0, RX_DR: 1 } null
execCommand [ 'R_REGISTER', 7 ] 1
_checkStatus, irq = false checking = false
_checkStatus, irq = false checking = true
 - exec read: <Buffer 4e 31 31 31 31 31>
setStates { RX_DR: true, TX_DS: false, MAX_RT: false }
 - exec read: <Buffer 4e 4e>
execCommand [ 'W_REGISTER', 7 ] [ 78 ]
_checkStatus, irq = false checking = true
_checkStatus, irq = false checking = true
  101
_checkStatus, irq = false checking = true
execCommand [ 'R_REGISTER', 7 ] 1
_checkStatus, irq = false checking = true
 - exec read: <Buffer 0e 0e>
gotStates { RX_P_NO: 7, TX_DS: 0, MAX_RT: 0, RX_DR: 0 } null
execCommand [ 'R_REGISTER', 7 ] 1
_checkStatus, irq = false checking = false
 - exec read: <Buffer 0e 0e>
execCommand [ 'W_REGISTER', 7 ] [ 78 ]
_checkStatus, irq = false checking = true
_checkStatus, irq = false checking = true
11111

There is an execCommand R_RX_PAYLOAD but while that is in progress it gets back the status from a previous [presumably polling] command and that triggers another IRQ. to do another read. So — again/disclaimer: unconfirmed by real testing or even reviewing the code or datasheet — I suspect what is happening is simply the JS library thinks it needs to read because of the "second" IRQ, but when it does do so the chip doesn't have anything for it and so the second read gets back ± garbage as you're seeing.

I sort of patched over an unrelated bug due to a similar race condition in https://github.com/natevw/node-nrf/commit/385d9c5720c144b688c2e491b93a344bafac4c39 but even at the time that seemed a bit hacky of a solution. But this is an even worse bug, and I think it's time to figure out how to really fix all these kinds of asynchronous woes…

natevw commented 9 years ago

Oy, I always write soooo much. Executive summary:

Sorry, this is probably an architectural bug that may take a bit to fix…so it might get caught up in a more general overhaul. However, it is a pretty high priority bug since I can see how it would make this library useless in your case.

If you can, hooking up the hardware IRQ pin may make the problem much less significant [or potentially avoid it] in the meanwhile.

atomicus commented 9 years ago

Gosh, I missed github emails. I'll turn on IRQ pin and let you know if it fix it.

atomicus commented 9 years ago

I've tried to use IRQ pin, but whatever I use as 3 param of connect() (i tried, 25,14,17) i get this error:

node: symbol lookup error: /home/pi/nodejs/lihnode/node_modules/nrf/node_modules /pi-pins/node_modules/epoll/build/Release/epoll.node: undefined symbol: _ZN4node 12MakeCallbackEN2v86HandleINS0_6ObjectEEENS1_INS0_8FunctionEEEiPNS1_INS0_5ValueE EE

natevw commented 9 years ago

Seems like you probably need to rebuild your project's dependencies after upgrading to node 0.12?

atomicus commented 9 years ago

Yep, you were right.

Enabling IRQ fixes the problem with extra packets, i'll leave it up to you if to close this issue.

natevw commented 9 years ago

Glad to hear it! I'm going to leave this open since if we do have fallback for running without IRQ pin, it shouldn't be left broken like this.