Added SetOption37 for RGB remapping

[gsimon75]

The option value is interpreted as a 3-digit ternary number, each digit telling where the corresponding channel is mapped from: the rightmost digit being for channel 0, the middle digit for channel 1, the leftmost digit for channel 2.

The digit values are 0=red, 1=green, 2=blue, so:

• RGB = {0,1,2} = (0 * 1) + (1 * 3) + (2 * 9) = 21 means the identity mapping
• BGR = {2,1,0} = (2 * 1) + (1 * 3) + (0 * 9) = 5
• GRB = {1,0,2} = (1 * 1) + (0 * 3) + (2 * 9) = 19
• BRG = {2,0,1} = (2 * 1) + (0 * 3) + (1 * 9) = 11
• RBG = {0,2,1} = (0 * 1) + (2 * 3) + (1 * 9) = 15
• BBG = {2,2,1} = (2 * 1) + (2 * 3) + (1 * 9) = 17
  etc.

Just changing the option does not trigger the color change, you must either set a new color or send a 'power off/on' command.

Related to: Issue #5310

[arendst]

Ingenius!

As each permutation actually needs 2 bits per color change it must be possible to add the white channel too.

So suppose we use 2 bits per color (2 bits is 0,1,2,3) for RGBW. Then the 8-bit param byte bits could be used like WWBBGGRR. Just shift them out by 2 to get the index in the copy cur_col. So

• for RGBW the param value would be 11 10 01 00 = binary 0d11100100 = 0xE4 = decimal 228.
• for RBGW we need to swap GB so 11 01 10 00 = 0d11011000 = hex 0xD8 = decimal 216.
• for GRBW wee need to swap GR so 11 10 00 01 = 0d11100001 = hex 0xE1 = decimal 225.
  etc.

What do you think?

[netpok]

I didn't get much look on the code, but wouldn't it better to apply this feature in the common part of the driver so it could affect other drivers like the my92 or arilux ones?

Edit: Never mind I missed that this just does that.

[gsimon75]

As each permutation actually needs 2 bits per color change it must be possible to add the white channel too.
What do you think?

The only problem I see is that quite frequently we have 2 kinds of whites (cold and warm), which gives 5 channels in total.

For a complete N-to-N mapping we'd need a value domain of N ** N, this was 3 ** 3 = 27 for the rgb-to-rgb and it's 4 ** 4 = 256 for the rgbw-to-rgbw you mentioned, and these indeed fit in one byte. For an rgbwc-to-rgbwc mapping we'd need 5 ** 5 = 3125, which is unfortunately too big for one option.

So I see three (somewhat) consistent ways to extend this rgb-only mapping:

1. Use two option bytes, 16 bits should be enough for a while :)
2. Don't support the full mapping, but only a subset, like only the permutations of RGB (6) times a full 2-to-2 mapping of the whites (4)
3. Your suggestion of 4-to-4 mapping, with some trickery: for the mapping we support only one white:

• When one of R,G,B channels maps to 'white', then it means the average of the whites that are present
• If 'White' maps to 'White', it means WW to WW and CW to CW
• If 'White' maps to anything else, it means WW to X and CW to X

Finding a numerical encoding is not a big deal, the problem is choosing the <= 256 combinations we'd like to support, so the question is what do the users actually need?

[gsimon75]

I didn't get much look on the code, but wouldn't it better to apply this feature in the common part of the driver so it could affect other drivers like the my92 or arilux ones?

It is that way, at least that's how I intend it :)

[arendst]

I think what the user currently needs is RGB control and as you might have seen for Neopixel color control there is also a selection for one white channel.

So I think let's go for the one byte option and see what the future brings.

[gsimon75]

OK, I'll check the Neo features and try to implement something similar, with one param byte.
(I'll be offline for a few hours now, but I hope it'll be ready for at least tomorrow.)

[netpok]

I have an Idea for a full 5 channel mapping in seven eeprom bytes, but probably needs to much program space. Will try it later today.

[arendst]

No problem.

Rgrding Neo features I think just add up to 4 color bytes instead of three. If you want to I can do it based on your work.

[digiblur]

Posted in the other pull request with kinda the same thing... would this work with duplicating channels as well? Like if you had a My9291/My9231 bulb with 1 chip/4 channels (much like the standard AiLight), but it had channels mapped like CW/WW/CW/WW? I've also seen another with 2 chips/6 channels with CW,CW,G,R,B. They can definitely be all over the place.

[arendst]

I think there are two different issues here:

• Color mapping where different colors needs to be mapped
• Hardware interface mapping where certain pins need to be connected to certain color leds as note by @digiblur these are changing by the minute.

This pull request is only about color mapping where the hardware has already been defined but the colors are wrongly connected.

Mapping hardware pins to colors opens up a can of worms.

[DavinKD]

I don't see that the whites are any different from colors. They are separate LED groups controlled just like rgb. I understand the original issue was only for rgb, but I don't believe it makes sense to make a change and not deal with the whites at the same time.

[gsimon75]

I understand the original issue was only for rgb, but I don't believe it makes sense to make a change and not deal with the whites at the same time.

I see two problematic points, a conceptual and a technical one.

The conceptual problem is the number of the channels. If we don't want to use dynamic structures everywhere (including in the saved settings), then we must set a fixed number of channels we 'allocate' compile-time. If this is too low, then we can't handle every situation, if it is too high, then we're wasting resources everywhere else. The current number of channels is MAX_PWMS=5, that is {r,g,b,w,c}, the individual values we'd like to set to change the light, and up to now it seems to cover the vast majority of the devices.

If a device has 6 channels, we can't handle it at all, because we get only 5 values from the user.
To handle channel multiplicity we would need another level of abstraction, say 'output channels' that would implement some settable mix of the 'component channels', and the number of these two channel sets could then be completely different.

It's a nice idea, but it would require a huge amount of re-consideration and re-implementation, it would 'open up a can of worms', as @arendst phrased it :). (And then we shouldn't stop at just assigning channels, but have a mixing matrix instead, etc.)

The technical issue is that now we have a very limited number of config param bytes available (that is, without changes that would affect the whole codebase), so the configuration space of the channel mapping should fit in one byte. A generic 4-to-4 mapping fits, but that's all. As soon as we exhaust these param bytes, the next feature that requires one would be possible only via incompatible changes, so one feature mustn't use up more than what's absolutely necessary.

So, it's indeed a nice idea, but it seems that it would require too extensive design-level changes to the codebase, and now at least we should start with a 'cheaper' change and learn what we can from it.

Most probably that's why @arendst added that '...and see what the future brings.' to the end :).

[DavinKD]

I'm ok with 5 as that would seem to handle everything tasmota is capable in terms of color control.

Here's the changes I made for my Lohas bulbs. Not being very familiar with the code yet, I may not have done it in the most appropriate way, but it worked for me. I only post this to show what I had to do to get these bulbs to work. If your changes can accommodate this, great. If not, I'm perfectly fine using a one-off custom build for them. Wouldn't want folks spending an inordinate amount of time on a small subset of bulbs.

Sonoff.ino - Change the AILigh base to RGBWC instead of RGBW
**FROM
else if (AILIGHT == my_module_type) { // RGBW led
light_type = LT_RGBW;
}

**TO
else if (AILIGHT == my_module_type) { // RGBW led
light_type = LT_RGBWC;
}

xdrv_04_light.ino - Rearrange the color channels for RGBWC
**FROM
uint8_t duty[2][6] = {{ duty_r, duty_g, duty_b, duty_w, 0, 0 }, // Definition for RGBW channels
{ duty_w, duty_c, 0, duty_g, duty_r, duty_b }}; // Definition for RGBWC channels

**TO
uint8_t duty[2][6] = {{ duty_r, duty_g, duty_b, duty_w, 0, 0 }, // Definition for RGBW channels
{ duty_r, duty_g, duty_b, duty_w, duty_c, 0 }}; // Definition for RGBWC channels

