Roland DJ-505: Use new ControlIndicator COs for blinking lights

[Holzhaus]

Based on #4157. Didn't test on real hardware yet, will do once the other PR is merged. Tested, works.

This should make all blinking LEDs that use the same interval light up at the same time, not shifted depending on when the timer was created.

[coveralls]

Pull Request Test Coverage Report for Build 1087482160

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 26.027%

---

Totals
Change from base Build 1087078133:	0.02%
Covered Lines:	20015
Relevant Lines:	76902

---

💛 - Coveralls

[Swiftb0y]

@Holzhaus could you rebase now that the prerequisite has been merged?

[Holzhaus]

@Holzhaus could you rebase now that the prerequisite has been merged?

Done.

[Holzhaus]

Now they are actually in sync. Here's an explicit leader track that is stopped and not on the cue point. Before, they were not blinking at the same time, despite their interval being the same.

[Swiftb0y]

Thanks, small nitpick, LGTM otherwise.

[Swiftb0y]

Now that we have ES7 support, we should phase out the function expression + .bind(this) pattern and use arrow functions instead IMO.

[Holzhaus]

Arrow functions are harder to read IMHO, but that aside, when I apply this patch:

+++ b/res/controllers/Roland_DJ-505-scripts.js
@@ -1642,7 +1642,7 @@ DJ505.SavedLoopMode = function(deck, offset) {
         output: function(value, _group, _control) {
             this.stopBlinking();
             if (value === 2) {
-                this.connections[2] = engine.makeConnection("[Master]", "indicator_250millis", function(value, _group, _control) {
+                this.connections[2] = engine.makeConnection("[Master]", "indicator_250millis", (value, _group, _control) => {
                     var colorValue = this.colorMapper.getValueForNearestColor(
                         engine.getValue(this.group, this.colorKey));
                     if (value) {
@@ -1650,7 +1650,7 @@ DJ505.SavedLoopMode = function(deck, offset) {
                     } else {
                         this.send(colorValue + DJ505.PadColor.DIM_MODIFIER);
                     }
-                }.bind(this));
+                });
             } else if (value === 1) {
                 var colorValue = this.colorMapper.getValueForNearestColor(
                     engine.getValue(this.group, this.colorKey));

I get:

Uncaught exception at line 1649 in file file:///home/jan/Projects/mixxx/res/controllers/Roland_DJ-505-scripts.js.

Exception:
  TypeError: Property 'send' of object [object Object] is not a function

Backtrace:
@file:///home/jan/Projects/mixxx/res/controllers/Roland_DJ-505-scripts.js:1649

[Swiftb0y]

Thats strange. I'll try to reproduce.

[Swiftb0y]

Arrow functions are harder to read IMHO

I disagree, they're just like closures from most other functions (C++ lambdas, Rust Fn, etc). And they get rid of the .bind(this) that most people without indepth JS knowledge don't understand.

[Holzhaus]

Arrow functions are harder to read IMHO

I disagree, they're just like closures from most other functions (C++ lambdas, Rust Fn, etc). And they get rid of the .bind(this) that most people without indepth JS knowledge don't understand.

I'd argue that people without indepth JS knowledge wouldn't even recognize that foo => bar is a function. I think function(foo) { return bar; } is much clearer to a beginner. Just my 2 cents.

[Swiftb0y]

it depends, I think function expressions are being thought quite early now especially with the rising popularity of functional programming and APIs that use callbacks.

[Swiftb0y]

I tried to reproduce the issue but I was unable to:

let Foo = function () {
    components.Button.call(this);
}
Foo.prototype = new components.Button({
    init: function() {
        engine.makeConnection("[Channel1]", "play", (value, group, control) => {
            print(typeof this.send);
        });
    }
});

var MyController = new Foo();

Obviously the example is a bit atypical but the point is the same.

[Swiftb0y]

I don't really care if its an arrow function or a plain old function expression in this case. This is just a controller mapping after all. Though I'd like to establish the use of arrow functions for any new code in the future with the ES6 rewrite of components JS.

[Holzhaus]

Ok, let's just merge this then, and I'll try to make a PR to use arrow functions later.

[Swiftb0y]

LGTM.
Also, it would be great if you could take care of documenting the ControlIndicator COs in the manual.