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
Implement device driver for BNO055 9DOF sensor #14956
Conversation
Hi @Achilleshiel thanks a lot for the contribution and welcome to RIOT! I took a quick glimpse at the code, looks good, I'll take a more detailed look tomorrow. Travis pointed out a couple of nitpicks though. Also AFAIK BTW do you have some test output you can show? I don't have the driver to test it but I can trust your test results! |
43708b0
to
dd9fd72
Compare
Hi @fjmolinas, I think I have statisfied Travis now. Except that I have to squash this PR at some point =) The output of my test:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks almost good.
We recently updated the device driver implementation guide to encourage the use of errno codes instead of custom return codes. I won't insist on this point but if you feel like it and want to update this part, that would be nice.
Another thing that would be nice to add is a basic README for the test application.
/** | ||
* @brief Status and error return codes | ||
*/ | ||
enum { | ||
BNO055_OK = 0, /**< exit without error */ | ||
BNO055_NOBUS = ENXIO, /**< cannot connect to module on i2c bus */ | ||
BNO055_NODEV = ENODEV, /**< cannot read any data from module */ | ||
BNO055_NORW = EIO, /**< cannot read data from module */ | ||
BNO055_NOTREADY = EBUSY, /**< no new data ready for reading */ | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole enum should be removed and the errno codes directly returned in the implementation. The doxygen documentation also needs to be adapted.
#define BNO055_DIV_SCALE_ACC_MSS (100.0) /**> Acceleration Raw to m/s2 scaling */ | ||
#define BNO055_DIV_SCALE_ACC_G (1.0) /**> Acceleration Raw to g (9.81m/s2) scaling */ | ||
#define BNO055_DIV_SCALE_GYR_RPS (900.0) /**> Gyroscope Raw to rad/s scaling */ | ||
#define BNO055_DIV_SCALE_GYR_DPS (16.0) /**> Gyroscope Raw to deg/s scaling */ | ||
#define BNO055_DIV_SCALE_EUL_RAD (900.0) /**> Euler Data Raw to rad scaling */ | ||
#define BNO055_DIV_SCALE_EUL_DEG (16) /**> Euler Data Raw to deg scaling */ | ||
#define BNO055_DIV_SCALE_QUAT_UN (16384.0) /**> Quaternion data Raw to 0..1 (unit less) scaling */ | ||
#define BNO055_DIV_SCALE_TEMP_DC (1.0) /**> Temperature Data Raw to Celsius scaling */ | ||
#define BNO055_DIV_SCALE_TEMP_DF (0.5) /**> Temperature Data Raw to Fahrenheid scaling */ | ||
#define BNO055_DIV_SCALE_MAG (16.0) /**> Magnetometer Data Raw to microtesla scaling */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define BNO055_DIV_SCALE_ACC_MSS (100.0) /**> Acceleration Raw to m/s2 scaling */ | |
#define BNO055_DIV_SCALE_ACC_G (1.0) /**> Acceleration Raw to g (9.81m/s2) scaling */ | |
#define BNO055_DIV_SCALE_GYR_RPS (900.0) /**> Gyroscope Raw to rad/s scaling */ | |
#define BNO055_DIV_SCALE_GYR_DPS (16.0) /**> Gyroscope Raw to deg/s scaling */ | |
#define BNO055_DIV_SCALE_EUL_RAD (900.0) /**> Euler Data Raw to rad scaling */ | |
#define BNO055_DIV_SCALE_EUL_DEG (16) /**> Euler Data Raw to deg scaling */ | |
#define BNO055_DIV_SCALE_QUAT_UN (16384.0) /**> Quaternion data Raw to 0..1 (unit less) scaling */ | |
#define BNO055_DIV_SCALE_TEMP_DC (1.0) /**> Temperature Data Raw to Celsius scaling */ | |
#define BNO055_DIV_SCALE_TEMP_DF (0.5) /**> Temperature Data Raw to Fahrenheid scaling */ | |
#define BNO055_DIV_SCALE_MAG (16.0) /**> Magnetometer Data Raw to microtesla scaling */ | |
#define BNO055_DIV_SCALE_ACC_MSS (100.0f) /**> Acceleration Raw to m/s2 scaling */ | |
#define BNO055_DIV_SCALE_ACC_G (1.0f) /**> Acceleration Raw to g (9.81m/s2) scaling */ | |
#define BNO055_DIV_SCALE_GYR_RPS (900.0f) /**> Gyroscope Raw to rad/s scaling */ | |
#define BNO055_DIV_SCALE_GYR_DPS (16.0f) /**> Gyroscope Raw to deg/s scaling */ | |
#define BNO055_DIV_SCALE_EUL_RAD (900.0f) /**> Euler Data Raw to rad scaling */ | |
#define BNO055_DIV_SCALE_EUL_DEG (16f) /**> Euler Data Raw to deg scaling */ | |
#define BNO055_DIV_SCALE_QUAT_UN (16384.0f) /**> Quaternion data Raw to 0..1 (unit less) scaling */ | |
#define BNO055_DIV_SCALE_TEMP_DC (1.0f) /**> Temperature Data Raw to Celsius scaling */ | |
#define BNO055_DIV_SCALE_TEMP_DF (0.5f) /**> Temperature Data Raw to Fahrenheid scaling */ | |
#define BNO055_DIV_SCALE_MAG (16.0f) /**> Magnetometer Data Raw to microtesla scaling */ |
/** | ||
* @brief change the current I2C register page | ||
* | ||
* @param[in] dev device descriptor of IMU | ||
* @param[in] page desired page number (0 or 1) | ||
* | ||
* @retval BNO055_OK on success | ||
* @retval BNO055_NOREAD if reading mag data is not possible | ||
* @retval BNO055_NOTRDY if no new data is available | ||
*/ | ||
int bno055_set_page(const bno055_t *dev, uint8_t page); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that something that should be exposed to the user?
* @ingroup drivers_saul | ||
* @brief Device driver for the Bosch BNO055 9-axis sensor | ||
* | ||
* This driver provides @ref drivers_saul capabilities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't provide a bno055_saul.c
for that 😉
* @name BNO055 chip id registers and who am I values | ||
* @{ | ||
*/ | ||
#define BNO055_REG_CHIPID (0x00U) /**> chip id register */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define BNO055_REG_CHIPID (0x00U) /**> chip id register */ | |
#define BNO055_REG_CHIPID (0x00U) /**< chip id register */ |
This doxygen issue is present all over the place.
In current master:
$ git grep "/\*\*>" | wc -l
0
git grep "/\*\*<" | wc -l
373126
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
@Achilleshiel there seems to be very little missing here, any chance you can take a look at the missing comments? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Contribution description
Driver for the BOSCH BNO055 IMU-sensor. This sensor has an internal MCU that does a lot of automatic sensor fusion magic for you.
I added some conversion functions for the float values as some sensor fusion outputs are only usefull when they are specified as floats. However, one can still read the raw integer data. And depending on the setting it can easily be scaled to SI-units.
The driver has read functions for:
It can also select units for:
Testing procedure
I made a testing-program that reads and prints all the sensor values.
Issues/PRs references
--