Skip to content

Allow user to call TwoWire::begin themselves #38

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

Closed
wants to merge 1 commit into from

Conversation

Tasssadar
Copy link

@Tasssadar Tasssadar commented Jan 16, 2020

  • That way, they can use non-default pins and have multiple
    devices on a single TwoWire bus.

* That way, they can use non-default pins and have multiple
  devices on a single TwoWire bus.
@caternuson
Copy link
Contributor

To use non-default pins, create a Wire object with those and pass in that instance:
https://github.com/adafruit/Adafruit_BMP280_Library/blob/master/Adafruit_BMP280.h#L185

Can you clarify what you mean by having multiple devices? That is supported on any I2C bus as long as each device has a unique address.

@Tasssadar
Copy link
Author

Let's say you connect two different sensors on one I2C bus.

You are supposed to only call Wire.begin() once1, and if the library calls it, you can't use two libraries that use I2C (because they both call begin()).

Also, on ESP32 with Arduino framework port, Wire.begin is used to set the I2C pins2, not the Wire constructor.

@ladyada
Copy link
Member

ladyada commented Jan 17, 2020

you can def call Wire.begin() as many times as you like
ESP32/ESP8266 do it in an incompatible way that breaks every other platform, we dont allow that change in our libraries :)

@Tasssadar
Copy link
Author

Tasssadar commented Jan 17, 2020

While you definitely can, I don't think you should. As far as I can tell, there are no guards against it anywhere in the Arduino core, so if there is any I2C transfer in progress already, the state will become corrupted.

From the code, it's clear TwoWire::begin() is not designed to be called multiple times, even though it might work in most trivial use cases.

I agree that ESP32's begin is "unofficial", but it's also not exactly unpopular platform to build stuff on, and I'd say it's a good idea to at least consider supporting it.

@ladyada
Copy link
Member

ladyada commented Jan 17, 2020

we support esp32 just fine - but the Wire interface pins must be defined in the BSP like every other arduino :)
Wire.begin() must be able to be called multiple time - all arduino libraries will call wire.begin() - its 10 years too late to change.
Please ask the espressif team to keep compatibility, it isnt hard, and we must make sure our libraries work with all platforms

@ladyada
Copy link
Member

ladyada commented Jan 17, 2020

thank you for the PR, but we will not be able to merge it

@ladyada ladyada closed this Jan 17, 2020
@Tasssadar
Copy link
Author

Tasssadar commented Jan 20, 2020

I understand, thanks for considering it anyway :)

Wire.begin() must be able to be called multiple time - all arduino libraries will call wire.begin() - its 10 years too late to change.

Honestly, that's horrifying.

@JarekParal
Copy link

@ladyada

we support esp32 just fine - but the Wire interface pins must be defined in the BSP like every other arduino :)

Why constrain the user for one specific configuration when we have a super device as ESP32, where you can set almost each peripheral to any pin. Help us to move to 21. century in MCU development.

Wire.begin() must be able to be called multiple time - all arduino libraries will call wire.begin() - its 10 years too late to change.

Why you couldn't make a small step for improving the Arduino libraries by merging this PR in your library and make Arduino world a little bit better. This will not add you any other workload.

@ladyada
Copy link
Member

ladyada commented Feb 28, 2020

@JarekParal have you opened an issue with esp32 asking them to make a new, non-conflicting function say setPins() ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants