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.

Sign up for GitHub

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

Jump to bottom

drivers: add lis2mdl mag driver #14844

Merged
merged 4 commits into from May 7, 2020
Merged

drivers: add lis2mdl mag driver #14844

merged 4 commits into from May 7, 2020

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented May 6, 2020

It has less functionality than the lis3mdl, but supposedly the lis2mdl is better than the lis3mdl.

@bkueng bkueng requested a review from dagar May 6, 2020 07:00
dagar
dagar reviewed May 6, 2020
View reviewed changes
src/drivers/drv_sensor.h Outdated Show resolved Hide resolved
dagar
dagar reviewed May 6, 2020
View reviewed changes
src/drivers/magnetometer/lis2mdl/lis2mdl_i2c.cpp Outdated
}

int
LIS2MDL_I2C::ioctl(unsigned operation, unsigned &arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IOCTLs can be removed.

@bkueng
Copy link
Member Author

bkueng commented May 7, 2020

Lot's of copy-paste. I did another cleanup round.

dagar
dagar previously approved these changes May 7, 2020
View reviewed changes
@dagar
Copy link
Member

dagar commented May 7, 2020

I suppose we should actually build it?

https://github.com/PX4/Firmware/blob/3a7fd3f63f6a9dbdb93f8695edf30cb82b99350a/src/drivers/magnetometer/CMakeLists.txt#L40

@dagar dagar merged commit 678c3fe into master May 7, 2020
@dagar dagar deleted the mag_lis2mdl branch May 7, 2020 19:21
@bkueng
Copy link
Member Author

bkueng commented May 11, 2020

I suppose we should actually build it?

I wasn't adding it for flash, and as no other board than v5x is using it.

@ryankierteal
Copy link

Has this been tested in hardware? I'm thinking specifically of SPI on the STM32F7. By default the LIS2MDL SPI interface operates in 3-wire mode (p. 25 of datasheet):
image
I haven't found anywhere that bit 2 of CFG_REG_C is being set, and it doesn't look like 3-wire SPI is supported by the driver framework.

Perhaps LIS2MDL_SPI::init() needs this bit set before ADDR_WHO_AM_I is read, and LIS2MDL::init() needs to read ADDR_CFG_REG_C, OR the readback result with CFG_REG_C_BDU, then write the new value to ADDR_CFG_REG_C?

@bkueng
Copy link
Member Author

bkueng commented May 18, 2020

I only tested this with I2C. Feel free to make the required changes for SPI and create a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagar dagar dagar left review comments

Assignees
No one assigned
Labels
Admin: Enhancement (improvement) 💡 Drivers 🔧 Sensors, Actuators, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bkueng @dagar @ryankierteal