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

Apogee mpu9150 baro sdlog #1498

Closed
wants to merge 20 commits into from

Conversation

philipan
Copy link
Contributor

integrated Gautiers patch to access baro on 9dofimu apogee boards and added SDLOG features for baro, sht75, airspeed_ets and temod (together with gps data). Example airframe is AULA/apogee_sdlog.xml (use together with radios/AULA/mx-16-aula.xml and fixedwing_fligh_trecoder.xml). In target sim the airspeed_ets still cannot find sdlog.h!

@@ -150,10 +153,19 @@ bool_t mpu60x0_configure_i2c_slaves(Mpu60x0ConfigSet mpu_set, void *mpu)
mpu_i2c->slave_init_status++;
break;
case MPU60X0_I2C_CONF_SLAVES_CONFIGURE:
#ifdef MPU9150_SLV_MAG
while(cpt<mpu_i2c->config.nb_slaves) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should not be done in a while loop, rather it should call mpu_i2c->config.slaves[cpt].configure(mpu_set, mpu) once and move to the next slave if it succeeded.
That also means that cpt would need to be static...

@flixr
Copy link
Member

flixr commented Dec 27, 2015

@philipan I don't quite understand the need to add anything special for the MPL3115 baro compared to the existing apogee driver... Could you please elaborate?

@philipan
Copy link
Contributor Author

Hi,

the existing Apogee driver only supports addressing the MPL3115 if there
is a 6-dof-IMU. But often there is a 9-dof-IMU used on the board (I have
six of them and it is the only type of AP I use). In this case the baro
cannot be used with the standard driver. Therefore Gautier wrote the
code for this branch as a patch and I just included his code in this
branch to solve the problem in master and not by applying a patch each
time. Unfortunately I am not yet able to follow the details of the code
(just started to understand pprz below user level).

However using Gautiers patch makes the baro work on the
9-dof-IMU-Apogee, even though it seems it is still not used to enhance
altitude and climb rate estimation for fixedwing (only for rotorcraft?),
which is what I want in the future (in fact I need the baro measurements
for two reasons: meteorological calculation of potential temperature
etc. (logging on sd-card) and better heigth and climb rate control for
fixedwing flights).

This all relates to the problems also discussed here:
http://lists.paparazziuav.org/Configuration-for-apogee-quadrotor-tt16439.html#a16499

Best regards,
Andreas

Am 27.12.2015 um 21:06 schrieb Felix Ruess:

@philipan https://github.com/philipan I don't quite understand the
need to add anything special for the MPL3115 baro compared to the
existing apogee driver... Could you please elaborate?


Reply to this email directly or view it on GitHub
#1498 (comment).


PD Dr. Andreas Philipp
Institute of Geography
University of Augsburg
Universitaetsstrasse 10
D - 86135 Augsburg
Germany

Phone: ++49/821/598-2266
Fax: ++49/821/598-2264
Email: a.philipp@geo.uni-augsburg.de

http://www.geo.uni-augsburg.de/lehrstuehle/phygeo/personal/philipp/

@flixr
Copy link
Member

flixr commented Dec 29, 2015

Well, as @gautierhattenberger wrote, his patch needs a bit more work, otherwise he could have committed it right away...

Still don't understand why the baro doesn't work in I2C pass-through mode like in the MPU60x0 case...
@gautierhattenberger maybe you can shed some light on this?
With #1499 we should have nicer support for multiple slaves without these nasty ifdefs and while loop.

In any case baro stuff should not be in the mpu9150_i2c.c file as it is not part of the MPU peripheral and rather in imu_apogee or probably even better in a new imu_apogee_mpu9150...

@gautierhattenberger
Copy link
Member

I did wrote this myself, and my colleague was writing his first Paparazzi driver. I was not completely happy so I didn't merged it as his.
The reason for the baro reading not in pass-through mode was that he wanted to have all data at once (IMU + MAG which is merely an independent device + Baro). It makes things more complicated, so I think making a simpler driver with both extra sensors in pass-through might be a good idea.
I can have a look if found some time...

@gautierhattenberger
Copy link
Member

on the other hand, the parts of this PR referring to meteo sensors logging could be merge separately (actually, it would have been easier to make a dedicated pull request for that)

@philipan
Copy link
Contributor Author

Of course I would really be extremely happy if I (and my students) could use the Apogee boards with the up-to-date pprz software without each time patching or rebasing/merging with git. However I understand that the master branch should keep a clear structure, which might be not the case for the PR as it is now.

In order to include at least the logging of the sht75, temod and airspeed I can revert the recent commit and leave out the baro things for a new commit if you think this is useful. Concerning the airspeed_ets I have the problem that "sdlog.h" cannot be found for the "#include" statement in "airspeed_ets.c" if target "sim" is compiled. I tried to figure out where the problem is, but didn't find a solution. For the other modules (temod and sht) it is no problem.

Further I found a problem in "conf/modules/temp_temod.xml" where the variable name "TEMOD_DEV" should be changed to "TEMOD_I2C_DEV" as far as I can see. Should this be an extra PR too?

@flixr
Copy link
Member

flixr commented Dec 30, 2015

Thanks for the TEMOD_I2C_DEV fix, committed with 5cad5b5

… sd (made available by Gautier)."""

This reverts commit 8875e1e.

Conflicts:
	conf/airframes/AULA/AULA_XENO_I_01.xml
	conf/airframes/AULA/apogee_sdlog.xml
	conf/radios/AULA/mx-16-aula.xml
	sw/airborne/modules/meteo/temp_temod.c
	sw/airborne/modules/sensors/airspeed_ets.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants