rwaldron / johnny-five

JavaScript Robotics and IoT programming framework, developed at Bocoup.
http://johnny-five.io
Other
13.3k stars 1.77k forks source link

Ideas for Servo.js and to() #304

Closed dtex closed 10 years ago

dtex commented 10 years ago

I'm working on a Johnny-Five controlled hexapod and have some ideas for things that might help with my walk sequences. If you think this makes sense I can submit a PR.

Option to use FPS instead of rate

Allow users to express rate in terms of Frames Per Second instead of a number of steps. That way we wouldn't burden them with choosing the correct number of steps for each individual movement (it should be dependent on the duration right?). We could just take FPS once and use that for the life of the Servo instance.

Users could choose to use the current rate implementation or add an FPS parameter to the Servo opts. If rate is not passed we can calculate rate based on the opts.fps.

Tween on current time instead of by step

Refactor to() so that it calculates percent complete using the clock and then tween from there instead of using steps. That could smooth out any latency on the JS side. The bigger plus for tweening is that it would be handy for constructing sequences.

Sequences

We could extend to(), allowing users to create sequences by passing in an array of keyframes as the first parameter like this:

// move from our current pos to 45 degrees in 250ms, then to 180 deg in 750ms
// Assumes current deg = 0
to([ 
  { cue: 0.25, deg: 45 },
  { cue: 1.0, deg: 180 }
], 1000 );

By using a cue point between 0.0 and 1.0 instead of ms values we could scale the sequenced animation by simply changing the duration parameter.

We could also pass easing functions

// Gravity-ish
// Assumes current deg = 0
to([ 
  { cue: 0.5, deg: 180, ease: 'easeOutQuad'  },
  { cue: 1.0, deg: 0, ease: 'easeInQuad'  }
], 1000 );

Servo groups

A method for building sequences on N servos that use one timer so that all servos in a group are updated using their individual sequence when we reach a new frame. This adds efficiency over having N timers since much of the tween work can be done once for each frame instead of doing the math N times on separate timers.

I think this could all be pretty awesome for any walking bots.

rwaldron commented 10 years ago

Option to use FPS instead of rate

Tween on current time instead of by step

Let's explore this—I haven't actually used the rate argument for anything I'm working on, but I've had a "note to self" to revisit this and see what can be done to refactor and reduce the 100 steps per movement to a calculated number of steps based on the distance to travel and the time to travel.

Sequences

I've been using temporal's temporal.queue for walking nodebot sequences with great success.

Counter proposal?

I want to give you complete freedom to explore your ideas presented here, but instead of further overloading to, I'd like to create a new API that I think more accurately describes the operation you're creating: animate, ie. servo.animate(keyframes, duration). Does that makes sense?

dtex commented 10 years ago

I think revisiting rate would be good. My gut says using an arbitrary value is kinda like animating in javascript without requestAnimationFrame. It may look okay, but you are probably rendering things that will never be painted or missing paints completely.

I also agree that it makes sense to add animate() instead of overcomplicating or refactoring to().

I'm going to switch my hexapod's walk sequence to temporal and also mock up how the same sequence might look with Servo.animate() in the mix. I'm not using any easing yet so it'll be easy. I'll compare the complexity of each and see if it makes sense to implement animate(). My hunch is it will.

Does temporal work well with high frequencies (10 - 30 ms)? If so, I think animate() and temporal would work very well together. I'll use temporal and all the methods there-in to manage the refresh loop. Animate would just add a shorthand to replace the functions that temporal calls. It would also add tweening, easing and duration scaling.

Is there any reason temporal is not used more in Johnny-Five?

rwaldron commented 10 years ago

Does temporal work well with high frequencies (10 - 30 ms)?

It should, as long as your only doing cheap serial write in each of the task handlers (which is essentially what I created it for). This may not be as reliable with node 0.10.x, but the issues with setImmediate were apparently resolved in 0.11.x and will arrive in 0.12.x stable.

In the meantime, I've used temporal on 0.10.x with a 3dof (per leg) quadruped and had no issues. Here's a really rough sketch of preset walking code (ie. no dynamic position transforms or IK): https://gist.github.com/rwaldron/df83899044683fac7390 I can get you some video later if you'd like.

rwaldron commented 10 years ago

Looks like I also pasted in the "wave" code... that's funny, the quad does "the wave" by lifting and dropping one leg at a time in series :D

dtex commented 10 years ago

I've been playing around with some patterns to see if they felt right before I spent any time on code. I used your walk function and converted it to use Servo.animate() and Servo.animateGroup().

For reference, here's a link to your walk() using temporal: https://gist.github.com/dtex/9140660

And here's the same done with Servo.animate() https://gist.github.com/dtex/9140696

As for functional differences they're what I mentioned before:

I wouldn't use animate() in a situation with that many servos on the same timeline. Servo.animateGroup() is a better solution for that: https://gist.github.com/dtex/9140728

It would have a lot of the same properties as Servo.animate(), but all the servos in the group would share a timeline and cue points. It's a lot more concise without being hard to read. You could move Cue points around within the animation so less work to refactor animations.

Both would use temporal for the refresh loop.

Next goal would be Chains() for IK. You would animate Chains using a signature like Servo.animate(). The big difference is we would pass in target coordinates instead of degrees on the cue points. Chains could be members of animateGroup as well.

rwaldron commented 10 years ago

The last gist is beginning to look like the pattern I've been experimenting with for my 3DOF biped: https://gist.github.com/rwaldron/9145816

rwaldron commented 10 years ago

After looking at the two gists, A and B I've noticed that B doesn't include all of the steps in the gait—there should be 4 phases, but only appears to be 1?

rwaldron commented 10 years ago

Same with C

