BB.LeapMotion Suggestions

[brannondorsey]

Hey @cfvalencia9277, I've got a few suggestions for the Leap Motion module:

1. Provide getLeapData(...) functionality in the BB.LeapMotion() constructor instead of as it's own function. There is no real use case to use this module without needed what getLeapData(...) provides, so we might as well put it in the constructor. Besides, in JavaScript it is the convention to have any function that begins with get return a value instead of simply being a void function.
2. It would be helpful if this leap module could be used independent of the need for an HTML5 canvas, and so far I only see it being passed into getLeapData(...) so that its width and height properties can be accessed. Instead we should pass in width and height (and depth) properties in the constructor with the option to update/alter them after construction via some method as well.
3. We've also traditionally gone with config-object style of initializing constructors that looks like this:

var leap = BB.LeapMotion({
    width: 500,
    height: 400,
    gestures: true
});

vs

var leap = BB.LeapMotion(500, 500, true);

This also allows us to easily have defaults if a property is not included in the config object. That looks like this in code.

4. Have you seen the documentation for defining/mapping "Interaction Boxes" when using the LeapMotion? The way that the leap frame data is used varies dependent on if it is being used in a 2D or 3D scene, so we should have some way of mapping for 2D vs 3D etc. I really like how ofxLeapMotion handles this, particularly these three methods (see here for source). Perhaps we could adopt that style.
5. Because we would like to eliminate the need for canvas, it would be helpful if canvasX and canvasY were simply renamed to x and y. Down the road I'm hoping to integrate this leap module with BB.Pointer, especially because the idea of a leap motion as a device having x, y and z properties doesn't really make sense, but rather the leap object could be funneled into a BB.Pointer instance and then it would extract that data from a finger (kind of like how BB.MouseInput works with BB.Pointer to provide it with added functionality.
6. It is important for us to have access to at least all of the functionality that the base SDK provides, so it would be worth having a BB.LeapMotion.lastFrame property that begins as null and then in each controller.on("frame") callback it assigns the current value of frame to lastFrame. That way we can have access to properties like hands, etc...
7. Finally, it would be nice to have access to all of the functionality in BB.LeapMotion via an event-style API as well as the current property-access API, so that if the programmer preferred to use this style if they preferred:

// assuming leap is an instance of BB.LeapMotion()
leap.on('grabGesture', function(frame){
    // some code...
});

vs

// assuming update() is running at the animation frame rate of the app
function update() {
    if (leap.grab) {
        // some code...
    }
}

This is less pressing than the other methods however. Plus, I have been working on but have yet to push a BB.EventEmitter class that BB.LeapMotion could extend from which would make this easier. I will ping you when that is included in the library core and then these updates can be made so you can hold off on implementing no. 7 for now.

I'll be back in the studio on Friday if you have any questions. Thanks for your contributions!

[brannondorsey]

Awesome improvements with https://github.com/brangerbriz/liBB.js/pull/42. Here are some thoughts:

1. canvasConstructor optional config object property in the constructor can be named canvas for clarity.
2. What is the reasoning for making mapX(), mapY(), and mapZ static methods (i.e. BB.LeapMotion.mapY rather than BB.LeapMotion.prototype.mayY)? It seems to me that behavior should be tied to each leap motion object itself. This isn't too much of an issue and if you've got a specific intention behind it that I am missing then maybe it makes sense. Although it is a bit of an outlier situation it may be nice at some point to use two LeapMotions in one project (granted I'm not even sure the Leap library supports multiple devices) and I can see the rare possibility of wanting to define an interaction box per leap rather than for all leaps as a whole.
3. Will mapX() and mapY() eventually support minY, minX, maxY, or maxX properties for their config objects or is that functionality that only mapZ() will provide? If not we should not pass a config object to those methods if they only have one property available to configure, and should instead pass that value as a single parameter.
4. Im not convinced that the config.mapDimension property in the constructor config is the best way to configure the enabling/disabling of the Z dimension. These lines of code make me fearful of a bug where the programmer passes a canvas into canvasConstructor and sets mapDimension: 3 in the config, or if no canvas is given but mapDimension is not set or set to 2. Is there any harm in always using three dimensions (even if a canvas is specified) and having a default map range (something arbitrary but documented like -1000 to 1000)? Then it can be assumed that in a case like that a user would probably only use x and y values but would have the option of using z (even if they didn't call mapZ()). That also skips a config step of setting the number of dimensions.

[cfvalencia9277]

hey Brannon i have worked on the new suggestion, but i would like to talk to you to clear some things out, maybe we can skype tomorrow, let me know. Thanks !

[brannondorsey]

Hey Fabian, great! I will open skype now and see if there is a good time we can connect today.

On Thu, Mar 31, 2016 at 3:38 PM, Fabian Valencia notifications@github.com wrote:

hey Brannon i have worked on the new suggestion, but i would like to talk to you to clear some things out, maybe we can skype tomorrow, let me know. Thanks !

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/brangerbriz/liBB.js/issues/40#issuecomment-204116523

http://brannondorsey.com