felixge / node-ar-drone

A node.js client for controlling Parrot AR Drone 2.0 quad-copters.
http://nodecopter.com/
MIT License
1.76k stars 428 forks source link

Socket binding collision #20

Closed yocontra closed 11 years ago

yocontra commented 12 years ago

So I got node running on the drone (hurray) but I can't run node-ar-drone on it and have it connect to itself. (since the drone already owns the navdata port)

https://github.com/felixge/node-ar-drone/blob/master/lib/navdata/UdpNavdataStream.js#L39 seems to be the culprit. If I comment this out it doesn't seem to break anything and it allows me to run the REPL on the drone via telnet (mindfuck)

Is there a reason for the line that I'm missing or should it actually be removed? I looked at https://github.com/joyent/node/blob/master/test/simple/test-dgram-pingpong.js and it seems like you don't need to bind a client socket to a port

bkw commented 12 years ago

Good catch. Usually you have to bind a udp socket to receive data on it, but it should not be this._port (since that's the one we're sending to, we can receive on any port). Imho this should just be changed to this._socket.bind() without any argument.

felixge commented 12 years ago

@bkw SGTM. Have you tried it if it works?

I think I'll actually need to use the ar-drone lib on a drone myself fairly soon, so I'll probably check this out in a few days if nobody else does.

yocontra commented 12 years ago

@felixge - I can confirm that removing the port bind doesn't mess anything up

felixge commented 12 years ago

@Contra so nav data is still being received?

yocontra commented 12 years ago

@felixge - Yep - everything is working as it should. I'm flying mine around right now and receiving the navdata state changes from https://github.com/felixge/node-ar-drone/pull/18

felixge commented 12 years ago

@Contra alright, if you want to send a patch (that passes the test suite), go ahead, and I'll merge it right away!

bkw commented 12 years ago

The idiomatic way to receive on a UDP socket is to bind it (not necessarily to a specific port), so no other local processes can snatch it while you're using it. Maybe academic in our case but should not hurt either. AFD (away from drone), but I'll make an extra check later.

bkw commented 12 years ago

vote to close, continued in #21

yocontra commented 11 years ago

Fixed in master now - closing