Add mixer function to LED spec

[WasabiFan]

@ddemidov

[ddemidov]

I was looking at all the Red On, Green Off, Set Green methods, and had an idea: what if we introduce a predefined set of colors into specification of the LED class? Something like

"colors" : [
  "red" : [0.0, 1.0],
  "green" : [1.0, 0.0],
  "orange" : [1.0, 1.0]
]

Then all of the functions above could be easily generated with autogen. Introducing another color would also be easy: just put it into the specification and regenerate all of the bindings. What do you think?

[dlech]

What about LEDs that don't have a known color? Anything that you plug into an output port (e.g. RCX or Power Functions LEDs) don't have a color. Also, BrickPi has blue LEDs.

I think that it is good to have special functions for the built-in LEDs on the Brick, but it seems like the generic LEDs class might not be the right place for it, IMHO.

[ddemidov]

red_left, green_right are predefined static instances of the generic LED class. They, together with the helper functions above, only exist for convenience of development for the EV3 brick. But one can also create an instance of the LED class for any led present in the system.

[WasabiFan]

@ddemidov I like that idea; I'll add it to this PR.

[WasabiFan]

How do we want these methods to react? If I call "Green On", does it turn the red ones off first? And what happens if I call "Green Off"?

I think we should make the various "on" methods turn all others off, and have a single "off" method that turns it all off.

[ddemidov]

I can think of at least one use case where it would be beficial to be able to control red and green leds independently of each other. Say, we want to use the leds as indicators of two different states of a robot, state1 and state2. Then in state1 handler we call red_on when state1 is true, and red_off otherwise, and we don't have to think about state2 at all.

[WasabiFan]

That's what is causing me to hesitate... in most cases, you want to turn the others off. But in the case where you are making an indicator like that, you want it to not modify any other state.

Maybe we should have two sets of methods with different names to make it clear what they will do; or we could just decide that if you want to control them individually you need to use the mixColors method.

[ddemidov]

After some thought, set_green, set_red, and set_any_other_color should definitely work the way you are talking about, since those are just shortcuts for mixColors() with predefined arguments. Then one can argue that green_on should be equivalent to set_green(1.0) to be intuitive, so may be what you are suggesting does indeed make sense. Or may be we should skip the color_on methods entirely?

And the use case I was talking about may indeed be implemented with mixColors(state1, state2).

[WasabiFan]

I'm not sure I fully understand what you are saying in the first sentence; what do you believe set<color> should do? Does it take a percentage, or set the LEDs to our predefined state for that color?

set_green, set_red, and set_any_other_color should definitely work the way you are talking about, since those are just shortcuts for mixColors() with predefined arguments

...

...that green_on should be equivalent to set_green(1.0)

[ddemidov]

I was thinking that set_color(brightness) should be equivalent to mixColors(brightness * color[green], brightness * color[red]). This touches both leds, and so set_green() can not work independently of set_red(). Hope that is clearer.

[WasabiFan]

Ahhhh OK, I get it.

[WasabiFan]

I have just pushed the spec changes (and JavaScript implementation to make sure that the autogen works) to this branch. I decided to add four colors: red and green are as we discussed before, "amber" is the combination of red and green, and then "orange" is a redder version of amber. I was debating adding a "yellow" that had more green and less red, but don't have access to my EV3 at the moment so I decided to hold off on it to make sure that I can tune the colors.

Can you take a look and make sure that I captured the conversation correctly?

[ddemidov]

I think you forgot to push the changes to the js submodule.

[WasabiFan]

I think you forgot to push the changes to the js submodule.

Fixed; thanks :wink:

[ddemidov]

I just checked the colors on my ev3. Its very hard to tell a difference between [1, 1] and [1, 0.7]. I'd say [1, 0.5] looks more distinct. Also, [0.5, 1] does look yellow to me. But I don't claim to have a perfect color vision.

[WasabiFan]

OK, thanks for checking on that -- I'll change the orange color and add a yellow.

But I don't claim to have a perfect color vision.

I'm sure it's fine :wink:

[ddemidov]

I think its ready to be merged whether you want to add the yellow color or not.

[WasabiFan]

I think its ready to be merged whether you want to add the yellow color or not.

OK, thanks for reviewing this. Merging now.