This bulb does have 2 white channels, but what I configured for warm is just a tiny less cold version of cold. No idea why they have 2 banks of cold leds instead of 1 cold, 1 warm. Maybe they just wanted a really bright cold white when both are turned on. The color temp difference I see could just be natural variations of the same leds.

[netpok]

Based on this I tried to implement a full 5 color remapper, the best I could come up is +160byte compared to this pull request. It allows mapping any channel to any channel but 1 channel could be mapped only to one channel so RGBCwWw => BGRWwCw is valid, but RGBCwWw=>RGBRG is not.

I made a pull request against the repository of @gsimon75 available here: gsimon75-iot#5

Here is the color mapping table: https://docs.google.com/spreadsheets/d/1ovEWyUBObOYYAl8ey6K6LffHhiniRXuIq7iivyxmyEU/edit?usp=sharing

[gsimon75]

I've reverted the RGBW 4-to-4 mapping, and I'm merging the permutation-based approach of @netpok , but there are some minor loose ends (initializing the param to a sane default, recalculating the mapping when the SetOption37 arrives, etc.), and I'll do some testing before I commit the merge :).

[gsimon75]

@netpok: Nice work, thanks for the contribution!

I've added a few things:

• Re-added the RGB_REMAP_whatever constants, but with the values from your mapping
• Re-added the param initialisation in case the flash is empty or we're updating from an older version
• Moved the calculation of the remap indices to a separate function, because
• Added a call to it where the SetOption37 is processed, so we don't need a restart for a new value to take effect
• Renamed remap to color_remap :)

I haven't updated the wiki yet, just in case the reviewers find something we haven't noticed...

[DavinKD]

Question for you. Based on this change would I still set my bulb to AILights and then use this setoption to fix the colors? Or would I use a template based of AILight?

As I pointed out in my code change the AILight assumes 4 channels, but I have 5.

Thanks again for your work on this.

[gsimon75]

Question for you. Based on this change would I still set my bulb to AILights and then use this setoption to fix the colors?

If I get it right, you need two changes:

1. You need RGBWC instead of RGBW

As far as I see, if you choose SONOFF_B1 instead of AILIGHT, it accomplishes exactly this change.

2. You need {{R,G,B,W,0,0},{R,G,B,W,C,0}} instead of {{R,G,B,W,0,0},{W,C,0,G,R,B}} (in that duty[][] matrix)

Now, that's the trouble point.

All the proposed RGBWC remapping happens before the LightMy92x1Duty() call, so we're only rearranging the RGBWC components, but no such rearrangement can achieve what you need.
The first {R,G,B,W,0,0} must remain intact, so R,G,B,W mustn't be replaced by anything else.
But the second 6-tuple should be rearranged, so they must be replaced by something else, and that's a contradiction.

So it is not a problem of device-agnostic channel remapping, but of supporting a not-fully-standard variant of SONOFF_B1. I don't think we can solve it here, but it may be worth a separate issue, so it won't get forgotten but doesn't block this one.

As of how to solve that, the only way I see is by having a separate duty[][] array, exactly the way you wrote.

To make such a thing choosable from build config, we'd need some sort of sub-model define, like USE_MY92X1_CTYPE (similar to USE_WS2812_CTYPE), but that would be only a compile-time choice, only a little better than applying a specific patch manually.

Unfortunately I don't see any feasible way to add this degree of flexibility :(, maybe some of the more experienced developers have some idea...

On the other hand, if/when that sort of bulbs start spreading on the market, we'll have to face this problem nevertheless, so it's worth thinking about it anyway.

[netpok]

I removed the initialization because I assumed, that the default values are zero based on the lots of commented out equals zero lines. Of course if that's not the case then my bad and sorry.

Also I removed the defines to save flash, but I did forget that those will be stripped anyways if they aren't used. Also I left out the live update because of the same reasons.

[netpok]

I added the option to the wiki, but the color mapping table should be probably migrated to the wiki from my spreadsheet I will do it when I have some time (of course anyone can do it ;)), but till then I will leave the spreadsheet available.

[gsimon75]

@netpok : Well done :) !

[gsimon75]

