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

New feature: custom_i2c component #6241

Draft
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

javawizard
Copy link
Contributor

@javawizard javawizard commented Feb 19, 2024

So I started implementing a custom I2C component (see this discord message and this discord thread for the humble beginnings) and I wanted to throw up a draft PR with what I have so far. This is still a work-in-progress - there's plenty of stuff left to do before it's ready for review, but I want to start a discussion and gather thoughts/feedback/requests in the mean time.

What is it?

This is a custom I2C component: it allows one to talk to I2C devices that don't have an ESPHome component written for them yet.

It's easy to use: talking to a SparkFun Qwiic Relay, for example, is as simple as:

custom_i2c:
  devices:
    - address: 0x18

switch:
  - platform: template
    name: "Relay"
    optimistic: true
    turn_on_action:
      custom_i2c.write: 1
    turn_off_action:
      custom_i2c.write: 0

Why

It turns out there's a pretty steep learning curve involved in learning to write ESPHome components from scratch - you have to know Python and C++, you have to learn ESPHome's codegen API (what the heck is a Pvariable?), schema validation, etc., etc., and then you have to write all the C++ stuff to do the actual thing you want your component to do.

That's all perfectly doable, but it's a bit daunting when all you want to do is "hey, there's this switch, when it turns on please write this value to this register, when it turns off please write this value to this other register, also you can poll this third register to get its current state and it's on if that register's value is 1" (a la https://www.sparkfun.com/products/15093).

The custom_i2c component intends to address that. My goal is to have it be possible to talk to any I2C device and both simple and easy to talk to ~80% of I2C devices. Having it be way easier/simpler/quicker to write I2C component support using custom_i2c than it is to write a dedicated component is explicitly a design goal of the custom_i2c component.

Goals

  • Really easy to use: the hardest part should be reading your device's datasheet and figuring out what you need to tell it in the first place
  • Batteries included: support for emulating GPIO pins (and by extension binary sensors and switches) and outputs (and by extension lights) is already included, and I'm very open to adding support for other types of entities as well.
  • Flexible: as few artificial limits as possible. (Registers whose addresses are arbitrary numbers of bytes are supported; controlling this board involves using both 2-byte and 3-byte register addresses, for example, and the custom_i2c component fully supports that.)

Non-goals

  • 100% comprehensive: While it would be nice to support every I2C device in existence, there are plenty of devices that are way too complicated to talk to without lots of specialized device-specific logic. It's a goal for the custom_i2c component to allow you talk to them if you're willing to write piles upon piles of lambdas to do the heavy lifting, but it's not a goal for it to bend over backward to do the heavy lifting for you if it would be just as much work to write a dedicated ESPHome component to do it.

Possible future goals

  • Support for device "templates": Assuming the custom_i2c component does its job right, it's only a small logical leap from teaching custom_i2c how to talk to your devices to sharing those configurations with others. In particular, if custom_i2c turns out as good as I hope it will, I think it would be interesting to explore the idea of allowing core ESPHome support for new I2C devices to be written as custom_i2c configs, which would lower the barrier to official support being added for new I2C devices. It's still far too early to tell whether that's a tenable goal, however; a more manageable short term goal would be to allow hosting of, and importing, device configs from e.g. Git/GitHub repos.

Current status

It works! It compiles and runs and everything, and I have it controlling a Adafruit LED arcade button board and a SparkFun single relay board and a SparkFun quad relay board and everything's working flawlessly.

Now comes the fun part. I want to:

  • Get feedback from the community - I realized I'm cutting somewhat against the grain here, so early thoughts and requests would be welcome. (I really do believe this is worth doing though.)
  • Knock out all of the tasks listed below
  • Then get this officially reviewed and (hopefully) merged in

And then 🎉 🎉 🎉

How can I try it out?

Easy: drop this in your config file:

external_components:
  - source: github://pr#6241
    components: [ custom_i2c ]

Then add a custom_i2c: config block and go to town. (More documentation on how exactly to write a custom_i2c config coming soon)

Feedback welcome!

So that's it. If you have any thoughts, requests etc., please leave a comment!

Things left to do

  • Add more example configs to this PR summary and/or discussion thread
  • Stabilize API/config format
  • Write esphome-docs documentation
  • Code cleanup
    • Constantize a few things that aren't constantized
    • Split custom_i2c.h and __init__.py into multiple files (probably)
    • Implement all the dump_config methods
    • (possibly) Split CustomI2CPinBank's registers into separate components (there's a code comment on the what and why)
  • Add tests
  • Test on devices other than ESP32 (help appreciated from anyone who owns any!)

@probot-esphome
Copy link

Hey there @javawizard,
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 = ["@javawizard"]

And run script/build_codeowners.py

(message by NeedsCodeownersLabel)

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.46%. Comparing base (4d8b5ed) to head (419baf4).
Report is 482 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #6241      +/-   ##
==========================================
- Coverage   53.70%   53.46%   -0.25%     
==========================================
  Files          50       50              
  Lines        9408     9545     +137     
  Branches     1654     1685      +31     
==========================================
+ Hits         5053     5103      +50     
- Misses       4056     4131      +75     
- Partials      299      311      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@javawizard javawizard force-pushed the jw/custom-i2c branch 5 times, most recently from 443aea1 to aec3d8c Compare February 19, 2024 02:11
@javawizard javawizard force-pushed the jw/custom-i2c branch 2 times, most recently from 50bbee7 to a2de556 Compare February 19, 2024 02:34
@javawizard
Copy link
Contributor Author

javawizard commented Feb 19, 2024

Using ESP_LOG* in header files is currently not possible - please move the definition to a source file (.cpp)

These seem to be working fine for me. Is this still relevant and/or is it that it applies on other (non-ESP32) platforms?

These classes are templated out the wazoo so moving the ESP_LOG* statements to a .cpp file is going to be a pain.

@clydebarrow
Copy link
Contributor

Looking at the code (in the absence of docs) it seems to me there are two different concepts conflated here - a generic I2C driver component, and a generic I/O expander component. Might it be better to split the I/O expander stuff out to a separate component? It could still use the generic I2C stuff as a base.

@clydebarrow
Copy link
Contributor

Also it looks like you've added some (useful) comments in core code - that would be best as a separate PR, which would likely be very quickly merged without waiting for a full review of your component.

Also temporarily add verbose log statements in all of the on_setup
methods; see the code comment for why.
(specifically fix the case where outputs are not used; we need to
not write C++ code that depends on output things in that case)
javawizard added a commit to javawizard/esphome that referenced this pull request Apr 26, 2024
@javawizard javawizard mentioned this pull request Apr 26, 2024
4 tasks
@javawizard
Copy link
Contributor Author

Core comments extracted to #6643 (cc @clydebarrow).

clydebarrow pushed a commit that referenced this pull request Apr 26, 2024
@jesserockz jesserockz mentioned this pull request May 8, 2024
@jesserockz jesserockz mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants