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

lis3mdl with two sensors not working #10740

Closed
hdiethelm opened this issue Oct 22, 2018 · 6 comments
Closed

lis3mdl with two sensors not working #10740

hdiethelm opened this issue Oct 22, 2018 · 6 comments

Comments

@hdiethelm
Copy link

hdiethelm commented Oct 22, 2018 •
edited by AuterionWrikeBot
Loading

I found a problem with the starting order if two lis3mdl sensors are available.

Version: stable branch

In QGroundControl PREFLIGHT FAIL: MAG 1 SELFTEST FAILED showed up, so I tested some things.

Configuration: Pixracer r15 with lis3mdl onboard and lis3mdl on GPS module:
https://store.mrobotics.io/mRo-GPS-u-Blox-Neo-M8N-HMC5983-Compass-p/mro-gps003-mr.htm

In the default rc.sensors for PX4FMU_V4, the order is the following:
lis3mdl -X start
lis3mdl start

When I display the sensor values, the onboard sensor shows all 0:

nsh> lis3mdl stop
nsh> lis3mdl -X start
LIS3MDL_I2C on I2C bus 1 at 0x1e (bus: 100 KHz, max: 400 KHz)
INFO [lis3mdl] Poll rate set to max (80hz)
INFO [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl start
LIS3MDL_SPI on SPI bus 1 at 7 (11000 KHz)
INFO [lis3mdl] Poll rate set to max (80hz)
INFO [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl -X status
WARN [lis3mdl] running on bus: 2 (/dev/lis3mdl_ext)

lis3mdl_read: 504 events, 1153644us elapsed, 2288us avg, min 2011us max 3685us 270.915us rms
lis3mdl_comms_errors: 0 events
INFO [lis3mdl] poll interval: 13 ticks
sensor_mag_s
timestamp: 389383395 (0.008588 seconds ago)
error_count: 0
x: 0.799
y: -0.324
z: -0.242
range_ga: 4.000
scaling: 0.000
temperature: 35.500
device_id: 335369
x_raw: 5471
y_raw: -2241
z_raw: -1676
is_external: 1
report queue 2/112 (0/2 @ 2002ab70)
nsh> lis3mdl -S status
WARN [lis3mdl] running on bus: 3 (/dev/lis3mdl_spi)

lis3mdl_read: 0 events, 0us elapsed, 0us avg, min 0us max 0us 0.000us rms
lis3mdl_comms_errors: 0 events
INFO [lis3mdl] poll interval: 0 ticks
sensor_mag_s
timestamp: 0
error_count: 0
x: 0.000
y: 0.000
z: 0.000
range_ga: 0.000
scaling: 0.000
temperature: 0.000
device_id: 0
x_raw: 0
y_raw: 0
z_raw: 0
is_external: 0
report queue 2/112 (2/2 @ 2002b3b0)
nsh>

When I start them the other way around, they run just fine. However, starting order should not matter:

nsh> lis3mdl stop
nsh> lis3mdl -S start
LIS3MDL_SPI on SPI bus 1 at 7 (11000 KHz)
INFO [lis3mdl] Poll rate set to max (80hz)
INFO [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl -X start
LIS3MDL_I2C on I2C bus 1 at 0x1e (bus: 100 KHz, max: 400 KHz)
INFO [lis3mdl] Poll rate set to max (80hz)
INFO [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl -S status
WARN [lis3mdl] running on bus: 3 (/dev/lis3mdl_spi)

lis3mdl_read: 480 events, 413035us elapsed, 860us avg, min 652us max 2378us 170.875us rms
lis3mdl_comms_errors: 0 events
INFO [lis3mdl] poll interval: 13 ticks
sensor_mag_s
timestamp: 522282524 (0.007740 seconds ago)
error_count: 0
x: -0.098
y: 0.390
z: 0.987
range_ga: 4.000
scaling: 0.000
temperature: 40.250
device_id: 329482
x_raw: -645
y_raw: 2619
z_raw: 6761
is_external: 0
report queue 2/112 (0/2 @ 2002acd0)
nsh> lis3mdl -X status
WARN [lis3mdl] running on bus: 2 (/dev/lis3mdl_ext)

lis3mdl_read: 426 events, 1038242us elapsed, 2437us avg, min 2034us max 4079us 386.869us rms
lis3mdl_comms_errors: 0 events
INFO [lis3mdl] poll interval: 13 ticks
sensor_mag_s
timestamp: 525748397 (0.008619 seconds ago)
error_count: 0
x: 0.801
y: -0.327
z: -0.227
range_ga: 4.000
scaling: 0.000
temperature: 35.250
device_id: 335369
x_raw: 5512
y_raw: -2214
z_raw: -1519
is_external: 1
report queue 2/112 (0/2 @ 2002b570)
nsh>
@dakejahl
Copy link
Contributor

dakejahl commented Oct 22, 2018 •
edited
Loading

I can take a look at the code tomorrow and see what's going on. @dagar we should ModuleBaseMulti and Mag this driver. Idk if I want to do it, but it would be good to move all drivers in that direction yeah?

@hdiethelm
Copy link
Author

Just looked at the code and I think I found the bug:
lis3mdl::start(bus_id, rotation) called with LIS3MDL_BUS_ALL initializes all in a loop.
Next, lis3mdl::calibrate(bus_id) and lis3mdl::init(bus_id) access just the first device.

Now if the first device is initialized first and then the second with LIS3MDL_BUS_ALL, the calls to
lis3mdl::calibrate(bus_id) and lis3mdl::init(bus_id) go to the first device, so the second is not initialized.

calibrate / init / test / info and reset should have the loop over all devices also. Might be it is easier to do the loop all inside main and remove it in for start / stop.

When the second start is called with -S, everything works:

nsh> lis3mdl stop
nsh> lis3mdl -X start
LIS3MDL_I2C on I2C bus 1 at 0x1e (bus: 100 KHz, max: 400 KHz)
INFO  [lis3mdl] Poll rate set to max (80hz)
INFO  [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl -S start
LIS3MDL_SPI on SPI bus 1 at 7 (11000 KHz)
INFO  [lis3mdl] Poll rate set to max (80hz)
INFO  [lis3mdl] Set mag range to 4 Gauss
nsh> lis3mdl -X status
WARN  [lis3mdl] running on bus: 2 (/dev/lis3mdl_ext)

lis3mdl_read: 531 events, 1305410us elapsed, 2458us avg, min 2053us max 4111us 357.943us rms
lis3mdl_comms_errors: 0 events
INFO  [lis3mdl] poll interval:  13 ticks
 sensor_mag_s
	timestamp: 445725414 (0.005689 seconds ago)
	error_count: 0
	x: 0.786
	y: -0.322
	z: -0.232
	range_ga: 4.000
	scaling: 0.000
	temperature: 33.000
	device_id: 335369
	x_raw: 5397
	y_raw: -2203
	z_raw: -1659
	is_external: 1
report queue	2/112 (0/2 @ 10009ef0)
nsh> lis3mdl -S status
WARN  [lis3mdl] running on bus: 3 (/dev/lis3mdl_spi)

lis3mdl_read: 697 events, 649888us elapsed, 932us avg, min 700us max 1287us 158.250us rms
lis3mdl_comms_errors: 0 events
INFO  [lis3mdl] poll interval:  13 ticks
 sensor_mag_s
	timestamp: 451122611 (0.009870 seconds ago)
	error_count: 0
	x: -0.106
	y: 0.382
	z: 0.993
	range_ga: 4.000
	scaling: 0.000
	temperature: 39.000
	device_id: 329482
	x_raw: -727
	y_raw: 2617
	z_raw: 6757
	is_external: 0
report queue	2/112 (0/2 @ 2002aa50)
nsh>

@dakejahl
Copy link
Contributor

@hdiethelm If you'd like to fix this and submit a PR I'd be happy to review and help test the code. I feel I should warn you the LIS3MDL is very temperature sensitive. We found that exceeding +/- 10°C from the temperature at which the device was calibrated is enough to cause unstable flights and fly-aways. We've since switched to using the AK8963 on board the mpu9250 as our primary mag sensor.

hdiethelm pushed a commit to hdiethelm/Firmware that referenced this issue Oct 23, 2018
Fix lis3mdl handling when multiple sensors are available
6026f26 
Fixes PX4#10740
@hdiethelm
Copy link
Author

^^Sorry about all the messages. As you see, I'm working on it. But I did not know that github tracks references on issues and just force pushed on my own branch. ;-)
It's more complicated to make multiple device handling with one driver as it seamed at first glance, but I think i found a way. I still have to test it.

@dakejahl
Copy link
Contributor

dakejahl commented Oct 23, 2018 •
edited
Loading

I've done this work already with ModuleBaseMulti as found in https://github.com/PX4/Firmware/blob/985f645cdc62a68f1d9348b817256ccf8ae6eb50/src/platforms/px4_module_multi.h

Which is part of #10602. You can cherry pick this file px4_module_multi.h and use it to do multi-instance as well as follow the ModuleBase architecture that many other modules/drivers use. This file still needs review, but I've used it successfully in #10602 😄. You don't have to use it if you don't want to though, figured I'd try and save you some effort.

If you're unfamiliar with ModuleBase checkout src/drivers/heater src/modules/land_detector src/drivers/gps. Note that the setup/teardown is slightly different between tasks and workqueue implementations.

@hdiethelm hdiethelm mentioned this issue Oct 23, 2018
Closed
@hdiethelm
Copy link
Author

ModuleBaseMulti looks quite nice, however, I have already finished the modifications and tested them. At the moment, I have no time porting all to the new framework. Might be later.
I just create a pull request.

hdiethelm pushed a commit to hdiethelm/Firmware that referenced this issue Oct 24, 2018
Fix lis3mdl handling when multiple sensors are available
e277f4c 
Fixes PX4#10740
@hdiethelm hdiethelm mentioned this issue Oct 24, 2018
Merged
dagar pushed a commit that referenced this issue Nov 1, 2018
@dagar
lis3mdl: fix handling when multiple sensors are available (#10753)
3342270 
* lis3mdl: Report calibration successful when starting with -C option
* lis3mdl: Use PX4_OK, PX4_ERROR for return value / orb_unadvertise
* Fixes #10740
zenbotinc pushed a commit to zen-bot/PX4Firmware that referenced this issue Mar 7, 2019
@zenbotinc
lis3mdl: fix handling when multiple sensors are available (PX4#10753)
a1b7a4f 
* lis3mdl: Report calibration successful when starting with -C option
* lis3mdl: Use PX4_OK, PX4_ERROR for return value / orb_unadvertise
* Fixes PX4#10740
kevindehecker pushed a commit to tudelft/Firmware that referenced this issue Jun 11, 2019
@kevindehecker
lis3mdl: fix handling when multiple sensors are available (PX4#10753)
c1e9721 
* lis3mdl: Report calibration successful when starting with -C option
* lis3mdl: Use PX4_OK, PX4_ERROR for return value / orb_unadvertise
* Fixes PX4#10740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hdiethelm @dakejahl