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

Updates to aerofc #6862

Closed
wants to merge 15 commits into from

Conversation

Projects
None yet
6 participants
@lucasdemarchi
Copy link
Contributor

lucasdemarchi commented Mar 20, 2017

Several changes that are still being validated, but we've got better perfomance with this, particularly AltHold, less vibration, better GCS connection, etc. The changes to baro will affect all boards (hopefully positively).

The UART link to Aero got the baudrate changed to 460800, providing much better connection to GCS.

Now we are using MPU9250 driver that's really an MPU6500 with additional mag in the same package. In future, migrating those 2 to MPU6000 would allow to have a single driver for them.

The last commit is a workaround to a bug introduced in master branch that doesn't allow updating the parameters.

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 20, 2017

Also worth noting that after this PR the baudrate on mavlink-router needs to be changed manually on the Linux side to match the baudrate here. This will already be set up on next version of the BSP.

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

Thanks! I think I can give this a try today

* Internal ADC is 14 bits, so also reduce z to +- 2457.3 µT.
* Check if value makes sense, and discard any outlier
*/
if (report.x > 5334 || report.x < -5334 ||

This comment has been minimized.

Copy link
@magicrub

magicrub Mar 21, 2017

Does z need to be inverted?

This comment has been minimized.

Copy link
@lucasdemarchi

lucasdemarchi Mar 21, 2017

Author Contributor

No... X and Y are swapped

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

@lucasdemarchi I was just out testing this PR on a stock Aero. The QGC connection is now way better and more stable. I was not able to log anything with QGC though. So I currently can't say anything about vibrations. However, flight performance was still the same. The Aero couldn't hold its altitude and crashed into the ground twice with a starting altitude of ~2m.

* Check if value makes sense, and discard any outlier
*/
if (report.x > 5334 || report.x < -5334 ||
report.y > 5334 || report.y < -5334 ||

This comment has been minimized.

Copy link
@dagar

dagar Mar 21, 2017

Member

Where does 5334 come from? Use a define or const instead of the literal value.

This comment has been minimized.

Copy link
@lucasdemarchi

lucasdemarchi Mar 21, 2017

Author Contributor

Isn't the comment just above sufficient?

This comment has been minimized.

Copy link
@dagar

dagar Mar 21, 2017

Member

I'm not following how you got to 5334. Either way it's still best to avoid magic numbers, especially if you're using it multiple times.

This comment has been minimized.

Copy link
@lucasdemarchi

lucasdemarchi Mar 21, 2017

Author Contributor

It's basically checking for values within the FSR as per comment above, but using sensor units instead of converting to µT:

FSR for x, y is +- 1600 µT
Resolution is 0.3 µT/LSB
FSR in LSB units is 1600/0.3 == 5333.3.

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 21, 2017

@ChristophTobler you need to recalibrate at least the accel and compass. I would just reset the params and calibrate again.

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

@lucasdemarchi I have recalibrated all the sensors

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 21, 2017

@lucasdemarchi I was just out testing this PR on a stock Aero. The QGC connection is now way better and more stable. I was not able to log anything with QGC though.

You may want to update mavlink-router (https://github.com/01org/mavlink-router). The master version has a configuration file and a command line switch that allows you to give a directory in which logs are saved.

So I currently can't say anything about vibrations. However, flight performance was still the same. The Aero couldn't hold its altitude and crashed into the ground twice with a starting altitude of ~2m.

If you calibrated everything, not sure what was the problem. Can you send a copy of the params?

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

@lucasdemarchi Not sure what you mean I should update.? Here are the parameters

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 21, 2017

# Git Revision: ca9393eb97aa4e77

not on the right revision?

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

@lucasdemarchi Hm, not sure where this is from. But if I do git log the latest commit is

commit 492283564be7426df772c07ef7a0709735b4ed2f
Author: Lucas De Marchi <lucas.demarchi@intel.com>
Date:   Mon Mar 20 09:34:56 2017 -0700

    aerofc: disable flash based dataman
    
    This is causing problems when saving parameters. Disable it for now.

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 21, 2017

@lucasdemarchi I have just flashed your PR again (I checked git log), flashed it with make aerofc-v1_default upload, rebooted, killed mavlink, restarted it with /usr/bin/mavlink-routerd -b 460800 -e 192.168.7.255 -e 192.168.1.255 /dev/ttyS1, refreshed the params in QGC and saved them to a file. The Git Revision still shows an older commit... I have no idea why

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 21, 2017

@ChristophTobler maybe a problem in the build system maybe? did you try a clean build (removing the build directory). After that, a "param reset" command in the nsh shell.

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 21, 2017

@lucasdemarchi Not sure what you mean I should update.? Here are the parameters

What I was saying is that you can replace the mavlink-router on Aero with master from upstream, which has support for saving the .ulg files.

@lucasdemarchi lucasdemarchi force-pushed the lucasdemarchi:pr-aerofc branch from 4922835 Mar 23, 2017

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 23, 2017

@dagar I added a variable to hold the calculation of the value limits. The last commit will be obsolete soon since @zehortigoza already has a fix for it, so I would hold on a little bit more on applying.

@zehortigoza

This comment has been minimized.

Copy link
Member

zehortigoza commented Mar 23, 2017

@lucasdemarchi The fix is in here: https://github.com/PX4/NuttX/pull/89
I talked with @davids5 and probably it will be merged tomorrow if his PR got also merged.
You can choose if you merge this with or without the last patch.

@dagar

This comment has been minimized.

Copy link
Member

dagar commented Mar 23, 2017

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 24, 2017

@lucasdemarchi

What I was saying is that you can replace the mavlink-router on Aero with master from upstream, which has support for saving the .ulg files.

Is there any documentation on how to do that? I cannot use wifi station mode on my Aero since once I run /etc/init.d/autostart-supplicant.sh start I just can connect to it with USB but that connection breaks after a few seconds and I cannot configure the wifi settings (however, I was able to switch it back to AP mode).
I tried copying the folder mavlink-router with scp but when I run make on the Aero it lacks future. pip install future doesn't work without internet access (obviously). Is there an other way?

@ChristophTobler

This comment has been minimized.

Copy link
Member

ChristophTobler commented Mar 24, 2017

I was able to copy the mavlink headers from my desktop (copy the includes folder) and then it worked.

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 24, 2017

I usually build on my machine and just copy the binary

@lucasdemarchi

This comment has been minimized.

Copy link
Contributor Author

lucasdemarchi commented Mar 24, 2017

@ChristophTobler you can also generate a new complete image. mavlink-router is being updated on this PR: intel-aero/meta-intel-aero#28

* rate of 100Hz which is be very safe not to read the ADC before the
* conversion finished
*/
#define MS5611_CONVERSION_INTERVAL 10000 /* microseconds */

This comment has been minimized.

Copy link
@priseborough

priseborough Mar 24, 2017

Contributor

This should reduce measurement noise provided the downstream processing is using averaging to down-sample to the rate that the sensors_baro topic is published at.

It will however increase self heating within the Baro and therefore increase the rate of baro drift.

This comment has been minimized.

Copy link
@priseborough

priseborough Mar 24, 2017

Contributor

I may be off the mark here - does this change increase the internal read rate of the sensor, or just the output rate?

This comment has been minimized.

Copy link
@priseborough

priseborough Mar 25, 2017

Contributor

If it doesn't affect the internal read rate then the self heating issue does not apply and this is something that should be considered for all boards.

This comment has been minimized.

Copy link
@lucasdemarchi

lucasdemarchi Mar 25, 2017

Author Contributor

It doesn't change the internal sensor configuration. We will just ask the sensor more frequently to get us a sample.

In the past we changed the OSR from 4096 to 1024 due to the self heating issue. However we kept the sampling rate which means we get more noisy data. With OSR at 1024 each internal sensor conversion takes around 3msec. Getting a sample every 10msec appeared pretty safe and improved AltHold on my tests with MS5607

This comment has been minimized.

Copy link
@priseborough

priseborough Mar 26, 2017

Contributor

What is the downside to reading the sensor output faster given that the occasional repeated value is not an issue because of the way we are averaging data? Could we go to 250Hz (4msec) ?

* rate of 100Hz which is be very safe not to read the ADC before the
* conversion finished
*/
#define MS5611_CONVERSION_INTERVAL 10000 /* microseconds */
#define MS5611_MEASUREMENT_RATIO 3 /* pressure measurements per temperature measurement */

This comment has been minimized.

Copy link
@priseborough

priseborough Mar 24, 2017

Contributor

Can we increase this given we don'r require temperature at the higher rate?

@lucasdemarchi lucasdemarchi force-pushed the lucasdemarchi:pr-aerofc branch 2 times, most recently Mar 31, 2017

lucasdemarchi added some commits Feb 13, 2017

ROMFS: set baudrate to 460800 for aerofc
We are getting way to much transfer errors with high baudrate
mpu6500: switch driver to 16G range
All the other drivers use 16G, do the same for MPU6500.
aerofc: fix documentation about pins
  - There's no I2C2, so remove it
  - Fix alignments everywhere in board_config.h
  - Clarify STM32 ADC (not available) and the ADC on the FGPA
  - Fix format on some comments

lucasdemarchi added some commits Mar 13, 2017

ms5611: reduce macro scope
Move the addresses to .cpp where they are used. This should help
noticing discrepancies like the OSR we are using and the sampling
interval.
aerofc: remove USB initialization
There's no USB, don't initialize it.
mpu9250: add support to MPU6500
MPU9250 is mostly an MPU6500 with a mag (AK8963) in the same package.
Support driving MPU6500 with the MPU9250 driver. The id of the driver
isn't set differently since this way it allows to force a recalibration.

Ideally MPU9250 driver could even not exist and the support for these
sensors be merged back in the MPU6000 that's more complete. This is an
intermediate step in that direction.
ist8310: remove commented out and dead code
This code just makes it harder to grok the driver. When support for this
feature is available we can bring it back.
ist8310: be resilient to bad data coming from sensor
It doesn't hurt to test if the sensor is outputing data in the expected
range and it should help with elimination of spikes in case of errors.
ist8310: remove undef
Use PX4_ERROR instead.

@lucasdemarchi lucasdemarchi force-pushed the lucasdemarchi:pr-aerofc branch to 90c8a07 Apr 1, 2017

@lucasdemarchi lucasdemarchi referenced this pull request Apr 4, 2017

Merged

Updates to aerofc #6972

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.