Spreadsheet to wiki conversion done: SetOption37, and I've linked it to the short description in the SetOption list

[dlashua]

A MY9291 has 4 channels. This is what you get with the "AILight" device type.

A MY9231 has 3 channels, but can be daisy chained. You get a two chip configuration with the "Sonoff B1" device type.

What would be ideal is a configuration option to Map Channels to colors, including an "Empty" Channel. Either have the device type set the number of channels (4 for AILight, 6 for Sonoff B1) and then offer Channel Selectors for each channel name Tasmota is aware of (Red, Green, Blue. Cold White, Warm White) or offer a text field that should be filled with a string like "RGB0WC" to denote which channel goes to which color. SetOption37, as implemented here, doesn't seem to allow any way to access the 6th channel, which my Bulb device uses (mine is 0CWGRB). So, I'm unable to access the Blue LEDs. This way the duty matrix is would be configurable since different devices will use the channels differently. As it is now, without a code change specific to this device, I am unable to use Tasmota.

Alternately, SetOption37 could provide 6 channels in the matrix (instead of 5 like it does now). This would support the most common device configurations (one MY9291 or two MY9231s) but would require more than 120 "options" to enumerate them all (720 - 5040 depending on implementation).

[netpok]

The problem is that currently setoption37 resides in an 8bit variable, so to implement this it would need some changes. Just to confirm, there is no known configuration in which you can access all leds, right?

[digiblur]

Looks like he can't access 6 channels? Only other configs I know of are the ones with duplicate colors on multiple channels.

[dlashua]

@netpok I'll admit, I didn't try all 119 variations available in SetOption37. But after trying the ones that were in the same order as my LED channels, I tried those in reverse order, and those with the second chip listed first. No matter what I did, channel 6 was not accessible.

The device is a LOHAS RGBW bulb (Tuya OEM firmware). It's made more complicated because white is actually two channels (though both channels are basically the same temperature of light). I didn't even try to address that issue, yet, and just settled for the one white channel.

If it helps any, I was able to get this running using ESPHome with no issues, though I'd prefer to use Tasmota.

[gsimon75]

The issue does occur in the config of @dlashua, so we should think up some solution for it, even if it won't be easy :), but we must keep the backward compatibility.

The current solution is able only to permute the 5 channels (R,G,B,W,C), it isn't possible neither to skip one channel, nor to assign the same component to more than one channel, so it cannot solve his problem.

The reason for this limitation is that (as of now) the configuration space for this re-mapping must fit into one uint8_t, the permutations require 5! = 120 so it fits, an arbitrary remapping of {none,R,G,B,W,C} to six channels would require 6 ^ 6 = 46656 configurable values so it would require two bytes.

The SetOptionNN values are stored as an uint8_t array here, the size of this array is limited to 18, because the other indexes are already assigned to other purposes (see the parser code here), currently the indices 32..37 are assigned, 38..49 are available, so if we absolutely must, then we can allocate more, the question is how many do we need?

If we make this a generic config item, we shouldn't just consider this particular model, but try to solve all the similar cases as well. With 2 bytes we could do a 6-channel full remapping, but will it be enough for all similar cases? With 3 bytes it could support 9 channels, with 4 bytes 12. I'd say 12 channels are impractical, but I would've said that for 6, and yet there they are :)...

Can we define a reasonable limit for the number of channels to support? We must, because the config is stored as a block in the flash, but once we settle for a number, then it'll be that, it can't be extended later, and it'll use up those bytes forever, even on devices that doesn't even need this.

On the other hand, there is a different approach.

Instead of extending the complexity of the config for being able to support everything and anything, we could consider this as a 3rd My92x1-based device type (in addition to AiLight and B1), and implement these local specialities in model-specific code, rather than in general config.

Now the decision (AiLight or B1) is based on the presence or absence of the 2nd white channel here, so a 3rd choice doesn't fit there, but it is possible to distinguish based directly on the module type, like for the MI_DESK_LAMP here.

As we have far more available space for the code than for the config, and this way whatever we do, it wouldn't affect any other device type at all, I'd rather vote for this approach.

