[spec] Extends specification of LED class

[ddemidov]

• Improved documentation of trigger attribute.
• Added description of delay_{on,off} attributes.
• Added the four of EV3 leds as special properties.

I am not sure that the four EV3 leds should go into special properties section though. Those are static class members in C++, and they could be called properties in python. Also, there are no other preexisting suitable sections in wrapper-specification.md. @WasabiFan, what do you think?

There are other convenience methods in C++ API, which are implemented in terms of the documented sysfs attributes. These are on/off methods, flash method, and a family of *_on/*_off static methods that control groups of leds (e.g. red_on, all_off). Should those be in the specification?

[WasabiFan]

I am not sure that the four EV3 leds should go into special properties section though. Those are static class members in C++, and they could be called properties in python. Also, there are no other preexisting suitable sections in wrapper-specification.md .

Hmmm... so the question is about what we want to do with static helper properties? I think we could create a section where needed for this sort of thing, although I'm having trouble coming up with a reasonable name. Maybe something like "Shorthand values" or "Reference properties"? Maybe we could call them "constants" and separate the other constants section?

...Should those be in the specification?

Those seem like easy-to-implement helpers that would make it much easier to use LEDs for signaling, so I'd say that they would be a valuable addition.

[dlech]

LEDs were already on my mind because of an issue in python-ev3. If you haven't already seen issue ev3dev/ev3dev#225, have a glance. Basically, delay_on/off are going to give people problems, so you may want to leave them out.

[ddemidov]

@WasabiFan,

I'm having trouble coming up with a reasonable name

Yes, naming the section correctly was my primary problem, because it needs to be language-agnostic.

Maybe we could call them "constants" and separate the other constants section?

Those are not constants, since we can change their state (control the leds). May be predefined instances?

Those seem like easy-to-implement helpers that would make it much easier to use LEDs for signaling, so I'd say that they would be a valuable addition.

Ok, I'll add those to the PR.

[ddemidov]

@dlech, I was able to work around ev3dev/ev3dev#225 in faf366fe2f25abfaa633d41c54ec41f61ac2ed6c by introducing a short sleep after changing the trigger. I think this may be easier if one just needs to turn blinking once and not care about it afterwards.

[WasabiFan]

Those are not constants, since we can change their state (control the leds). May be predefined instances ?

Oh, I see what you are saying now. I don't think I had fully thought through the concept before proposing those names :wink: Yes, something like "Predefined instances" or "Default instances" should be good.

[ddemidov]

I've moved predefined led instances to their own section and described convenience methods in 2e6bf08

[WasabiFan]

OK, looks good. Feel free to merge if you think it's ready.

[WasabiFan]

I think it would be beneficial to add (a) static method(s) on the LED class for mixing colors. Give it a percentage (0-1), and it sets the LEDs accordingly:

mixColors(0)   -> red: 100%, green: 0%
mixColors(0.5) -> red: 50%,  green: 50%
mixColors(1)   -> red: 0%,   green: 100%

We could also add a version to mix sides instead of colors, but that would need to take some form of color input to decide what color to use.

What do you think? I can write it up in the spec if you think it's beneficial.

[WasabiFan]

Also, I'm thinking about a pair of set<color>(brightness) static methods that would set the LEDs of that color to the given brightness. What do you think about that?

[ddemidov]

I think it would be more general for the mixColors function to take two brightness values: one for the green leds and the other for the reds. Then not only color, but also the resulting brightness could be controlled:

mixColors(1.0, 1.0); // bright orange
mixColors(0.5, 0.5); // dim orange

I also like your second idea about setting a predefined color to a brightness level.

[WasabiFan]

OK, sounds good. I'll open a PR for the spec changes soon so that you can review it.