Fix bug where begin() would return false even on success

[egnor]

Very simple change to fix a very simple bug where LSM6DS::begin() wouldn't return true even if it succeeded.

[ketan]

Please merge this. All the examples fail because of this bug. I can validate that this fix works.

[caternuson]

What specific sensor is being used and what is an example sketch that demonstrates this issue?

The false return might be intentional, as a way to prevent using the LSM6DS class directly. One of the derived classes should be used. Note that none of the examples import and use LSM6DS directly.

[egnor]

@caternuson if so that's direly bad design! (hopefully not yours, I gather you're speculating as much as the rest of us?)

• it's never commented or documented that LSM6DS by itself should not be used
• the return false; is not associated with an error message or even a comment
• the _init() method is actually protected, so can't be called directly regardless; it's called by the base class's begin(), which
• there are much better ways to prevent direct use of the base class

If we can confirm that the base class is not intended to be used, or otherwise establish the design intent, I am happy to revise the PR accordingly!

[caternuson]

If you can provide more specifics, that would help. What is the hardware being used? What is the sketch that is not working?

[egnor]

In my case I was in fact using the LSM6DS class directly (and it works totally fine if you ignore the begin() return value), with a custom board. I can't speak to @ketan's experience.

I don't need support, I just want to make the code good, and figured I'd be a good open source citizen by submitting a fix as I saw it.

[ketan]

Here is my snippet of code that does not work as expected:

#include <Adafruit_LSM6DS.h>

Adafruit_LSM6DS lsm6ds;

void setup() {
  Serial.begin(115200);
...
  if (!lsm6ds.begin_I2C(0x6b)) {
    Serial.println("Failed to initialize LSM6DS");
    while (1)
      ;
  }

  sensors_event_t a, g, t;
  lsm6ds.getEvent(&a, &g, &t);
...
}

void loop() {
...
}

The root cause is that Adafruit_LSM6DS::begin_I2C seems to return _init() - which essentially always returns a false.

[caternuson]

@ketan That code is not in any of the examples from this repo. So not sure what your comment All the examples fail is applying to?

There is no specific LSM6DS sensor. It is an entire series of sensors. There are lots of sensor variations with LSM6DS in the name.
https://www.st.com/content/st_com/en/search.html#q=lsm6ds-t=products-page=1

What actual sensor are you using? Where did you get that example code from?

[ketan]

@caternuson — I can see what you mean here. I was importing Adafruit_LSM6DS.h instead of Adafruit_LSM6DS33.h which corresponds to the sensor I use, and the code works. My bad for mis-reading, and appreciate the quick turn around time.

Problem exists between keyboard and chair.

[caternuson]

@ketan Cool. Glad it was that simple. If you are finding any out of date instructions - like in a Learn guide, please link that here so it can get updated. This library did change with time to support various LSM6DS series sensors. So it's possible there is some info somewhere else that did not get updated and is showing incorrect / out of date usage.

@egnor What specific sensor are you using on your custom board?

[egnor]

I'm using the LSM6DSM which has no explicit subclass.

The main Learn guide gives examples but does not talk about what classes can and can't be used. For reference, it links to the doxygen output which includes an entry for the base LSM6DS class which never says it should not be used directly.

As a process note @caternuson I would consider that this is a pull request (proposed change to the open source code), not a service ticket. Since you seem uninterested in accepting this PR, probably we should just close it, and I can open some other thread elsewhere about the right way to improve the documentation (at least) and code (ideally) to avoid this particular undocumented pitfall??

(Or you can just tell us to GTFO rather than trying to suggest improvements, if you don't have bandwidth to consider such things...?)

[caternuson]

Documentation has been updated to more explicitly reference LSM6DS as a base class not to be used directly.