pololu / vl53l0x-arduino

Pololu Arduino library for VL53L0X time-of-flight distance sensor
https://www.pololu.com/product/2490
Other
345 stars 163 forks source link

Optionally support other Wire objects. #45

Closed KurtE closed 4 years ago

KurtE commented 4 years ago

Many processors, now days have more than one hardware I2C on them. Sometimes it is convenient to use a different I2C object like Wire1 or Wire2 instead of the default Wire object.

This change allows you to pass in a pointer to the desired I2C(Wire) object on the constructor. which defaults to Wire.

This change requires the main header file to include the wire.h header file.

KurtE commented 4 years ago

Looks like this is more or less the same as #13

Other than I did with the constructor, and @mjs513 did it on the begin. I was 6 of one and half of a dozen the other way so either would be good.

But as seeing that the earlier PR was several years ago. I am guessing it will be even harder to try to add additional functionality.

ryantm commented 4 years ago

Hi, @KurtE.

We would prefer the Wire pointer be set via a separate function (instead of as a constructor argument), because:

Sincerely, Ryan Mulligan

KurtE commented 4 years ago

Hi @ryantm - I can easily add a new method for this. probably a set and a get of the current one.
Do you have a suggestion on names? Something like ?

void setWire(TwoWire *theWire = &Wire);
TwoWire *getWire(void);

Although I don't have all of your sensors, At some point these should be added to your other similar libraries. like the VL53L1X and probably the VL6...

The question I have for myself and others, is there are several simple features that if I were to use this library I would like. I started implementing some of them, but I also see that they overlap with the other current PR #42

So I am trying to decide what the best approach would be to take.

kevin-pololu commented 4 years ago

Hi, KurtE.

I went ahead and merged your changes along with some further revisions of our own (including making separate setBus() and getBus() functions instead of doing it through the constructor, like you and Ryan were discussing). Some of the changes were mostly stylistic, but we wanted this to be a good template for adding this feature to our other libraries, so we thought it was worth some extra effort to make the code better match our internal coding style and preferences. Thanks for your contribution!

Kevin