rwaldron commented 10 years ago

Oh my, nevermind, I just figured out what I was missing. Sorry, ignore all of that.

rwaldron commented 10 years ago

Ok, now that I'm not completely misunderstanding this ;)

It would be nice to pass a single array of servos that correspond to the "members" (this avoids requiring the servo property on every member) https://gist.github.com/rwaldron/9146135 (we could easily support all three)

rwaldron commented 10 years ago

It occurred to me that "animateGroup" might make sense as a method of "Servos" (the collection API), or maybe not?

dtex commented 10 years ago

I like passing an array of servos one time. I was worried that the terse syntax might be hard to read and you do have to mentally match indexes, but if we support all three then the user can choose the style that works best for them. Not unlike the way pins are handled on Motor.

As an aside, I like having options but I do worry about things like this sometimes. The way files are passed to Grunt.js is a good example. I like Grunt, but if you are looking at various examples to learn and none of them are done the same way it's frustrating.

If I'm looking at the the right thing, the servos collection is all the servos that have been constructed. We wouldn't necessarily want to script all the servos (For example, arms and legs would be independent of each other). Maybe we need a ServoGroup constructor and then we'd have servoGroup.animate() https://gist.github.com/dtex/9157009

We could then extend servoGroup to support chains for kinematics:

// Get the effector position
where = myGroup.getPos(); // returns an array

// Set the effector position
myGroup.setPos( [ 1,2,3 ] );

I realize getPos and setPos imply a lot of math, but hopefully we could leverage Vektor for that and give Johnny-Five users a relatively simple gateway to that power. Designing an API that allows them to easily define a chain without tons of existing knowledge would be a good challenge.

I know Vektor can already handle the forward kinematics for getPos), but it's not clear to me if Vektor does everything we might need for setPos(). If it doesn't maybe we could help get it there (and by "we" I mean people smarter than me).

dtex commented 10 years ago

Just had another idea:

// A group of servos
var lf = new ServoGroup({
  coxa: { servo opts }, // Creates a new servo instance
  femur: myExistingServo, // Uses an existing servo instance
  tibia: {servo opts }
  walk: {
    cuePoints: [0.125, 0.5, 0.625, 1.0], 
    duration: 2000,
    timeline: [
      { coxa: [82, dir === "fwd" ? 130 : 82, 103, 103] }, 
      { femur: [111, dir === "fwd" ? 0 : 111, 70, 70] }, 
      { tibia: [103, 103, 77, 77] }
    ]
  },
  sneak: { /* another sequence */ }
});

/* Define the other legs here */

// A group of groups
var legs = new ServoGroup([ lf, lb, rf, rb ]);

// Move one joint
lf.tibia.to(128); // === legs.lf.tibia.to()

// Move one leg
lf.walk();

//Move all the legs
legs.walk();
rwaldron commented 10 years ago

the servos collection is all the servos that have been constructed.

This used to be true, but a few months ago I upgraded the constructor to allow for the creation of arbitrary groups:

var legs = new five.Servos([ left, right ]);
...

And so forth.

I'm checking email before dinner, which is silly to do, so I will respond to the rest later :)

dtex commented 10 years ago

Sorry for the delay, but I'm about ready with servo.animate() and I'm looking at the servos collection for working with groups of servos. servos.animate() will work as is but it will be inefficient since it will perform a full tween for every servo. In some cases our values will be exactly the same and don't need to be recalculated. In others our cuePoints will be the same and only our keyFrames will vary so there is no need to recalculate our position on the timeline.

Bottom line, we will have servo.animate(), servos.animate() and servos.animateGroup(). The latter will just add the ability to share cuePoints and or keyFrames across multiple servos.

I also want to ask if it's okay to add a dependency. I'm using ease-component from @tjholowaychuk . I had intended to just borrow the easing code from jQueryUI but this module already exists.

rwaldron commented 10 years ago

The new dependency is fine

dtex commented 10 years ago

Still peeling the onion on animation and have decided to add in support for I2C controlled servos (targeting the Adafruit PWM shield). That lends itself to a Devices implementation for servo.js. Do you have a preference on wether that comes in a separate pull request or should it be part of this one?

rwaldron commented 10 years ago

That's a different issue and PR please. I've tried controlling this in the past but had no luck getting it to work correctly. I was also not sure how its addressing system would make sense when Servo is one servo, but it sounds like you have ideas?

dtex commented 10 years ago

I know I'm rambling here but it really helps me to "talk things out".

I've got a feature I want to add to animation but I'm not sure how best to approach it. I want a concise way to run animations consecutively. There are many reasons to to do this:

  1. Animations can be too complex to manage on a single timeline so you may break them into smaller pieces
  2. Animations may depend on earlier animations (I can't start my walk loop unless I'm in a suitable starting position)
  3. We may just want larger sequences that are made up of smaller, distinct animation parts that are useful on their own.

We already have some ways to do this, but they are not particularly concise.

We can use the onComplete parameter in the animate opts object

hexapod.stand = function() { 
  // hexapod.legs is a servoArray
  hexapod.legs.animate({
    duration: 1000,
    cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
    keyFrames: [ ... ],
    onComplete: function() {
      hexapod.preWalk();
    }
  });
};

hexapod.preWalk = function() {
  hexapod.legs.animate({
    duration: 2000,
    cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
    keyFrames: [ ... ],
    onComplete: function() {
      hexapod.walk();
    }
  });
};

hexapod.walk = function() {
  hexapod.legs.animate({
    duration: 2000,
    loop: true,
    cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
    keyFrames: [ ... ]
  });
};

That assumes that animations always run in a certain sequence, and I don't want that to be the case.

We can do it with temporal:

temporal.queue([
  {
    task: function() {
      hexapod.stand();
    }
  },
  {
    delay: 1000,
    task: function() {
     hexapod.preWalk();
    }
  },
  {
    delay: 2000,
    task: function() {
     hexapod.walk();
    }
  }
]);

That's definitely better, but if an animation duration changes (or is scaled) then the temporal delay values are wrong. Plus, you have to define a different queues for different sequences and that could be kind of a PIA.

We could add the ability to chain animation calls in Johnny-Five, but I don't think that adds much value and it makes things harder to read (keyFrames are an array of arrays which totally looks like robot vomit):

hexapod.legs.animate({
  duration: 1000,
  cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
  keyFrames: [ ... ]
}).animate({
  duration: 2000,
  cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
  keyFrames: [ ... ]
}).animate({
  duration: 2000,
  loop: true,
  cuePoints: [0, 0.3, 0.5, 0.7, 1.0],
  keyFrames: [ ... ]
});

I'd really like to see a syntax like this:

hexapod.stand().preWalk().walk();

That's so easy a human could script custom animations from the REPL.

But here's the thing: Those methods are not Johnny-Five methods, they are in the user's code. It'd be easy enough for the user to chain calls but making them run in sequence might be asking a lot of them.

Here's my big idea: I could make it so that when Servo.animate() or ServoArray.animate() is called we automatically append the animation to a queue that is bound to the servo or servoArray. The user could opt out by passing flushQueue: true in the params object (or maybe something easier to spell). If an animation is looping, there are more animations in the queue and the user calls stop() on the servoArray then we would stop the current animation and start the next. For example:

hexapod.stand().preWalk().walk().preWalk().wave().sleep();

Would cause our bot to stand, get into a "ready to walk" position and start walking (the walk animation loops). It would just keep on walking until hexapod.legs.stop() is called. Then it would "automatically" get back into it's ready to walk position, wave and get back into it's sleep position.

Maybe servoArray.stop() should flush the queue instead? I could add servoArray.next() to jump to the next animation in the queue.

Any thoughts on this craziness?

Resseguie commented 10 years ago

I'll spend some time thinking about your options, but I definitely like the idea of being able to compose animations from smaller parts.

rwaldron commented 10 years ago

I know I'm rambling here but it really helps me to "talk things out".

Never apologize for this :)

  1. Animations can be too complex to manage on a single timeline so you may break them into smaller pieces
  2. Animations may depend on earlier animations (I can't start my walk loop unless I'm in a suitable starting position)
  3. We may just want larger sequences that are made up of smaller, distinct animation parts that are useful on their own.

Agreed with all of this.

That's definitely better, but if an animation duration changes (or is scaled) then the temporal delay values are wrong.

By "changes", do you mean that the animation duration is changed by hand then I have to update the delays in temporal? I've been using a system that builds temporal queues dynamically:

// Note: "duration" refers to the duration for EACH move set in the
// sequence. The "total duration" would be duration * sequence.length
var defs = [
  {
    id: "lean",
    sets: [
      {
        id: "right",
        isLoop: false,
        isInterruption: true,
        duration: 650,
        sequence: [
          { moves: [
            // f  a  k  t  r  y
              18, -35, -62, -32, 18, 0,
              32, -35, -62, -32, 32, 0,
            ]
          }
        ]
      },
      {
        id: "left",
        isLoop: false,
        isInterruption: true,
        duration: 650,
        sequence: [
          { moves: [
            //f  a  k  t  r  y
              -32, -35, -62, -32, -32, 0,
              -18, -35, -62, -32, -18, 0
            ]
          }
        ]
      },
    ]
  },
  {
    id: "lift",
    sets: [
      {
        id: "right",
        isLoop: false,
        isInterruption: true,
        duration: 650,
        sequence: [
          // lean and lift
          { moves: [
              -17, 0, 0, 10, -12, 0,
              -20, 0, 0, 10,   2, 0
            ]
          },

          { moves: [
              -17, 25, 40, 10, -12, 0,
              -20,  0,  0, 10,   2, 0
            ]
          }
        ]
      },
      {
        id: "left",
        isLoop: false,
        isInterruption: true,
        duration: 650,
        sequence: [
          // lean and lift
          { moves: [
              17,  0,  0, 10,  2, 0,
              20,  0,  0, 10, 12, 0
            ]
          },

          { moves: [
              17,  0,  2, 10,  2, 0,
              20, 25, 40, 10, 12, 0
            ]
          }
        ]
      },
    ]
  },
  {
    id: "home",
    sets: [
      {
        id: "arabesque",
        isLoop: false,
        isInterruption: true,
        duration: 750,
        sequence: [
          { moves: [ 17, -19, -17, 10, 2, 0, 20, 11, 40, 10, 12, 0 ] },
          { moves: [ 17, -19, -12, -1, 2, 0, 20, -22, 21, 48, 12, 0 ] },
          { moves: [ 17, -19, -1, 8, 2, 0, 20, 35, -51, 48, 12, 0 ] },
          { moves: [ 17, -33, -49, -37, 2, 0, 20, 35, -51, 48, 12, 0 ] }
        ]
      },
      {
        id: "stand",
        isLoop: false,
        isInterruption: true,
        duration: 750,
        sequence: [
          { moves: [
              -8, 0, 0, 0, 0, 0,
               5, 0, 0, 0, 2, 0,
            ]
          }
        ]
      },
      {
        id: "rest",
        isLoop: false,
        isInterruption: true,
        duration: 500,
        sequence: [
          { moves: [
            //f   a   k   t  r  y
              null, 30, 35,  null, null, null,
              null, 30, 35,  null, null, null
            ]
          },
          { moves: [
            //f   a   k   t  r  y
              0, 62, 70,  5, 0, 0,
              0, 62, 70,  5, 0, 0
            ],
            duration: 500
          }

        ]
      }
    ]
  }
];

// Sometime later...
// assume there is a `servos` object in scope

// I believe this gets replaced by calls to ServoArray.animate
function move(servos, positions, timeToMove) {
  servos.forEach(function(part, i) {

    if (positions[i] !== null) {
      if (argv.log) {
        console.log(
          "Moving %s from %d => %d", part, servos[part].value, positions[i]
        );
      }
      servos[part].to(positions[i], timeToMove);
    } else {
      if (argv.log) {
        console.log(
          "Holding %s at %d", part, positions[i]
        );
      }
    }
  });
}

var controls = defs.reduce(function(accum, gait) {
  accum[gait.id] = gait.sets.reduce(function(seq, set) {

    // Create an API from gait.set
    seq[set.id] = function(opts, callback) {
      var noLoop;

      if (typeof opts === "function") {
        callback = opts;
        opts = {};
      }

      opts = opts || {};

      noLoop = opts.noLoop;

      var hash = [gait.id, set.id].join(":");

      if (set.isInterruption) {
        controls.current.queue.stop();
        servos.stop();
      }

      // Allow interruptions
      if (controls.current.name !== hash) {
        controls.current.name = hash;
        controls.current.isMoving = false;
      }

      // If `controls.current.isMoving` is currently `true`,
      // bail out to avoid pile up.
      if (controls.current.isMoving) {
        return;
      }

      // If `controls.current.isMoving` is currently `false`,
      // set it to `true` to avoid pile up
      controls.current.isMoving = true;

      if (opts.prerequisite) {
        set.sequence.unshift(prerequisite);
        /*
        Any sequence record, eg.
        {
          moves: [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
          duration: 500
        }
        */
      }

      var queue = set.sequence.map(function(relative) {
        var duration = set.duration;
        var absolute;

        // A relative duration overrides the set's duration
        if (relative.duration) {
          duration = relative.duration;
        }

        // Convert the relative movement values
        // to absolute servo positions.
        absolute = relative.moves.map(function(value, i) {
          return value === null ?
            null : value + servos[i].startAt;
        });

        return {
          wait: duration,
          task: function() {
            if (absolute.length) {
              move(servos, absolute, duration);
            }
          }
        }
      });

      // Allows overriding the default loop behaviour
      if (set.isLoop || opts.isLoop) {
        queue.push({
          wait: 10,
          task: function() {
            controls.current.isMoving = false;

            if (set.callback) {
              set.callback();
            }

            if (callback) {
              callback(set.id);
            } else {
              controls[gait.id][set.id]();
            }
          }
        });
      } else {
        queue.push({
          wait: 10,
          task: function() {
            controls.current.isMoving = false;

            // sets can have their own completion callback
            if (set.callback) {
              set.callback();
            }

            // User code may define a completion callback
            if (callback) {
              callback(set.id);
            }
          }
        });
      }

      // Expose this task to the rest of the program.
      controls.current.queue = temporal.queue(queue);
    };

    // Define some meta data as an expando property of
    // the control api we've just defined.
    //
    seq[set.id].meta = {
      sequences: set.sequence.length,
      duration: set.duration * set.sequence.length
    };

    return seq;
  }, {});

  return accum;
}, Object.create(null));

controls.current = {
  isMoving: false,
  queue: null,
  name: ""
};

// I created this as a way of joining sets of tasks for demoing at empirejs:

function chain(tasks) {
  temporal.queue(
    tasks.map(function(task) {
      return {
        delay: task.meta.duration,
        task: function() {
          task();
        }
      };
    })
  );
}

// This is what an example "chain" looks like:

function ballet() {
  chain([
    controls.commands.home.rest,
    controls.commands.home.stand,
    controls.commands.lean.right,
    controls.commands.lift.left,
    controls.commands.home.arabesque
  ]);
}

// Of course, those could/should be aliased.

This is obviously too complicated to be widely successful, but maybe it's useful as additional perspective?

rwaldron commented 10 years ago

Some of the thinking behind the defs object was that I'd like to store that in JSON, but I'm not hell-bent on this

dtex commented 10 years ago

Yes, I was thinking of changes in duration made to the animation. I was also thinking of changes to the speed of the playback head that would apply system wide and stretch/compress the animation realDuration = duration / playbackSpeed.

It's cool to see how we each encountered and dealt with the same problems. Looking at the two I focused on the animation definition objects as those are going to have the most surface area for users. Here's how our two schemes look when given the same animation:

...
{
  id: "arabesque",
  isLoop: false,
  isInterruption: true,
  duration: 750,
  sequence: [
    { moves: [ 17, -19, -17, 10, 2, 0, 20, 11, 40, 10, 12, 0 ] },
    { moves: [ 17, -19, -12, -1, 2, 0, 20, -22, 21, 48, 12, 0 ] },
    { moves: [ 17, -19, -1, 8, 2, 0, 20, 35, -51, 48, 12, 0 ] },
    { moves: [ 17, -33, -49, -37, 2, 0, 20, 35, -51, 48, 12, 0 ] }
  ]
},
...

becomes this in mine

...
// scout is a servoArray
var arabesque = scout.animate({
  duration: 3000,
  cuePoints: [0, 0.25, 0.5, 0.75, 1.0],
  keyFrames: [
    [ null, null, null, null, {step: 68}],
    [ null, null, null, {step: -57}, {step: -33}],
    [ null, {step: -17}, {step: -12}, {step: -1}, {step: -49}],
    [ null, {step: 10}, {step: -1}, {step: 8}, {step: -37}],
    [ null, null, null, null, {step: 8}],
    [ null ],
    [ null, null, null, null, {step: 80}],
    [ null, {step: 11}, {step: -22}, null, {step: 70}],
    [ null, {step: 40}, {step: 21}, null, {step: -102}],
    [ null, {step: 10}, null, null, {step: 144}],
    [ null, null, null, null, {step: 48}],
    [ null ]
  ]
});
...

The fundamental differences I can see in our animation definitions are:

I think a hybrid could end up with a definition like this:

...
{
  duration: 3000,
  cuePoints: [0.25, 0.5, 0.75, 1.0],
  keyFrames: [
    [ ,,, 68 ],
    [ ,, -57, -33 ],
    [ -17, -12, -1, -49 ],
    [ 10, -1, 8, -37 ],
    [ ,,, 8 ],
    null,
    [ ,,, 80 ],
    [ 11, -22,, 70 ],
    [ 40, 21,, -102 ],
    [ 10,,, 144 ],
    [ ,,, 48 ],
    null
  ]
}
...

or swapping the keyFrames axes

...
{
  duration: 3000,
  cuePoints: [0.25, 0.5, 0.75, 1.0],
  keyFrames: [
    [,,-17,10,,,,11,40,10,,,],
    [,,-12,-1,,,,-22,21,,,,],
    [,-57,-1,8,,,,,,,,,],
    [68,-33,-49,-37,8,,80,70,-102,144,48,]
  ]
}
...

It gets more verbose when you start easing between keyFrames. Here's my ugliest case (wave):

...
{
  duration: 2000,
  cuePoints: [0.1, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0], 
  keyFrames: [
    [0, { degrees: 120, easing: inOut }, 0, 0, 0, 0, 0, { degrees: 52, easing: inOut }, [0] ],
    [{ step: -25, easing: inOut }, { step: 60, easing: inOut }, 0, 0, 0, 0, 0, { step: -35, easing: inOut }, [0] ],
    [{ degrees: 85, easing: inOut }, { degrees: 45, easing: inOut }, { step: -15, easing: inOut}, { step: 30, easing: inOut}, [3], [4], [3], [1], {degrees:[0], easing: inOut} ],
    null,
    null,
    null,
    null,
    null,
    null,
    null,
    [{ step: 25, easing: inOut }, 0, 0, 0, 0, 0, 0, 0, {degrees: [0], easing: inOut} ],
    [{ step: 17, easing: inOut }, { degrees: 120, easing: inOut }, 0, 0, 0, 0, 0, 0, {degrees: [0], easing: inOut} ],
  ]
}

Well, that's enough of a brain dump for tonight.

rwaldron commented 10 years ago

Eeeeeek, no holes! (that is, those icky empty slots in arrays)

I actually like this alot:

// scout is a servoArray
var arabesque = scout.animate({
  duration: 3000,
  cuePoints: [0, 0.25, 0.5, 0.75, 1.0],
  keyFrames: [
    [ null, null, null, null, {step: 68}],
    [ null, null, null, {step: -57}, {step: -33}],
    [ null, {step: -17}, {step: -12}, {step: -1}, {step: -49}],
    [ null, {step: 10}, {step: -1}, {step: 8}, {step: -37}],
    [ null, null, null, null, {step: 8}],
    [ null ],
    [ null, null, null, null, {step: 80}],
    [ null, {step: 11}, {step: -22}, null, {step: 70}],
    [ null, {step: 40}, {step: 21}, null, {step: -102}],
    [ null, {step: 10}, null, null, {step: 144}],
    [ null, null, null, null, {step: 48}],
    [ null ]
  ]
});

Quick notes:

You recognize Number values as degrees to move.

Actually, it's number of degrees to move from "center", which is generally the starting position of the servo.

(If I change this to undefined it will be more concise and arguably easier to read)

Holes are figuratively the devil, let's just use null—I will sleep better at night.

duration: 3000, cuePoints: [0, 0.25, 0.5, 0.75, 1.0],

cuePoints are fractions of a second? Should this add up to duration?

I'm super tired, this is all I can think of at the moment

Resseguie commented 10 years ago

cuePoints are fractions of a second? Should this add up to duration?

cuePoints are defined as relative points from 0-1, then actual cue point times are calculated based on the duration, fps, speed, etc. (Off the top of my head I forget the various options @dtex set up for scaling the duration.)

dtex commented 10 years ago

@Resseguie I was just going to give the user an option to update the overall playback speed for all animations. It would just be a scaling factor so that the whole bot could be sped up or slowed down.

@rwaldron Dave's right on the cuePoints. It gets a little weird when you start factoring in fps. Take a look at this intentionally awkward example (we wouldn't use fps: 11).

{
  duration: 500,
  fps: 11,
  cuePoints: [0.0, 0.5, 0.7, 1.0],
  ...
}

Our cuePoints get set to 0ms, 250ms, 350ms and 500ms

Our fps results in the servo position being updated at 0ms, 91ms, 182ms, 273ms, 364ms, 455ms and 546ms

The last write at 546ms uses the value at 1.0

dtex commented 10 years ago

Eeeeeek, no holes! (that is, those icky empty slots in arrays)

I'm glad you said this. I was golfing the animation definition and got carried away. The other thing that doesn't "feel" right is the single element array referring to another element in the array trick. In case it's not clear, here's a simple example of what it does:

In this keyframe set assume that both servo.last.degress === 90:

keyFrames: [
  [ null, { degrees: 150 }, -20, [1], [0]], // servo A
  [ null, 50, [0], [1], [0]] // servo B
]

becomes

keyFrames: [
  [ 90, 150, 130, 150, 90],
  [ 90, 50, 90, 50, 90]
]

I like it because I can return a servo to its starting position even after setting a fixed degrees value... it just seems kind of obtuse. As an alternative I could let the user use a string ("start", "previous" or "n" where n is a keyFrame index). The keyframes are all normalized to degrees when the animation object is instantiated so we are not dealing with this on every update.

The other thing I'd like input on is expecting a value for the 0 cuePoint (it's implicit in your examples and explicit in mine). Even though I haven't actually overridden last.degrees in any of my animations, the option to do so is there and it gives a tangible representation of what's happening in the early parts of the animation.

I actually like this alot:

:-) Me too. Here's my punch list:

Resseguie commented 10 years ago

@dtex for the key frame index could you use a special option instead of the single array trick? More typing but maybe more obvious. Something like the following, though I'm not sure the best name.

{frame: "previous"}

or

{frame: 1} On Jun 8, 2014 12:41 AM, "Donovan Buck" notifications@github.com wrote:

Eeeeeek, no holes! (that is, those icky empty slots in arrays)

I'm glad you said this. I was golfing the animation definition and got carried away. The other thing that doesn't "feel" right is the single element array referring to another element in the array trick. In case it's not clear, here's a simple example of what it does:

In this keyframe set assume that both servo.last.degress === 90:

keyFrames: [ [ null, { degrees: 150 }, -20, [1], [0]], // servo A [ null, 50, [0], [1], [0]] // servo B ]

becomes

keyFrames: [ [ 90, 150, 130, 150, 90], [ 90, 50, 90, 50, 90] ]

I like it because I can return a servo to its starting position even after setting a fixed degrees value... it just seems kind of obtuse. As an alternative I could let the user use a string ("start", "previous" or "n" where n is a keyFrame index). The keyframes are all normalized to degrees when the animation object is instantiated so we are not dealing with this on every update.

The other thing I'd like input on is expecting a value for the 0 cuePoint (it's implicit in your examples and explicit in mine). Even though I haven't actually overridden last.degrees in any of my animations, the option to do so is there and it gives a tangible representation of what's happening in the early parts of the animation.

I actually like this alot:

:-) Me too. Here's my punch list:

  • Switch to Number defaulting to step instead of degrees
  • Allow anonymous functions as keyFrame elements
  • Figure out that keyFrame cross referencing thing above
  • Decide on the 0 cuePoint issue
  • Chaining animations
  • Lint / Make code less crappy
  • Add docs

— Reply to this email directly or view it on GitHub https://github.com/rwaldron/johnny-five/issues/304#issuecomment-45428252 .

dtex commented 10 years ago

@Resseguie Of course, that's perfect! How about copyFrame: 1 which will get the frame with all of its properties and copyDegrees: 1 which will do the same but use the calculated degrees value from that frame?

Resseguie commented 10 years ago

And after thinking about it, string values like "previous" and "first" don't make much sense here. First is just 0. Previous is just a null operation. Unless "previous" actually stacked and backtracked multiple steps so you could build a palindrome of actions. But that just seems overly complicated for anyone (me!) to understand exactly what is intended for it to do. I think I like the simple option (or two) with a key frame index as the value.

rwaldron commented 10 years ago

In this keyframe set assume that both servo.last.degress === 90:

keyFrames: [
  [ null, { degrees: 150 }, -20, [1], [0]], // servo A
  [ null, 50, [0], [1], [0]] // servo B
]

becomes

keyFrames: [
  [ 90, 150, 130, 150, 90],
  [ 90, 50, 90, 50, 90]
]

The nulls are exactly what I would've guessed, but is the single element array meant to be a shorthand? In every case it's more characters to type.

Also, how does -20 become 130 from 90?

dtex commented 10 years ago

is the single element array meant to be a shorthand?

I think the single element array is dead. It was a shorthand for referencing other elements in the same array but @Resseguie had a much better solution.

In every case it's more characters to type.

Yes, but it gives us two things.

  1. You may not know the start value in advance but this gives us the ability to go back to that value even if we've used an absolute position in our animation. I use this so my hexapod can wave from any of its five standing positions, and return to that position in the last frame.
  2. You can set a position on a frame and then reference that frame later on. In animations where you are having to go back to the same value over and over, you can change it in one place to update the position everywhere. Very helpful when you are tweaking an animation.

how does -20 become 130 from 90?

There's a disconnect on the Number values. I think you are seeing them as degrees from a static home position 90. I've been reading them as a step value from the previous frame. 150 - 20 = 130

rwaldron commented 10 years ago

You may not know the start value in advance but this gives us the ability to go back to that value even if we've used an absolute position in our animation. I use this so my hexapod can wave from any of its five standing positions, and return to that position in the last frame.

I think I follow this

having to go back to the same value over and over, you can change it in one place to update the position everywhere. Very helpful when you are tweaking an animation.

Ah, this is compelling.

There's a disconnect on the Number values. I think you are seeing them as degrees from a static home position 90. I've been reading them as a step value from the previous frame. 150 - 20 = 130

Oh my, yes... that was it. I need to recondition my brain.

dtex commented 10 years ago

Not a question... just something I thought I'd share. Nested Servo.Arrays just work. I had to edit about four lines of my Animation code, but now the instance that makes my quadriped stand has gone from this:

// ph.legs is a Servo.Array with 12 servos
ph.legs.animate({
  duration: 500,
  cuePoints: [0, 0.5, 1.0],
  keyFrames: [
    [null, false, { degrees: 45 }],
    [null, { degrees: 136, easing: out }, { degrees: 180, easing: out }],
    [null, { degrees: 120 }, 60],
    [null, false, { degrees: 45 }],
    [null, { degrees: 136, easing: out}, {degrees: 180, easing: out}],
    [null, { degrees: 120 }, 60],
    [null, false, { degrees: 45 }],
    [null, { degrees: 136, easing: out}, {degrees: 180, easing: out}],
    [null, { degrees: 120 }, 60],
    [null, false, { degrees: 45 }],
    [null, { degrees: 136, easing: out}, {degrees: 180, easing: out}],
    [null, { degrees: 120 }, 60]
  ]
});

To this:

// ph.joints as a 3 element array of Servo.Arrays, each with four servos 
ph.joints.animate({
  duration: 500,
  cuePoints: [0, 0.5, 1.0],
  keyFrames: [
    [null, false, { degrees: 45 }],
    [null, { degrees: 136, easing: out }, { degrees: 180, easing: out }],
    [null, { degrees: 120 }, 60]
  ]
});

I'm digging the undocumented awesomeness.

Resseguie commented 10 years ago

So useful anytime you have several identical "limbs" that need to be moved identically? Where else is it useful?

I feel a new servo example coming on.

rwaldron commented 10 years ago

I'm digging the undocumented awesomeness.

:smile: :frowning:

We should fix that too

Btw, what does false signify?

Resseguie commented 10 years ago

From @dtex:

I have a series of keyFrame shortcuts. They are not mandatory, they are just an optional shorthand: ... If I find false in an array element I copy the nearest degrees on the left (or step: 0)

Though I'm not sure exactly what that means. :)

dtex commented 10 years ago

I think this might help clear things up.

screen shot 2014-06-10 at 12 46 18 pm The Y axis is degrees, the X axis is time 0.0 - 1.0

Servo 1 uses false at 0.5. False means make no movement between the prior cuePoint and this one, then use the prior keyFrames value for tweening between 0.5 and 1.0. In other words, "hold still" until this frame has passed.

Servo 2 uses null at 0.5. Null means we should tween between the first frame to the left that has a valid value and the first frame to the right that has a valid value. In other words, ignore this cuePoint for this servo.

Servo 3 uses an explicit value at 0.5

I realize this all seems like a lot to learn, but a nice wiki article called "How to Move a Servo" would be helpful. It could start with Servo.to(deg), move on to Servo.to(deg, duration) and then build one feature/property/option at a time until we arrive at "passing functions the leverage Vektor as keyframe values" to animations that use nested servoArray()'s with null, false, copyFrame, copyValue, easing, right side keyFrame padding, null start values and anything else we add.

Resseguie commented 10 years ago

@dtex thanks, that helps and make sense. I know it's more verbose, but is this another good example of where a "hold still" option would make sense to avoid confusion?

{degree: "hold"}

or

{hold: true}

Or maybe even something like {tweening: null} or {easing: null}?

Actually, now that I think about it, this is where your original idea of {copyFrame: "previous"} would actually apply.

Maybe one that represents null as well? {ignoreCuepoint: true}

Of course you likely have better names. Just thinking out loud here. You've thought about this a lot more. Feel free to ignore if I'm not making sense or making this too convoluted.

dtex commented 10 years ago

{copyDegrees: n} Where n is one less than the current frame's index is equivalent to false.

{ignore: true} Would be a good alternative to null. I'd like to keep these two (and only these two) shorthand options in there because they come up a lot.

dtex commented 10 years ago

I've spent the past few days noodling over animation "chaining" and I think I've finally got it ironed out. I just want to run it by y'all and make sure I'm not falling into any anti-patterns.

I want the user to add their methods directly to the servo or servoArray object. This seems wrong and oh so right at the same time. The advantage is we can simply return this from all those user methods and the chaining of animations just happens. The risk is that a user could overwrite a Johnny-Five defined method or property. For example:

// legs is a Servo.Array

// legs.stand(), legs.walk() and legs.sit() are user defined methods
legs.stand = function() {
  this.animate(opts);
  return this;
};

legs.walk = function() {
  this.animate(opts);
  return this;
};

legs.sit = function() {
  this.animate(opts);
  return this;
};

legs.stand().walk().sit();

If a user defined legs.step() we'd have a problem.

Chaining animation commands (or calling additional animation commands later) simply adds the animations to a queue (a la the jQuery effects queue). When an animation completes we check the queue and see if there are any more animations in line. If so, run the next one. No user defined callbacks are required.

We add methods play(), pause(), stop() and next() to the servo and servo.array prototypes. play() starts an animation or resumes playing a paused animation. pause() halts the progress of the animation but keeps the current playback position and animation queue intact. stop() stops an animation immediately and flushes the animation queue. next() stops the current animation and moves on to the next animation in the queue.

A chained animation that has loop: true in the opts will continue to run until next() is called.

onStop can be added to animation opts. It is a function that executes when the user/program calls stop(). It does not execute when an animation completes naturally. This will be useful for returning robots to a "home" or "ready" position.

rwaldron commented 10 years ago

On Friday, June 20, 2014, Donovan Buck notifications@github.com wrote:

I've spent the past few days noodling over animation "chaining" and I think I've finally got it ironed out. I just want to run it by y'all and make sure I'm not falling into any anti-patterns.

I want the user to add their methods directly to the servo or servoArray object. This seems wrong and oh so right at the same time. The advantage is we can simply return this from all those user methods and the chaining of animations just happens. The risk is that a user could overwrite a Johnny-Five defined method or property. For example:

// legs is a Servo.Array

// legs.stand(), legs.walk() and legs.sit() are user defined methods legs.stand = function() { this.animate(opts); return this; };

legs.walk = function() { this.animate(opts); return this; };

legs.sit = function() { this.animate(opts); return this; };

legs.stand().walk().sit();

If a user defined legs.step() we'd have a problem.

Once we open this box, we can't close it—I know it seems desirable at the moment, but I don't like the long term implications.

