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

[Feature] IMU cleanups #1022

Closed
laurensvalk opened this issue Mar 31, 2023 · 13 comments
Closed

[Feature] IMU cleanups #1022

laurensvalk opened this issue Mar 31, 2023 · 13 comments
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: control Issues involving control system algorithms topic: imu Issues related to IMU/gyro/accelerometer

Comments

@laurensvalk
Copy link
Member

This task is split from pybricks/pybricks-micropython#156 so we can look at it separately.


https://github.com/pybricks/pybricks-micropython/blob/0a0332f5a9cb8b33b9d8f67f85badf51a408331b/lib/pbio/drv/imu/imu_lsm6ds3tr_c_stm32.c#L203

We probably need to verify what the actual sampling rate is, or measure it as we go. Similarly, we'll want to check if we are occasionally missing a few samples or not.

We'll also need to check this on Technic Hub, where the I2C setup is slightly different, in case there are any difference.

Once that is settled, we may need to experimentally verify lsm6ds3tr_c_from_fs2000dps_to_mdps and adjust as needed. Possibly per axis, see also below.


There is currently no adjustment for misalignment of the chip within the hub. This is possible using a calibration routine (see #943), but the benefit is fairly small for everyday users.

However, it may be useful to do this locally when we test for sample rates as above, to ensure we register the full Z measurement when tweaking lsm6ds3tr_c_from_fs2000dps_to_mdps.


In pbio, there is currently not a global pbio_imu instance that gets passed around, since there is only one imu instance. It may still be useful to make the code easier to follow.


Originally posted by @laurensvalk in pybricks/pybricks-micropython#156 (comment)

@laurensvalk laurensvalk added enhancement New feature or request topic: control Issues involving control system algorithms topic: imu Issues related to IMU/gyro/accelerometer software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) labels Mar 31, 2023
@laurensvalk laurensvalk transferred this issue from pybricks/pybricks-micropython Apr 1, 2023
@laurensvalk
Copy link
Member Author

Hmm... if [init] fails, won't calling other functions later cause a crash due to uninitialized values?

Originally posted by @dlech in pybricks/pybricks-micropython#156 (comment)

@laurensvalk laurensvalk changed the title [Feature] Review IMU sample rate [Feature] IMU cleanups Apr 1, 2023
@laurensvalk
Copy link
Member Author

laurensvalk commented Apr 1, 2023

Should raise NotImplementedError instead?

Originally posted by @dlech in pybricks/pybricks-micropython#156 (comment)

should this return PBIO_ERROR_INVALID_ARG if use_gyro is true on move hub (#if !PBIO_CONFIG_DRIVEBASE_USE_GRYO`)?

Originally posted by @dlech in pybricks/pybricks-micropython#156 (comment)

Probably better struct packing/smaller code size if this is the last item in the struct.

Originally posted by @dlech in pybricks/pybricks-micropython#156 (comment)

Intuitive drivebase integration still needs a bit of work. The COUNTERCLOCKWISE direction should probably also be done at the pbio level, not the module as we do know.

@laurensvalk
Copy link
Member Author

Probably can do without the union here (we can cast to float * if we really need to but better if we just use the struct everywhere).

Or if we really must keep the union, the outer struct is redundant and we can just say typedef union {.

Originally posted by @dlech in pybricks/pybricks-micropython#156 (comment)

@laurensvalk
Copy link
Member Author

laurensvalk commented Apr 1, 2023

On Technic Hub, it never becomes stationary with the default settings. So as per OP, we need to dig into the sensor configuration a bit more. It does become stationary if you double them to:

hub.imu.set_stationary_thresholds(angular_velocity=3, acceleration=500)

EDIT: Solved in #1026

@laurensvalk
Copy link
Member Author

We probably need to verify what the actual sampling rate is, or measure it as we go. Similarly, we'll want to check if we are occasionally missing a few samples or not.

I wonder if the scale varies slightly per hub. One one hub, the heading seems to be off by a few degrees after one full rotation.

@dlech
Copy link
Member

dlech commented Apr 1, 2023

  • Rename orientation.h to imu.h
  • Drop #include <pbdrv/imu.h> and fix copyright date in above
  • Rename pbio_orientation_set_base_orientation() to pbio_imu_set_base_orientation()
  • Mark pbio_imu_init() as internal (doxygen `@condition INTERNAL``) or move to separate header file.

@laurensvalk
Copy link
Member Author

Yeah, those are already done and pushed, just not merged to master yet.

@laurensvalk
Copy link
Member Author

It turns out the sampling rate of the pbdrv_imu_lsm6ds3tr_c_stm32_process is quite different even between two Prime Hubs (826 Hz, vs 844 Hz). Once running on a particular hub, it is very constant.

It would be easy enough to compensate by measuring the frequency at runtime, but is there an explanation for this?

@laurensvalk
Copy link
Member Author

It would be easy enough to compensate by measuring the frequency at runtime, but is there an explanation for this?

Done via pybricks/pybricks-micropython@1dbc334

@dlech
Copy link
Member

dlech commented Apr 4, 2023

but is there an explanation for this?

Since the IMU doesn't have an external crystal oscillator, there is never going to be an accurate (as noted) or stable (e.g. could change with temperature) clock on the IMU chip.

@laurensvalk
Copy link
Member Author

laurensvalk commented Apr 7, 2023

@laurensvalk
Copy link
Member Author

The most critical parts here are now done and the implementation now matches the API proposal in pybricks/pybricks-api#137

@laurensvalk
Copy link
Member Author

This is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) topic: control Issues involving control system algorithms topic: imu Issues related to IMU/gyro/accelerometer
Projects
None yet
Development

No branches or pull requests

2 participants