Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read selector #34

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Read selector #34

wants to merge 3 commits into from

Conversation

eringerli
Copy link
Contributor

@eringerli eringerli commented Apr 28, 2022

With a selector (which defaults to the behaviour as before this commit), the calling code can specify which kind data to transfer from the IMU.

All the data transfer code is centralized in _read() now, the various getEvent() methods call it with a selector. As only the data needed is transfered, this should result in a small performance gain.

I removed some hard coded conversions, all reads should now respect the set ranges. I also added a method to read out the accelerometer and gyroscope at the same time.

As all the errors bubble up, this solves issue #26 and makes PR #27 obsolete.

Beware, this PR is based on PR #33:
Screenshot_20220429_004110

With dedicated converter methods, the LSM6DSO32 can do its conversion for
the bigger ranges there, so duplicated code in _read() can be removed.
Also, getAccelRange() and setAccelRange() are exactly the same as in
the base class, the difference is only the enum type. Replaced these
methods with a call to the baseclass with the casted range.
@eringerli eringerli force-pushed the read-selector branch 5 times, most recently from fa4fad8 to d34a85c Compare April 29, 2022 04:29
With this selector (which defaults to the behaviour as before this
commit), the calling code can specify which kind data to transfer from the
IMU. All the data transfer code is centralized in _read() now, the various
getEvent() methods only transfer the data they need. I removed some hard
coded conversions (all read outs now respect the set ranges) and also added
a method to read out the accelerometer and gyroscope data at the same time.
eringerli added a commit to eringerli/Adafruit_LSM6DS that referenced this pull request May 1, 2022
@ladyada ladyada requested a review from caternuson May 2, 2022 04:37
This was referenced May 4, 2022
@eringerli
Copy link
Contributor Author

@caternuson Did you have the time to read through this PR?

@caternuson
Copy link
Contributor

This needs #33, so looking at that one first.

@f-peri
Copy link

f-peri commented Mar 11, 2023

@eringerli and @caternuson thanks for your efforts to update this library. I tested your pull and found that the readAcceleration and readGyro members were not working. I investigated a bit and believe I found the typo in the following:

constexpr uint8_t registersAndNum[] = { LSM6DS_OUT_TEMP_L, 14, LSM6DS_OUTX_L_G, 12, LSM6DS_OUTX_L_G, 6, LSM6DS_OUTX_L_A, 6, LSM6DS_OUT_TEMP_L, 2};
seems like the wrong registers were referenced. If I change to the following:
constexpr uint8_t registersAndNum[] = { LSM6DS_OUT_TEMP_L, 14, LSM6DS_OUTX_L_G, 12, LSM6DS_OUTX_L_A, 6, LSM6DS_OUTX_L_G, 6, LSM6DS_OUT_TEMP_L, 2};
the functions appear to work as designed. I am in no way a pro at this, so I don't presume to know if there are other bits of code that should be checked.

thanks again.

@eringerli
Copy link
Contributor Author

You're right, @f-peri. I want to rebase this commit anyway, so I have a look at it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants