Not working with Core2 and AWS EduKit

[framinlab]

Just received a unit. Have got the encoder example working with the regular Core device so I know the hardware is fine. But with the Core2 and AWS EduKit it only returns an encoder value of -1. The LED lights and the button do not work.

Same code, except header is changed to:
#include <M5Core2.h>
(also tried it without).

Will dive into debugging, but thought you might want to test it.

EDIT: Dove deep with a logic analyzer. Turns out the problem is multi-faceted:

• On Core2 devices, there are two I2C busses and the external signals are on pins 32(SDA) and 33(SCL). On Core devices there's only one on ports 21(SDA) and 22(SCL).
• The encoder, in its begin method explicitly calls Wire.begin with SDA and SCL pre-defined to 21 and 22. The default doesn't work on the Core2, even if the Wire/two-wire initialization is done in the M5.begin.

The solution is to explicitly set up I2C on the Core2 for the ENCODER:

#include <M5Core2.h>

UNIT_ENCODER sensor;
...
void setup() {
    M5.begin(true, true, true, true); // the last parameter has to be 'true' to turns on I2C
    sensor.begin(&Wire, ENCODER_ADDR, 32, 33); // Explicitly set up SDA/SCL
    ...

A better way would be to NOT call Wire.begin in the ENCODER begin method, but assume that it's been already set up either by the M5.begin method or set up explicitly upstream. This is how the UNIT_ENV::init method does it. Which lets the same code work for both Core and Core2 devices.

I won't push out a pull-request since it would be a breaking API change, but something like this would work better. Also, you don't need to save _scl, _sda, and _speed as private variables in the class declaration since they wouldn't be used anywhere else:

UNIT_ENCODER.h:

class UNIT_ENCODER {
   private:
    uint8_t _addr;
    TwoWire* _wire;
...

   public:
    void begin(TwoWire* wire = &Wire, uint8_t addr = ENCODER_ADDR);
...

UNIT_ENCODER.cpp:

void UNIT_ENCODER::begin(TwoWire *wire, uint8_t addr) {
    _wire  = wire;
    _addr  = addr;
}

With this change, the client code would look like:

void setup() {
    M5.begin(true, true, true, true); // the last parameter has to be 'true' to turns on I2C
    sensor.begin();
    ...