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

Bug in setLowPass Filter Function - Accel lowpass is never configured #40

Open
savejeff opened this issue Jan 5, 2020 · 4 comments
Open
Assignees
Labels

Comments

@savejeff
Copy link

savejeff commented Jan 5, 2020

The function "imu.setLPF();" does not set the Accel Low Pass register but only the Gyro and Temperature Lowpass. i thing that is due to the difference to the very similar MPU6050 where both accel and gyro LP are set in the same register.

Hier the incorrect function call from the Example Library


// setLPF() can be used to set the digital low-pass filter
// of the accelerometer and gyroscope. (<- This is Wrong)
// Can be any of the following: 188, 98, 42, 20, 10, 5
// (values are in Hz).
imu.setLPF(42); // Set LPF corner frequency to 5Hz


The correct way to set the Accel Low Pass is to write to the MPU9250_ACCEL_CONFIG_2 register.
1.) Disable Bypass by setting the bypass bit to 0 (it is inverted from the given table in the datasheet
2.) Set lowpass config bits to the desired value.

here is an example code:


void MPU9250Class::setAccelLowPass(int f_cutoff)
{

#define REG_MASK_ACCEL_LP 0b1111
#define REG_VAL_ACCEL_LP_218Hz 0
#define REG_VAL_ACCEL_LP_99Hz 2
#define REG_VAL_ACCEL_LP_44Hz 3
#define REG_VAL_ACCEL_LP_21Hz 4
#define REG_VAL_ACCEL_LP_10Hz 5
#define REG_VAL_ACCEL_LP_5Hz 6

	uint8_t data = 0;
	if (f_cutoff >= 218)
		data = REG_VAL_ACCEL_LP_218Hz;
	else if (f_cutoff >= 99)
		data = REG_VAL_ACCEL_LP_99Hz;
	else if (f_cutoff >= 44)
		data = REG_VAL_ACCEL_LP_44Hz;
	else if (f_cutoff >= 21)
		data = REG_VAL_ACCEL_LP_21Hz;
	else if (f_cutoff >= 10)
		data = REG_VAL_ACCEL_LP_10Hz;
	else
		data = REG_VAL_ACCEL_LP_5Hz;

	imu.write_mask(mpu9250_register::MPU9250_ACCEL_CONFIG_2, REG_MASK_ACCEL_LP, data);

}

The write_mask function is one I have added to make the i2c function accessible from the outside to implement further functions but the logic should be clear.

It should also me noted that setLPF should be called after setting the sample rate because setting the samplerate changes the LPF to half the sample frequency.
That is also a bug in the basic example:

  // setLPF() can be used to set the digital low-pass filter
  // of the accelerometer and gyroscope.
  // Can be any of the following: 188, 98, 42, 20, 10, 5
  // (values are in Hz).
  imu.setLPF(5); // Set LPF corner frequency to 5Hz

  // The sample rate of the accel/gyro can be set using
  // setSampleRate. Acceptable values range from 4Hz to 1kHz
  imu.setSampleRate(10); // Set sample rate to 10Hz
@santaimpersonator
Copy link

santaimpersonator commented Jan 8, 2020

Thanks for looking into this issue! Feel free to file a pull request and I'll update the library.

@savejeff
Copy link
Author

savejeff commented Jan 8, 2020

Hi!
I myself have never contributed to a GitHub project but if I find the time I'll try to create a fix.
while working this the code I had problems understanding the struct that holds the registers. I understand its to make it compatible with multiple sensors but its very hard to find a good solution without step-by-step debugging

@santaimpersonator
Copy link

Contributing is relatively simple:

  1. you create a fork of the repository
  2. make the changes in that fork
  3. submit a pull request to the original project

GitHub has a simple guide on forking a project that should help get you started. After the pull request is submitted, we review the changes before accepting them. Once that is done, we will publish a release to update the Arduino library.

@savejeff
Copy link
Author

Thanks for the little guideline.

For me personally its less of a technical problem but a time problem.
I'm not sure if I find the time to implement it in the library. But if I do I'll submit a pull request.

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

No branches or pull requests

2 participants