Also, what does this mean:

legs.sit().walk().stand();
legs.sit();

Does the bot skip right to "sit"?

Chaining animation commands (or calling additional animation commands

later) simply adds the animations to a queue (a la the jQuery effects queue). When an animation completes we check the queue and see if there are any more animations in line. If so, run the next one. No user defined callbacks are required.

We add methods play(), pause(), stop() and next() to the servo and servo.array prototypes.

But these are meaningless to servo objects that don't have any animations—shouldn't they be methods of an "animation" object (whatever that is)?

play() starts an animation or resumes playing a paused animation. pause() halts the progress of the animation but keeps the current playback position and animation queue intact. stop() stops an animation immediately and flushes the animation queue. next() stops the current animation and moves on to the next animation in the queue.

A chained animation that has loop: true in the opts will continue to run until next() is called.

onStop can be added to animation opts. It is a function that executes when the user/program calls stop(). It does not execute when an animation completes naturally. This will be useful for returning robots to a "home" or "ready" position.

I dislike camel cased event handler names—the DOM doesn't do this, it's no fun when something doesn't work b/c wrong casing. Lowercase please.

I'm on vacation until the end of next week and won't have regular access, so please be patient if my feedback is slow going until then.

— Reply to this email directly or view it on GitHub https://github.com/rwaldron/johnny-five/issues/304#issuecomment-46640845 .

dtex commented 10 years ago

First off, focus on your vacation. A response can wait.

Once we open this box, we can't close it—I know it seems desirable at the moment, but I don't like the long term implications.

You're right of course... dammit.

Also, what does this mean:

legs.sit().walk().stand();
legs.sit();

Does the bot skip right to "sit"?

No, in that case the second sit() would be pushed onto the animation queue and would run after stand() has completed.

But these are meaningless to servo objects that don't have any animations—shouldn't they be methods of an "animation" object (whatever that is)?

Good point. Thank you for bringing me back to the API surface.

I need to be able to define normalize and render functions that are specifically for Servo and I don't want to put servo specific code into the animation class. Otherwise, I could just call new five.Animation(opts) and not even touch the Servo class.

I think I've got it now.

var myServo = new five.Servo(11);
var myAnimation = myServo.animate(opts); // Returns the Animation object, not the Servo object

// With chaining
var myAnimation = myServo.animate(standOpts).enqueue(walkOpts).enqueue(sitOpts);

// Pushing an animation segment onto an existing queue
myAnimation.enqueue(waveOpts);

// method call
myAnimation.pause();
dtex commented 10 years ago

I didn't like that the method returns a different object. This is better:

var myServo = new five.Servo(11); // normalize and render methods are on this prototype
var myAnimation = new five.Animation(myServo); // Could also be a servoArray
var stand = { /* animation opts (duration, keyFrames, cuePoints...) */ };
var walk = { /* animation opts (duration, keyFrames, cuePoints...) */ };
var sit = { /* animation opts (duration, keyFrames, cuePoints...) */ };

Define and enqueue segments with chaining

myAnimation.enqueue(stand).enqueue(walk).enqueue(sit);

Or pass an array of segments

myAnimation.enqueue([stand, walk, sit]);

Animation methods:

Animation properties:

rwaldron commented 10 years ago

No, in that case the second sit() would be pushed onto the animation queue and would run after stand() has completed.

This is the answer I hoped for, but I'm also wondering now about the appearance of synchronous execution.

var myAnimation = myServo.animate(opts); // Returns the Animation object, not the Servo object

I like where this is headed, but I agree with your follow up thoughts, specifically:

(Of course you already came to the same conclusion, I'm just documenting the progress of decision making :D)

Here's my suggestion, which is only a very small surface change from your follow up:

var myServo = new five.Servo(11);
var myAnimation = new five.Servo.Animation(myServo, opts); 
// Returns the Animation object, not the Servo object
// Omitting `opts` will invoke some kind of sensible no-op defaults

The idea is that new five.Servo.Animation holds the special servo-specific ingredients for creating an animation object bound to a servo object. What do you think?

dtex commented 10 years ago

I'm good with that. I can't think of any options that make sense without an animation segment so I don't think there is a need for the opts parameter here. We would set defaults when the animation instance is created and those could be overridden by values in Animation.enqueue( segmentOptions );. Only a few animation properties would persist between animation segments. For example a segment could pass a new playback speed and that would affect the entire animation, but the duration would always be segment specific.

rwaldron commented 10 years ago

:+1:

dtex commented 10 years ago

Wrapping this up and planning for the future I've realized something regarding this suggestion:

var myServo = new five.Servo(11);
var myAnimation = new five.Servo.Animation(myServo, opts); 
// Returns the Animation object, not the Servo object
// Omitting `opts` will invoke some kind of sensible no-op defaults

I've been focused on servos, but there is no reason that an animation need consist of only servos. We could also have LED's, continuous rotation servos, motors, steppers, etc. all working together in a single animation. I think forcing other device types into a new five.Servo.Animation would be kind of weird: var foo = new Five.Servo.Animation([myServo, myStepperMotor], opts);

With this in mind, I like having a separate class for Animation(). Each device class would still have it's own normalizeKeyFrames and renderFunction methods that are used by Animation() but animation would be its own, separate class.

var myServo = new five.Servo(11);
var myStepper = new five.Stepper({ ... options ... });
var myAnimation = new five.Animation([myServo, myStepper]);

I have to write one last test, tune up my comments, code review, squash and then I'll submit a PR. The hexapod walk and turn routines are looking really good. My dog hates that thing.

rwaldron commented 10 years ago

Looking forward to this :)

dtex commented 10 years ago

Handled by #411