What do you think?

[netpok]

I'm currently leaning towards the full six channel mapping because it would solve the issue for all, there is even a free location for it at 7BC. But the question is should we use a 5 to 6 or a 6 to 6 mapping. I'm using some drivers to drive led strips at multiple places so I can see the use of 6 channels, but in general probably way easier to go with 5 to 6 mapping.

The other solution I can think right now is just use 3 bits of data from flag2 (or steal it from somewhere, there are some variable which have free bits) and use it to mark the empty channel of the my92x chips, but this doesn't solve the same colored channels.

[gsimon75]

In the case of @DavinKD above, he needs {{R,G,B,W,0,0},{R,G,B,W,C,0}} instead of {{R,G,B,W,0,0},{W,C,0,G,R,B}}, so for these twin-my92x1 cases we do have 12 channels, 6 for the first chip and 6 for the second, see here.

Sure, in this case the first 6 channels are identical, but in the original cases it was a plain RGB-to-BGR, and in the NEO_whatever cases also the usual first 5 channels are being manipulated, so we must retain the ability to tweak the first 5 channels as well, we can't take them for constant identity.

So AFAIK, what we need here is a free remapping of {zero,R,G,B,W,C} to 12 channels, meaning 6 ^ 12, and that would require 32 bits.

On one hand, this whole redundant channel thing makes sense only for the My92x1, so it seems to be a very specific workaround for one particular model, and it would be an overkill to design a generic mechanism just to cover it.

On the other hand, if we implemented this, then we would no longer need the distinction of AiLight vs. B1, nor the whole NEO_whatever modes, nor this current SetOption37, so in the long run it would make the code clearer.

Both approaches have benefits, but it's not a trivial decision, so I suggest opening a separate issue for it, so that it isn't lost in all the discussions above :).

[netpok]

Jeez, 12 channels, but why... I currently don't even see why these chips are better than a simple pwm, but that won't change what the manufacturers are putting them in their device.

Also I opened the new thread: #5490

[DavinKD]

I don't believe my light had 12 channels, just 6 like the other dual myxxxx setups. However, I did have 2 CW channels like digiblur on his recent stream. So I would need the same color mapped on 2 channels.

[gdreelin]

I'm ok with 5 as that would seem to handle everything tasmota is capable in terms of color control.

Here's the changes I made for my Lohas bulbs. Not being very familiar with the code yet, I may not have done it in the most appropriate way, but it worked for me. I only post this to show what I had to do to get these bulbs to work. If your changes can accommodate this, great. If not, I'm perfectly fine using a one-off custom build for them. Wouldn't want folks spending an inordinate amount of time on a small subset of bulbs.

Sonoff.ino - Change the AILigh base to RGBWC instead of RGBW
**FROM
else if (AILIGHT == my_module_type) { // RGBW led
light_type = LT_RGBW;
}

**TO
else if (AILIGHT == my_module_type) { // RGBW led
light_type = LT_RGBWC;
}

xdrv_04_light.ino - Rearrange the color channels for RGBWC
**FROM
uint8_t duty[2][6] = {{ duty_r, duty_g, duty_b, duty_w, 0, 0 }, // Definition for RGBW channels
{ duty_w, duty_c, 0, duty_g, duty_r, duty_b }}; // Definition for RGBWC channels

**TO
uint8_t duty[2][6] = {{ duty_r, duty_g, duty_b, duty_w, 0, 0 }, // Definition for RGBW channels
{ duty_r, duty_g, duty_b, duty_w, duty_c, 0 }}; // Definition for RGBWC channels

This bulb does have 2 white channels, but what I configured for warm is just a tiny less cold version of cold. No idea why they have 2 banks of cold leds instead of 1 cold, 1 warm. Maybe they just wanted a really bright cold white when both are turned on. The color temp difference I see could just be natural variations of the same leds.

David how did you do this for the LOHAS bulbs? How did you enter this into the command line using the SetOption37 ? I was a bit lost on how you mapped yours since I have some that are exactly like what you have.