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

Allow ESP8266 to use multiple i2c busses #6145

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

LouDou
Copy link
Contributor

@LouDou LouDou commented Jan 25, 2024

What does this implement/fix?

I have some esp8266 D1 devices which I made which have both an AHT10 and BMP025 sensor attached, but I wired these to separate i2c busses. With my own custom firmware these worked just fine, but when installing esphome and setting up the i2c busses I found that only one sensor at a time would be working.

I traced this down to the Wire.h library, which stores SDA and SCL pin assignments in global variables 🙄

This PR works around this by resetting the SDA/SCL assignments and clock frequency between every i2c read and write.

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

Related issue or feature (if applicable):
EDIT: found an issue relating to this: esphome/issues#2558

Pull request in esphome-docs with documentation (if applicable): N/A

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

esphome:
  name: env-sensors

esp8266:
  board: nodemcuv2

# Enable logging
logger:

# Enable Home Assistant API
api:
  password: ""

ota:
  password: ""

wifi:
  ssid: !secret wifi_ssid
  password: !secret wifi_pass

  # Enable fallback hotspot (captive portal) in case wifi connection fails
  ap:
    ssid: "Fallback Hotspot"
    password: "nnnnnnnnnn"

captive_portal:
  #

external_components:
  - source:
      type: git
      url: https://github.com/LouDou/esphome
      ref: i2c-esp8266-multi-bus
    components: [i2c]

i2c:
  - id: bus_a
    sda: D2
    scl: D3
    frequency: 100kHz
    scan: false
  - id: bus_b
    sda: D5
    scl: D6
    frequency: 100kHz
    scan: false

# probably any two sensors on different busses would be able to reproduce the issue on ESP8266
sensor:
  # SDA D2, SCL D3
  # AHT10_ADDRESS_0X38
  - platform: aht10
    variant: AHT10
    i2c_id: bus_a
    address: 0x38
    temperature:
      name: "Temperature (1)"
    humidity:
      name: "Humidity"
    update_interval: 300s

  # SDA D5, SCL D6
  - platform: bmp085
    i2c_id: bus_b
    address: 0x77
    temperature:
      name: "Temperature (2)"
    pressure:
      name: "Atm. Pressure"
    update_interval: 300s

Checklist:

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

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

@probot-esphome
Copy link

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (i2c) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@LouDou LouDou force-pushed the i2c-esp8266-multi-bus branch from 124ccaa to c237e36 Compare January 25, 2024 22:16
@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (23071e9) 53.69% compared to head (c237e36) 53.64%.
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #6145      +/-   ##
==========================================
- Coverage   53.69%   53.64%   -0.05%     
==========================================
  Files          50       50              
  Lines        9398     9403       +5     
  Branches     1652     1653       +1     
==========================================
- Hits         5046     5044       -2     
- Misses       4053     4059       +6     
- Partials      299      300       +1     

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

@LouDou LouDou force-pushed the i2c-esp8266-multi-bus branch from c237e36 to 9de17fe Compare January 25, 2024 22:19
esphome/components/i2c/i2c_bus_arduino.cpp Outdated Show resolved Hide resolved
esphome/components/i2c/i2c_bus_arduino.cpp Outdated Show resolved Hide resolved
esphome/components/i2c/i2c_bus_arduino.cpp Outdated Show resolved Hide resolved
esphome/components/i2c/i2c_bus_arduino.cpp Outdated Show resolved Hide resolved
@esphome
Copy link

esphome bot commented Feb 20, 2024

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@esphome esphome bot marked this pull request as draft February 20, 2024 21:33
LouDou and others added 4 commits February 21, 2024 09:49
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
@LouDou LouDou marked this pull request as ready for review February 21, 2024 09:50
@esphome esphome bot requested a review from jesserockz February 21, 2024 09:50
@probot-esphome
Copy link

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (i2c) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@jesserockz jesserockz merged commit 76a3ffc into esphome:dev Feb 22, 2024
183 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 24, 2024
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.

3 participants