Added handling of ACK for config commands.

[wiseman]

Sending this for discussion/review. I've only done minimal testing so far; I wanted to get some feedback before spending more time on it.

We now have two classes of commands: blocking and non-blocking. CONFIG is blocking, all others are non-blocking. When a blocking command is sent, no other blocking commands will be sent until an ACK is received (and cleared) from the AR.Drone.

Blocking commands can also have a callback & a timeout value (currently defaults to 50 ms). The callback is called when the ACK is received or when a timeout occurred (an error object is passed in the timeout case). For example,

client.config(
  'general:navdata_demo', 'TRUE',
  {
    timeout: 100,
    callback: function(error) {
      if (error) {
        console.log('Oh no: ' + error);
      } else {
        console.log('FINALLY');
      }
    }
  });

I'm thinking of adding a numRetries option to blocking commands that would cause them to automatically be re-sent for the first N timeouts before signaling an error.

Handling CONFIG commands in a separate queue will help with implementation of multiconfig, which I am planning on doing next.

Note that sequence numbers are now generated in UdpControl, and there is another legal navdata header value (which may have additional data included with it, but the existing parser seems to at least successfully parse what's there).

See issue #33, issue #47, and https://gist.github.com/bkw/4416785 for reference.

[bkw]

Wow! 👏 First off: thank You so much for taking the time to go ahead with this!
Unfortunately, I'm really busy right now. I hope to take a look later this week.
Gut reaction: Let's merge it in and iron out the kinks later, if there should by any. Let's go ahead and break things ;-)
/cc @felixge @eschnou

[bkw]

we should fix the test, though ;-)

[wiseman]

Hm, when I run tests locally in a fresh checkout of felixge's master, it fails the same two tests for me. I'm not sure what's up.

[yocontra]

@wiseman - Any chance we can add support for this signature?

client.config('general:navdata_demo', 'TRUE', function(err) {
});

Specifying an object with a callback key has always felt naughty

[wiseman]

That seems like a good idea to me.

[wiseman]

Commit 2be22dc also allows you to specify just a bare callback function without a full options object.

[felixge]

(currently defaults to 50 ms)

Seems a bit too low. I've seen latencies well beyond 100ms during nodecopter events. My gut says ~500ms. What do you think?

For API, I think we should have: config(key, val, cb) or config(options, cb). options would be an object with the keys 'key' and 'value' being mandatory.

Hm, when I run tests locally in a fresh checkout of felixge's master, it fails the same two tests for me. I'm not sure what's up.

Not sure either, if somebody could do a git bisect that would be awesome, I don't have much time right now : /

[wiseman]

OK, the timeout has been increased to 500 ms and I've changed config to accept either config(key, value, callback) or config({key: k, value: v}, callback).

[felixge]

@wiseman awesome! Just gave you github/npm push access, so feel free to merge & release this whenever you're happy with it (i am!). 💖

[wiseman]

Thanks for the feedback, everyone. I merged it and pushed 0.3.0 to npm.