Add remaining navdata options#29
Conversation
Committing without testing. What could go wrong?
* 'master' of git://github.com/Contra/node-ar-drone: config test chaining
* `require('..')` not `require('ar-drone')`
* Use `client` (not `drone`) for ARDrone instance.
* Use hyphens, not underscores, for script names.
Unsure `ARDrone_SDK_2_0/ARDroneLib/Soft/Common/navdata_common.h:753` has `uint32_t link_quality` but using that: * Breaks a test * Produces a value from 0 - 500 * Causes strange behavior[^1] [^1]: (Values were observed to start at 500 and gradually decline to 0 in about 10 seconds. Sometime later the value would incrementally increase back to 500 and, again, slide back down to 0.)
Based on (guessing, after reading) ARDrone_SDK_2_0/ARDroneLib/Soft/Lib/ardrone_tool/Navdata/ardrone_navdata_file.c:51
…suffix, too." This reverts commit 4a87e5b.
There was a problem hiding this comment.
Don't use spaces after the function keyword in an unnamed function (to keep in sync with the style used in the project).
|
First of all, amazing work! ❤️ this patch! Problems:
Anyway:
So you can go ahead and merge / publish this as soon as you feel happy with it. You know what I'd like to see changed, but if you don't have time, IMO it's better to get this in now anyway. Going forward: I've been hacking on a new API over here: https://github.com/felixge/node-ar-drone/commits/new-api One of the the changes is turning the navdata stuff into a more human friendly object: https://github.com/felixge/node-ar-drone/blob/new-api/lib/navdata/message.js However, now that you've added all the navdata stuff there is, I'm not sure if my approach is a good idea, or if we should continue to group navdata elements by the option tag they belong to. The reason why I wanted to restructure this whole thing is to simplify the programming. Example: if (navdata.demo && navdata.demo.batteryPercentage < 30) {
console.log('Battery is too low for flips now!');
}Could be simplified to just: if (navdata.demo.batteryPercentage < 30) {
console.log('Battery is too low for flips now!');
}Would love to hear your thoughts on this! (One possible way to do this better is to pass all the navdata option tags you want to receive when configuring the client, and if incomplete navdata packages are received, we simply don't emit them - that may work really well) |
|
❤️ those patches above! |
|
👍 This is going to be very useful John! thanks |
|
Thanks for the compliment, I'm just glad to help. Thanks for adding me to the project, that's incredible. I'll work on addressing as many of the problems as I can before I merge this:
|
3 participants
Added the 22 missing navdata options.
Notes:
demoandrawMeasuresdata structures.Planifshort for the Frenchplanification, which would beplannedin English?I just saw your new-api branch. Do you already have a plan/roadmap for the API & navdata? I don't want to waste your time with PRs that have different structures than you'd like.