Servo animation enqueue is buggy

[ScreamZ]

@dtex @rwaldron Hi I'm trying to sequence animations on 3 servos using this code:

const sequence = new Animation(new Servos([coinReceiverServo, robotHeadServo, robotEyeServo]));

          sequence.enqueue({
            duration: 100,
            keyFrames: [[null, { degrees: 0 }], [null], [null]],
            onstart: () => console.log("1  start"),
            oncomplete: () => console.log("1 end"),
          });

          sequence.enqueue({
            duration: 5000,
            keyFrames: [[null, { degrees: 179 }], [null], [null]],
            onstart: () => console.log("2 start"),
            oncomplete: () => console.log("2 end"),
          });

          sequence.enqueue({
            duration: 1500,
            keyFrames: [[null], [null, { degrees: 180 }], [null]],
            onstart: () => console.log("3 start"),
            oncomplete: () => console.log("3 end"),
          });

Using this code, 3 never starts… I don't get why.

Have a look at #1376 but still having issues.

Also, it's impossible to pause the sequence before something has been enqueued.

Any idea?

[dtex]

So you see "2 end" but never "3 start" correct? No errors thrown?

[ScreamZ]

So you see "2 end" but never "3 start" correct? No errors thrown?

That is correct, I can give you more insight if you wish. For some reason, I've added some console logs inside animation.js file and I can see that this.segments.length is going to 1 to 0 then to 2 then directly to 0. I'm trying to investigate.

I suspect some kind of race conditions.

I think this could also be nice to buffer things before everything starts. Adding an autoStart property might be a good choice.

If I can contribute, just let me know. I'm also doing a big work updating typescript definition typings https://github.com/DefinitelyTyped/DefinitelyTyped/pull/67063

[ScreamZ]

@dtex Any idea on that ?

[dtex]

I don't know if it's a race condition, or we're just popping things off the queue too aggressively. I'll set up a test and see if I can find where that's happening.

[ScreamZ]

I don't know if it's a race condition, or we're just popping things off the queue too aggressively. I'll set up a test and see if I can find where that's happening.

Alright thank you very much, Keep me in touch, I would be really pleased because i need this feature, I'm trying debugging on my side.

[ScreamZ]

@dtex Issue happen on https://github.com/rwaldron/johnny-five/blob/094bf6cceb8b9ec424306da8e98308ebd6fa2252/lib/animation.js#L94

Added this code

Constructor

```ts class Animation extends Emitter { constructor(target) { super(); // Necessary to avoid loading temporal unless necessary if (!temporal) { temporal = require("temporal"); } console.log("Before constructor assign", this.segments?.length) Object.assign(this, new Animation.Segment()); console.log("After constructor assign", this.segments?.length) this.defaultTarget = target || {}; } ``` Output is ``` worker:dev: [dev] Before constructor assign undefined worker:dev: [dev] opts [] worker:dev: [dev] After constructor assign 0 ``` This is initializing the array to 0, which is fine.

Segments shifting in loop

```ts console.log("Before shift", this.segments.length) const firstElement = this.segments.shift() console.log("After Shift",this.segments.length) Object.assign(this, new Animation.Segment(firstElement)); console.log("After assign",this.segments.length) ``` Output is ``` worker:dev: [dev] Before shift 1 worker:dev: [dev] After Shift 0 worker:dev: [dev] opts [ 'duration', 'keyFrames', 'onstart', 'oncomplete', 'target' ] worker:dev: [dev] After assign 0 worker:dev: [dev] enqueue 1 worker:dev: [dev] enqueue 2 worker:dev: [dev] enqueue 3 worker:dev: [dev] enqueue 4 worker:dev: [dev] loopFunction 4 worker:dev: [dev] next 4 worker:dev: [dev] Before shift 4 worker:dev: [dev] After Shift 3 worker:dev: [dev] opts [ 'duration', 'keyFrames', 'onstart', 'oncomplete', 'target' ] worker:dev: [dev] After assign 0 worker:dev: [dev] loopFunction 0 worker:dev: [dev] Stop called ``` It's `1` because the first instruction is queued, then `1` again because i have a total of 5 enqueued tasks. So when the first trigger 0, the other become 1,2,3,4. `stop` is called, so it pops out the `this.segments` array. `loopFunction` console.log is from there and `next` is from `next` method. ```ts if ((this.progress === 1 && !this.reverse) || (progress === this.loopback && this.reverse)) { if (this.loop || (this.metronomic && !this.reverse)) { if (this.onloop) { this.onloop(); } if (this.metronomic) { this.reverse = this.reverse ? false : true; } this.normalizeKeyframes(); this.progress = this.loopback; this.startTime = Date.now() - this.scaledDuration * this.progress; this.endTime = this.startTime + this.scaledDuration; } else { this.isRunning = false; if (this.oncomplete) { process.nextTick(() => this.oncomplete()); } console.log("loopFunction",this.segments.length) if (this.segments.length > 0) { process.nextTick(() => this.next()); } else { this.stop(); } ```

FIXES

I've been able to fix the code doing this

Object.assign(this, new Animation.Segment(firstElement), {segments: this.segments});

We need to re-apply segments otherwise we crush the segment array each time. Or we need to pass current second to options so options.segments can trigger inside Animation.Segment

I can make a pull request if you think it's okay.