Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SM2235 and SM2335 LED drivers #3924

Merged
merged 36 commits into from Dec 22, 2022
Merged

Conversation

Cossid
Copy link
Contributor

@Cossid Cossid commented Oct 20, 2022

What does this implement/fix?

This implements a base LED driver for Sunmoon Microelectronics 10 bit LED driver base support the SM2235 and SM2335 LED drivers. The SM2335 LED driver can be found in the SwitchBot Color Bulb.

Note: The existing SM2135 driver is 8 bit and substantially different, so must remain a separate component not using this new base.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#2514

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266

Example entry for config.yaml:

SM2335:

    # Example configuration entry
    sm2335:
      data_pin: GPIO4
      clock_pin: GPIO5
      max_power_color_channels: 9
      max_power_white_channels: 9

    # Individual outputs
    output:
      - platform: sm2335
        id: output_red
        channel: 1
      - platform: sm2335
        id: output_green
        channel: 0
      - platform: sm2335
        id: output_blue
        channel: 2
      - platform: sm2335
        id: output_coldwhite
        channel: 4
      - platform: sm2335
        id: output_warmwhite
        channel: 3

SM2235:

    # Example configuration entry
    sm2235:
      data_pin: GPIO4
      clock_pin: GPIO5
      max_power_color_channels: 9
      max_power_white_channels: 9

    # Individual outputs
    output:
      - platform: sm2235
        id: output_red
        channel: 1
      - platform: sm2235
        id: output_green
        channel: 0
      - platform: sm2235
        id: output_blue
        channel: 2
      - platform: sm2235
        id: output_coldwhite
        channel: 4
      - platform: sm2235
        id: output_warmwhite
        channel: 3

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@probot-esphome
Copy link

Hey there @Cossid,
Thanks for submitting this pull request! Can you add yourself as a codeowner for this integration? This way we can notify you if a bug report for this integration is reported.
In __init__.py of the integration, please add:

CODEOWNERS = ["@Cossid"]

And run script/build_codeowners.py

(message by NeedsCodeownersLabel)

@Cossid
Copy link
Contributor Author

Cossid commented Oct 21, 2022

To the best of my understanding, all remaining CI failures are unrelated to changes made by this PR.

@sebcaps sebcaps mentioned this pull request Oct 23, 2022
10 tasks
@sebcaps
Copy link
Contributor

sebcaps commented Oct 23, 2022

@Cossid : esphome/issues#3423 ?

@ssieb
Copy link
Member

ssieb commented Oct 23, 2022

Other than the trivial setters, move the function contents to the .cpp file.

@Cossid
Copy link
Contributor Author

Cossid commented Oct 23, 2022

Other than the trivial setters, move the function contents to the .cpp file.

Yeah, I started with SM2135 as a base, but that does seem poorly written. SM2135 needs some work too, but it will require breaking changes. I will likely address that driver in the future as I work on a few other LED drivers.

@ssieb I have adjusted this driver to closer emulate the my9231 driver header/source structure, please re-review that this meets expectations.

@Cossid
Copy link
Contributor Author

Cossid commented Oct 24, 2022

Just adding a note that there was a request for a SM2235 driver on discord, which looks like it shares the majority of code with the 2335 driver. I have created a fork branch named sm_1024bit_base which shares as much code as I was able to, and pending feedback from users with sm2235 drivers, this PR may end up abandoned in favor of that one which supports both SM2235 and SM2335. If this one is merged, it should be a non-breaking change to the other branch.

@Cossid Cossid changed the title Add support for SM_10BIT_BASE and inheriting SM2235 and SM2335 LED drivers Add support for Sm10Bbit_Base and inheriting SM2235 and SM2335 LED drivers Oct 25, 2022
@Cossid
Copy link
Contributor Author

Cossid commented Oct 25, 2022

Last remaining CI failure is due to a naming convention (using an underscore in a class name), however, this appears to be the norm for other classes used as a base class. I am leaving it as is for now to match existing code, but can change if needed.

After further review, the underscore is only used in the component directory name and namespace, renamed class to match linter expectations.

@ryanjohnsontv
Copy link

Just wanted to add that I used this branch to compile firmware for my Switchbot RGB+CT E26 W1401400 bulb (ESP32-C3), and it works flawlessly. Thank you so much for your work!

Copy link
Member

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change, but otherwise good to go

esphome/components/sm10bit_base/__init__.py Outdated Show resolved Hide resolved
esphome/components/sm10bit_base/__init__.py Outdated Show resolved Hide resolved
Cossid and others added 2 commits December 8, 2022 16:34
Remove unnecessary coroutine declaration

Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Remove unnecessary coroutine declaration

Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
@jesserockz jesserockz changed the title Add support for Sm10Bbit_Base and inheriting SM2235 and SM2335 LED drivers Add support for SM2235 and SM2335 LED drivers Dec 22, 2022
@jesserockz jesserockz merged commit 53b60ac into esphome:dev Dec 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants