Feature request: Allow using secondary Wire buses

[matthijskooijman]

On some boards, such as STM32 boards (and probably SAMD boards too), there are multiple I²C controllers available, typically named Wire, Wire1, etc.

Currently, this library always uses Wire hardcoded, but it would be convenient if a secondary bus could also be used.

There is already a sensirion_i2c_select_bus(uint8_t idx) function declared, but it is never defined anywhere. Also, since I'm not sure whether there is a standard way to detect how much Wire objects are defined, this API might be hard to implement on Arduino. Therefore, I'd suggest implementing a selection function that actually passes the Wire, Wire1, etc objects as a reference. The type of these seems to be TwoWire on most cores, but just in case cores use other names, I'd suggest using decltype(Wire) instead.

So, that would be something like this (untested, should probably be split between .h and .cpp files as well, but just to get the general idea):

using SensirionWireType = decltype(Wire);

SensirionWireType *SensirionWire = &Wire;

sensirion_i2c_select_bus(SensirionWireType& bus)
{
  SensirionWire = &bus;
}

And then replace all other Wire. with SensirionWire-> too.

This allows a sketch to do e.g.:

sensirion_i2c_select_bus(Wire1);
sensirion_i2c_init();

It can even switch between multiple buses at runtime, as long as it does call sensiron_i2c_init() once for each bus beforehand.

[winkj]

Sounds good; this is partly because all the interfaces in sensirion_hw_i2c_implementation.cpp use generic - i.e. not platform specific - APIs, which is why we had the sensirion_i2c_select_bus() call in there with an index; changing the interface to an Arduino SDK one would break compilation on other platforms.

So this will require a little bit more thought on our end, or potentially just foregoing compatibility with embedded-common in the interest of better support of Arduino platform needs

[matthijskooijman]

changing the interface to an Arduino SDK one would break compilation on other platforms.

Yeah, so maybe adding a function (i.e. sensirion_i2c_select_wire(...) or so) could work? And then just ignoring the existing sensirion_i2c_select_bus()?

Even though the existing index-based API isn't necessarily bad, I'm afraid it will be hard to implement fully portably Arduino, since there is no generic index -> Wire object API, or a standardized and documented way to figure out how many wire interfaces there are.

Looking more closely, it does seem that there is a WIRE_INTERFACES_COUNT macro that is at least used by the Arduino sam, Arduino samd, Adafruit samd, stm32l0 and stm32l4 cores, so that could be a reasonably standard way to do this maybe? It seems that Teensy implements WIRE_IMPLEMENT_WIREX macros instead. So an alternative could be maybe:

int16_t sensirion_i2c_select_bus(uint8_t bus_idx) {
  switch (bus_idx) {
    case 0:
      SensirionWire = &Wire;
      return 0;
#if defined WIRE_IMPLEMENT_WIRE1 || WIRE_INTERFACES_COUNT > 1
    case 1:
      SensirionWire = &Wire1;
      return 0;
#endif
#if defined WIRE_IMPLEMENT_WIRE2 || WIRE_INTERFACES_COUNT > 2
    case 1:
      SensirionWire = &Wire2;
      return 0;
#endif
#if defined WIRE_IMPLEMENT_WIRE3 || WIRE_INTERFACES_COUNT > 3
    case 1:
      SensirionWire = &Wire3;
      return 0;
#endif
    default:
      return STATUS_FAIL;
  }
}

(as an alternative implementation for sensirion_i2c_select_bus, the other changes suggested above are still needed)

[abrauchli]

The generic usage we thought of is quite similar but shifts more logic in to the i2c read and write functions based on the current context. A quick recap: The HAL implementation is aware of all three implementations: sensirion_i2c_read, sensirion_i2c_write and sensirion_i2c_select_bus - the latter being the trigger to switching the context (since we don't pass context objects, those have to be maintained separate, e.g. as globals.) Thus, we modify the behavior of sensirion_i2c_read depending on the context, which is triggered by the index. E.g. the read function would then look like this switch (index) { case 1: Wire1.read(); // ...

[matthijskooijman]

Ah, that would probably be even better, since then the code will even work when Wire and Wire1 have different types somehow.

[mirlang]

+1

[winkj]

The cleanest way forward would be to port this to https://github.com/Sensirion/arduino-core/ since that allows selecting Wire objects already.

In the interest of time, maybe we could just modify sensiron_i2c_init() to take a reference to an initialized Wire object... @psachs, thoughts?

[matthijskooijman]

Any further thoughts on this issue? Would be good if we could switch back to an unmodified arduino-sps library for our project.

[psachs]

If it helps you in any way. We already have a newer driver for SCD30 that allows you to provide the Wire and I2C address on initialization. The driver is available on Arduino library and PlatformIO.

https://github.com/Sensirion/arduino-i2c-scd30

[matthijskooijman]

@psachs But that driver for SCD30 does not work for SPS030, right? Or was your suggestion to modify this library to allow bus selection in the same way as the SCD30 library? That does not fully apply, since AFAICS the SCD30 library does not use the common sensirion i2c code like sps030.

[psachs]

@matthijskooijman yes sorry for my wrong suggestion. This driver is for a different sensor and I somehow managed to reference the wrong driver.

Since we are not planning to release a driver for sps30 with the new interface soon, I think the easiest solution is what @winkj suggested to allow passing of an initialized Wire object .

[matthijskooijman]

Thanks for the confirmation. I've prepared a PR that implements this suggestion, see #54. Would be nice if that could be merged and released, then I can revert back to the default version of this library (and not have our users run into trouble if they accidentally reverted to the default version using the Arduino library manager).