esp8266_milight_hub
esp8266_milight_hub copied to clipboard
added support for Casalux cct panel
added support for the Casalux cct panel sold from Aldi in Germany. Please merge only commit # db77d0e. This is my first pull request, and I can't find out how to only select this commit for pull, sorry
@aeinstein: In case you didn't find out yet: Normally you would make a branch in your repository to do your development work and then just merge the wanted changes to your master and offer that for the pull request.
If you already changed master you can do it the other way around and make a branch with only the wanted changes and offer that for PR.
Merging a single commit from PR like this is not really possible.
Cheers!
Totally missed this, sorry @aeinstein.
Appreciate you adding this, and impressed that you were able to make this deep of a change unaided.
Couple of high level concerns I have with merging this --
- Correct me if I'm wrong, but this seems like a niche device. Adding new remote types and especially radio configs introduces overhead. So obviously I'm hesitant to do that unless a lot of people use the device.
- Seems like there are some extraneous changes here. Namely changing the
RF24Channel
enum touint8_t
(I prefer enums), some references to anNLG
remote type that doesn't look to be fully supported in your changes, and some changes to the default settings.
Let me know what you think.
@sidoh : I totally agree with you, nobody wants more overhead. I think about some checkboxes in the config tab for enable/disable the types of radios (channels, syncwords) that are used by the user. (I only have casalux and NLG)
Also I dont wanted the other changes (free channel input) to be merged, this was only for understanding and testing purposes. Sorry for my wrong usage of github, I used to act with my own git server. (blame on me). I suggest to cancel the merge request and I make a new branch for the casalux impl. and one more for the changes of radio config.
The NLG impl. is not yet ready and I struggle with it. Cant find the Checksum Algo. Perhaps you can help, if I give you the sniffed raw data?
@aeinstein
I have a casalux panel, too. But I cannot compile your version successful. I get several error for nodemcuv2, example:
fatal: not a git repository (or any of the parent directories): .git
Can you help me? Compiling of the normal version runs fine
@aeinstein ... I am a general user of this software and I learnt a lot from your PR (as to how I can go about submitting fixes and features.)
If I may, I think for this PR to get merged it needs the following:
- Resend a fresh clean PR with only the changes that needs to be merged
- Have the commits be atomic and build up to the end state with relevant commit messages
- If anything is changed, accompany comments or documents explaining why.
Lastly, various device type support can be made an early stage config read and then flagging via and enum to avoid commuting unneeded code-paths and not have overheads.
I have a NLG Panel and cant get to sniff the codes
I have a NLG Panel and cant get to sniff the codes This module don't understand NLG, cause I dont understand the checksum. I suffered to find the right algorhythm, sorry. I will try it again, when I rebuild my smarthome