See discussion here:
https://github.com/ArduPilot/ardupilot/issues/7331
we were getting some uninitialised variables. While it only showed up in
AP_SbusOut, it means we can't be sure it won't happen on other objects,
so safest to remove the approach
Thanks to assistance from Lucas, Peter and Francisco
On an invensense IMU with fast sampling we need to lower the slave
sample rate for slave sensors such as the built-in AK8963 compass on a
MPU9250.
The slave rate is set as a multiple of the main rate, so it needs to be
much lower for fast sampling. If we leave it high then it greatly
impacts on IMU sample rate.
Without this change a MPU9250 with fast sampling and a compass enabled
will give a gyro rate of around 7200Hz. With this change it gets 7760Hz
FIFO sensors produce data at a well known rate, but samples come in
bunches, so we can't use the system clock to calculate deltaT.
non-FIFO sensors produce data when we sample them, but that rate is
less regular due to timing jitter.
For FIFO sensors this changes makes us use a learned sample rate,
which allows for different clock speeds on sensor and system board.
For non-FIFO sensors we use the system clock to measure deltaT
the overall effect is a fix for sensors that produce samples at other
than the claimed datasheet rate.
this moves to using a 1p filter on the high rate data, followed by
averaging down to 1kHz then a 2p filter to apply configured cutoff
frequency.
It also fixes the FIFO reset to not cause data corruption. We need to
disable all FIFO channels before doing the reset, and wait for the
FIFO to stop in the sensor.
Finally it moves sampling of the MPU6000 and MPU9250 into the main
thread. That significantly improves scheduling performance as we no
longer get long FIFO SPI transfers happening during other tasks. All
transfers happen at the start of the fast loop. That makes timing much
more predictable.
Thanks to Leonard and Paul for help with this design!
When blocking forever there's no reason to call panic later since it
will never going to be reached. This reduces binary size in a few bytes
since the message isn't required anymore.
When the accel calibration fails leave the previous values saved but set them to defaults (scale default is ones, not zeros) and notify the GCS
This fixes an arithmetic exception when doing a second accel cal after the first one failed
this allows enable/disable of fast sampling per IMU, making
experimentation easier.
It also fixes the fast sampling to always average over 8 samples, and
fixes the 9250 to use the correct accumulator when not doing fast
sampling
Two cases of what seems to be FIFO alignment errors have been seen on
a Pixracer-beta board with a ICM-20608. At a cost of 2 extra bytes per
transfer we can catch these by looking for sudden temperature changes
caused by bad data in the temperature registers.
When registering accel and gyro we use the upper 16bits of the id to
store the driver version. When changing the driver behavior in
non-compatible ways, changing this version will trigger a request for
calibration.
This allows each sensor to be uniquely identified in the system by using
either the index inside the backend or for those that use the Device
interface, to use the bus type, location, and device id.
We leave 16-bit for each sensor to be able to change its own
identification in future, which allows them to be changed in an
incompatible manner forcing a re-calibration.
When we are initializing the gyro and then saving the calibration we are
also saving the calibration values for the accelerometers. Right now
this is non-problematic, but we want to check that the ID of the
accelerometer corresponds to the ID of the sensor detected. If we also
save accel calibrations we would actually override the ID of the
accelerometer.
Rename the method to _save_gyro_calibration() and save only on gyro
values.
We only leave the parameter there for backward-compatibility. However
product id on the inertial sensor is not much useful since it's only
kept for the first instance.
A better implementation per-gyro and per-accel is needed in order to
avoid problems with sensors taking the offsets configured for another
sensor.
By opening with O_CLOEXEC we make sure we don't leak the file descriptor
when we are exec'ing or calling out subprograms. Right now we currently
don't do it so there's no harm, but it's good practice in Linux to have
it.
RC_Channel: To nullptr from NULL.
AC_Fence: To nullptr from NULL.
AC_Avoidance: To nullptr from NULL.
AC_PrecLand: To nullptr from NULL.
DataFlash: To nullptr from NULL.
SITL: To nullptr from NULL.
GCS_MAVLink: To nullptr from NULL.
DataFlash: To nullptr from NULL.
AP_Compass: To nullptr from NULL.
Global: To nullptr from NULL.
Global: To nullptr from NULL.
Notify users of the potential for velocity noise when using larger offset values..
Specific advice in terms of values has not been provided because it is highly dependent on Gyro noise levels.
We currently check examples are buildable with waf which doesn't need
the libraries to be specified in a make.inc file. Having the makefiles
there is misleading since people try to build and realize the build is
broken.
Some notes:
- The only place that made sense to use
suspend_timer_procs()/resume_timer_procs() calls were where we registered
the timer process. Now there's no need for that anymore. Remove those calls
from other place in the source too.
- There's no need to acquire the device lock now that we are running as a
periodic callback.
- Use "INS_" prefix for the name in order to limit the scope for that macro.
- Don't define it in the code and check if it is defined instead of checking
the value. With that, there's no need to touch the code for enabling debug,
only a reconfiguration is necessary (e.g., `CXXFLAGS='-DINS_TIMING_DEBUG' waf
configure ...`).
The reason of defining BMI160_MAX_FIFO_SAMPLES as 8 can be found on the
following histogram of the number of samples in the FIFO on each read while
performing the accelerometer calibration process:
Samples Count Freq Acc. Freq
------------------------------
1 3842 0.1201 0.120111
2 13172 0.4118 0.531904
3 9065 0.2834 0.815300
4 2710 0.0847 0.900022
5 2231 0.0697 0.969769
6 816 0.0255 0.995279
7 137 0.0043 0.999562
8 13 0.0004 0.999969
13 1 0.0000 1.000000
The new function can deal with a variable number of function parameters.
Additionally, I renamed the functions to norm(), because this is the
standard name used in several other projects.
Several coding style problems were already fixed by previous commit,
just finish the cleanup on this example:
- replace tabs with spaces
- remove unneeded \r
- remove extra spaces
We should return the number of bytes written/read, not 0 on success.
This number may be useful in some cases so return it.
While at it fix a simple wrong space in the header.
Due to the way the headers are organized changing a single change in an
inertial sensor driver would trigger a rebuild for most of the files in
the project. Time could be saved by using ccache (since most of the
things didn't change) but we can do better, i.e. re-organize the headers
so we don't have to re-build everything.
With this patch only AP_InertialSensor/AP_InertialSensor.h is exposed to
most users. There are some corner cases to integrate with some example
code, but most of the places now depend only on this header and this
header doesn't depend on the specific backends.
Now changing a single header, e.g. AP_InertialSensor_L3G4200D.h triggers
a rebuild only of these files:
$ waf copter
'copter' finished successfully (0.000s)
Waf: Entering directory `/home/lucas/p/dronecode/ardupilot/build/minlure'
[ 80/370] Compiling libraries/AP_InertialSensor/AP_InertialSensor.cpp
[ 84/370] Compiling libraries/AP_InertialSensor/AP_InertialSensor_L3G4200D.cpp
[310/370] Linking build/minlure/ArduCopter/libArduCopter_libs.a
[370/370] Linking build/minlure/bin/arducopter
Waf: Leaving directory `/home/lucas/p/dronecode/ardupilot/build/minlure'
This is not used by any board and has a lot of commented out code. For
example, the compass is not enabled. The comment in the beginning of
the driver says it should serve as an example, but we should rather use
a working driver as an example. If this was at least a bit simpler and
that worked in the past we could refactor it to the new I2CDevice API.
This is not the case.
new functions that get a filtered min/max accel peaks on each axis with fixed 500ms timeout:
Vector3f get_accel_peak_hold_pos()
Vector3f get_accel_peak_hold_neg()
This allows slower mechanisms, such as is_flying, to detect accel spikes which would indicate ground or object impacts. Vibe is too filtered. Independent positive and negative peaks are available
The problem with using min() and max() is that they conflict with some
C++ headers. Name the macros in uppercase instead. We may go case by
case later converting them to be typesafe.
Changes generated with:
git ls-files '*.cpp' '*.h' -z | xargs -0 sed -i 's/\([^_[:alnum:]]\)max(/\1MAX(/g'
git ls-files '*.cpp' '*.h' -z | xargs -0 sed -i 's/\([^_[:alnum:]]\)min(/\1MIN(/g'
As commented in 8218140 ("AP_Common: add scanf format macro"), "FORMAT"
was a bad name for this macro since there's also the scanf. Rename to
FMT_PRINTF to follow the scanf name.
This method will be used to initialize and configure I2C backends that
have an auxiliary I2C bus that can be connected to the main I2C bus,
like MPU6000 and MPU9250.
Using MPU9250 over I2C we can connect the auxiliary bus where there is
a AK8963 and connect this bus to the main one, this way we don't need
any AuxiliaryBus infrastructure as we need with SPI and we can talk
with AK8963 as we would talk with a standalone AK8963.
Data-ready pin wasn't being used before due to a bug in the Kernel with
concurrent accesses to GPIO in Intel Baytrail platforms. That has been fixed in
Kernel version 4.2.
"%S" is used for wide string, but we are passing a char*. Use lowercase
in this case to remove warnings like this:
libraries/AP_InertialSensor/AP_InertialSensor.cpp: In member function
'bool AP_InertialSensor::calibrate_accel(AP_InertialSensor_UserInteract*, float&, float&)':
libraries/AP_InertialSensor/AP_InertialSensor.cpp:620:61: warning:
format '%S' expects argument of type 'wchar_t*', but argument 3 has type 'const char*' [-Wformat=]
"Place vehicle %S and press any key.\n", msg);
^
Most of AP_Progmem is already gone so we can stop including it in most
of the places. The only places that need it are the ones using
pgm_read_*() APIs.
In some cases the header needed to be added in the .cpp since it was
removed from the .h to reduce scope. In those cases the headers were
also reordered.
prog_char and prog_char_t are now the same as char on supported
platforms. So, just change all places that use them and prefer char
instead.
AVR-specific places were not changed.
Now variables don't have to be declared with PROGMEM anymore, so remove
them. This was automated with:
git grep -l -z PROGMEM | xargs -0 sed -i 's/ PROGMEM / /g'
git grep -l -z PROGMEM | xargs -0 sed -i 's/PROGMEM//g'
The 2 commands were done so we don't leave behind spurious spaces.
AVR-specific places were not changed.
The PSTR is already define as a NOP for all supported platforms. It's
only needed for AVR so here we remove all the uses throughout the
codebase.
This was automated with a simple python script so it also converts
places which spans to multiple lines, removing the matching parentheses.
AVR-specific places were not changed.
In order to avoid confusion between sample rate from sensor and sample rate
from the frontend class (AP_InertialSensor), use "raw sample rate" to refer to
the former.
The changes in the code were basically done with the following commands:
git grep -wl _accel_sample_rates | xargs sed -i "s,\<_accel_sample_rates\>,_accel_raw_sample_rates,g"
git grep -wl _set_accel_sample_rate | xargs sed -i "s,\<_set_accel_sample_rate\>,_set_accel_raw_sample_rate,g"
git grep -wl _accel_sample_rate | xargs sed -i "s,\<_accel_sample_rate\>,_accel_raw_sample_rate,g"
git grep -wl _gyro_sample_rates | xargs sed -i "s,\<_gyro_sample_rates\>,_gyro_raw_sample_rates,g"
git grep -wl _set_gyro_sample_rate | xargs sed -i "s,\<_set_gyro_sample_rate\>,_set_gyro_raw_sample_rate,g"
git grep -wl _gyro_sample_rate | xargs sed -i "s,\<_gyro_sample_rate\>,_gyro_raw_sample_rate,g"
And also with minor changes on indentation and comments.
Delta angle calculation is now unified, so there is no need for such a method.
That also avoids developers thinking they need that method being called
somewhere in their new drivers.
This commit basically moves delta angle calculation that was previously done in
AP_InertialSensor_PX4 to a common place. Instances must publish their gyro raw
sample rate to enable delta angle calculation.
The delta velocity calculation is now unified, so there is no need for such a
method. That also avoids delevopers thinking they need that method being called
somewhere in their new drivers.
This commit basically moves delta velocity calculation that was previously done
in AP_InertialSensor_PX4 to a common place. Instances must publish their accel
raw sample rate to enable delta velocity calculation.
We don't support HAL_CPU_CLASS <= HAL_CPU_CLASS_16 anymore. This makes
INS_MAX_INSTANCES, INS_MAX_BACKENDS and INS_VIBRATION_CHECK constant for
all supported boards.
The code with ifdef for !FAST_SAMPLING is bit rotting and the example
for AP_InertialSensor is currently broken for this case. Instead of
adding more ifdefs, remove the legacy implementation for !FAST_SAMPLING
since we don't support it anymore.
Reported by Grant:
/home/grant/3dr/ardupilot/libraries/AP_InertialSensor/AP_InertialSensor_MPU6000.cpp:
In member function 'void AP_InertialSensor_MPU6000::_accumulate(uint8_t*,
uint8_t)':
/home/grant/3dr/ardupilot/libraries/AP_InertialSensor/AP_InertialSensor_MPU6000.cpp:776:20:
error: no match for 'operator+=' (operand types are 'Vector3l {aka
Vector3<long int>}' and 'Vector3f {aka Vector3<float>}')
_accel_sum += accel;
/home/grant/3dr/ardupilot/libraries/AP_InertialSensor/AP_InertialSensor_MPU6000.cpp:777:19:
error: no match for 'operator+=' (operand types are 'Vector3l {aka
Vector3<long int>}' and 'Vector3f {aka Vector3<float>}')
_gyro_sum += gyro;
Instead of requiring every program to specify the HAL related modules,
let the build system do it (in practice everything we compiled depended
on HAL anyway). This allow including only the necessary files in the
compilation.
The switching between different AP_HAL was happening by giving different
definitions of AP_HAL_BOARD_DRIVER, and the programs would use it to
instantiate.
A program or library code would have to explicitly include (and depend)
on the concrete implementation of the HAL, even when using it only via
interface.
The proposed change move this dependency to be link time. There is a
AP_HAL::get_HAL() function that is used by the client code. Each
implementation of HAL provides its own definition of this function,
returning the appropriate concrete instance.
Since this replaces the job of AP_HAL_BOARD_DRIVER, the definition was
removed.
The static variables for PX4 and VRBRAIN were named differently to avoid
shadowing the extern symbol 'hal'.
Read temperature as part of the normal burst. This is not very costly since it
is part of the burst read in i2c and already read in spi.
It is meant to be used for imu heating.
The filter is set to 1Hz on temperature because of the inherent inertia of
heating systems.
Andrew Tridgell
bugobliterator
Tom Pittenger
Pierre Kancir
Randy Mackay
Peter Barker
HeBin
Alexey Bulatov
Georgii Staroselskii
Lucas De Marchi
Kevin Lopez Alvarez
Leonard Hall
murata
Julien Beraud
mirkix
priseborough
Pierre Kancir
hiro2233
Francisco Ferreira
Martin Evans
Mathieu OTHACEHE
Gustavo Jose de Sousa
Murilo Belluzzo
José Roberto de Souza
Grant Morphett
Ricardo de Almeida Gonzaga
dgrat
AndersonRayner
ashwinvasudevan
raspilot
Jonathan Challinger
Víctor Mayoral Vilches
Tom Pittenger
Don Gagne
Aaron Wang Shi
Caio Marcelo de Oliveira Filho
Víctor Mayoral Vilches