search

adafruit / Adafruit_SGP30

Arduino library for SGP30
Other
53 stars 38 forks source link

A library should never call begin() on a shared bus #9

Closed TD-er closed 5 years ago

TD-er commented 5 years ago

See: https://github.com/adafruit/Adafruit_SGP30/blob/00569ee7488516a03a12530f0bf57b98430187c7/Adafruit_SGP30.cpp#L50

The library for this sensor does call begin() on the I2C bus. A library should never call functions like these on a shared medium without explicit call to do so. A library should just add support for some sensor or device and should never change things on a shared medium.

This does cause undefined behavior when using the library together with other sensors on the same bus.

If you really need to have it done in the begin function of your library, then please either add an optional parameter to not call begin or use defines in the code to exclude it.

ladyada commented 5 years ago

hiya, wontfix - this is 100% the standard operation procedure for arduino libaries and we've done it 200+ times for 10 years. Wire.begin() should be idempotent in the board support implementation, as it is in all official Arduino boards!

ladyada commented 5 years ago

its also what Arduino does for their libraries, in case you think we're doing it wrong ;) https://github.com/arduino-libraries/Arduino_HTS221/blob/master/src/HTS.cpp#L47

TD-er commented 5 years ago

That doesn't mean it is the right thing to do.

I'm not suggesting you should change it to all, only if you want to consider it for this one and future libraries. If not, then other projects cannot use your libraries by linking to them but have to include them in a patched form.

ladyada commented 5 years ago

nope, wontfix - if you have a 'fork' and its not compatible with arduino's official libraries, its your fork thats the issue :) Wire.begin() was designed to be callable as many times as you like, if that implementation is broken, it needs to be fixed, not a decade worth of libraries