add support for SPI / WS2812B

[jangxx]

Thank you for your PR! I didn't even know that the Magic Home system supported individually addressable LED strips and it would be great if we could support them. Just one question though: What exactly is this functionality called in the app? All your method signatures use the word function, which I'm kind of uncomfortable with, since it's a reserved word and also methods like setFunction can be pretty confusing. I've tried to find some videos or screenshots of the part of the app, that's used to control the "function", but unfortunately I couldn't find any. Can you provide a bit more information and maybe think of another name for this class of functionality?

[jangxx]

I also took a look at your code, and I have some additional comments:

Control.js:76: I'm not sure what you're trying to do here but if you shouldn't have a function that either returns something or nothing. If you want to explicitly return nothing, return null.

Control.js:514: Why the additional number conversion?

Control.js:516: Declaring a const just to change it right afterwards is bad style in my opinion.

Control.js:521: As far as I understand the protocol, each command has to end with a 0x0F terminator, so I don't think you have to append the speed a second time after the terminator.

Control.js:523: What does the // 288 mean?

[jaapenstaart]

No problem.

I agree with you! I was also hesitant to use function. I couldn't find a better alternative for now. In the App it's called "Functions".

[jaapenstaart]

Control.js:76: I'm not sure what you're trying to do here but if you shouldn't have a function that either returns something or nothing. If you want to explicitly return nothing, return null.

I thought i leave it undefined, so it doens't appear for devices that doesnt support function, but good one, it can be null

Control.js:514: Why the additional number conversion?

the CLI makes it a string, so when I tried 100 + 99, the result was 10099 instead if 199

Control.js:516: Declaring a const just to change it right afterwards is bad style in my opinion

This is correct usage of const, I doesn't change, it's still a Buffer. I noticed a lot of let usage in the code where it's suppose to be const. (promises eg)

Control.js:521: As far as I understand the protocol, each command has to end with a 0x0F terminator, so I don't think you have to append the speed a second time after the terminator.

Correct, I removed it

Control.js:523: What does the // 288 mean?

Nothing, it was a note to myself, removed it

[jangxx]

the CLI makes it a string, so when I tried 100 + 99, the result was 10099 instead if 199

In that case it makes more sense to do the number conversion in the CLI code. The library function is not responsible for input sanitation.

This is correct usage of const, I doesn't change, it's still a Buffer. I noticed a lot of let usage in the code where it's suppose to be const. (promises eg)

You are right about the part with the promises. I copy&pasted that code all over the place and missed the let the first time. Other than that, I'm not sure that this is a question of the "correct" usage of the const keyword. I know that it only guarantees a constant reference, but like I said, I don't like to use it for variables that are intended to be changed at all. This is just personal preference however.

I also thought about the function thing some more; Looking at the app and also the code, it seems like these "functions" are basically the same as the patterns we already support, just with two bytes for the pattern code instead of one, as well as some other differences, like the speed parameter. Maybe we could fold the functionality into the pattern stuff we already have, and just call these additional patterns ia_pattern (for "individually addressable pattern") or something. In that case the method to change them would then be called setIAPattern and the additional helper methods would also be mostly obsolete. One problem remains however - the object returned by queryState becomes very inconsistent no matter what. The mode field is set to a string representation of the old patterns, but would be set to "ia_pattern" for all the new ones, with an additional field only used in that mode. I think it would make sense to set the mode field to either "pattern" or "ia_pattern" depending on the type of pattern and then adding a new pattern field, which either contains a string name or a number. Basically: Let's change "function" to "ia_pattern" everywhere, except for queryState, where we change it to "pattern", with the new field serving two uses.

[jaapenstaart]

Tnx for your input, I can implement those changes this evening.

[jaapenstaart]

I updated to iapattern instead of function. The only thing is that i figured that determine mode was broken for my type anyway, so I created an extra condition in determineMode. Now it works also correct with color. And the normal patterns never worked on my controller. (type 161)

Since I only have a SPI controller available, I think it's wise to test this commit for another type of controller. Are you able to @jangxx ?

[jangxx]

Thanks for the changes. I guess I will merge the current state into the new_features branch and then do some additional cleanup and tests before bumping the version to 2.5.0.

[jangxx]

I pushed my cleanup and some other imrovements in commit e46849ca4bed08c846ee1ea3cf35cc6726f7c0e8. I also tested that the code still works with my controllers, so after you have tested it with yours we are ready for a 2.5.0 release.

[jghaanstra]

@jaapenstaart , sorry to hijack this PR but what controller and LED strip are you using. I have tried various a 5V and 12V Magic Home SPI controllers with various types of LED strips but can not get it to work. Not even in de native app. I have seen other users with similar results. The controller can be paired by the app but the LED strip just shows some random lights that do not respond to changes in the app.

I would love to find the right combination and play with it.

[jaapenstaart]

@jangxx I tested your clean up commit, but unfortunately the setIAPattern function is broken. It happens in line Control.js:515 bufferArray.push(code >> 2); if I change it to bufferArray.push(code >= 255 ? 1 : 0); it works fine again. I'm not an expert in bitwise operators so i'm not sure how to write this in a neater way.

@jghaanstra No problem. I use the 5V magic home LED SPI controller in combination with btf lighting ws2812b 5 meter, 300 leds strip. Both ordered on AliExpress.

[jangxx]

Well, I guess I'm stupid, of course >> 2 doesn't work, it has to be >> 8. Fixed it in the last commit.

[jaapenstaart]

haha, no. It >> 8 works like a charm! :)

[jangxx]

So everything works fine then? I this case I can prepare the 2.5.0 version for release.

[jaapenstaart]

For me everything looks fine!