Having "BIT" in the name gives the impression we are using the macro is
the bit position; however they have the values for those actions, not
the bit position. Rename BIT to VAL to be less confusing.
This is the equivalent of sleep and wait for the conversion time,
after had triggered a new sample request. However it also has the added
benefits of sharing a thread with other sensors on the same bus.
Now we don't read the status register anymore since we have a guaranteed
wait time.
- Make sure device is reset while initializing.
- Give better names to register macros
- Average X, Y and Z by 16: sensor is internally running at 200 sps
(theoretical, ~160 pratical). The wait time is ~6msec averaging
by 16. We do 10msec.
- Inline code that needs to take and release the lock in init()
function, just like other drivers
- Minor changes to coding style and renames to be similar to other
drivers
This compass has been proved to work better on Navio boards. Users also
don't like high offsets (even though, they don't really mean much in
this context) reported by AK8963 in MPU9250 on Navio 2.
this makes the HMC5843 use single-shot mode, which makes it more
robust to bus resets. It also adds WHOAMI checking and enables
automatic temperature compensation.
Unfortunately this makes existing calibration values incompatible, so
a re-cal is required. The device ID has been changed to reflect the
driver change.
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.
Summary of significant changes:
-Autsave doesn't depend on STREAM_EXTRA3
-Don't risk only saving one compass on copter if CAL_ALWAYS_REBOOT is set
-Only calibrate compasses that are both health and marked for use (there was a inconsistency in handling the mask)
-Fix incorrect failure reporting on DO_ACCEPT_MAG_CAL with a mask of 0 if a channel was specifically not started
-Fix not starting the buzzer if the delay is set to 0 seconds
-Always send MAG_CAL_REPORT until its acknowledged
-Correct the field in MAG_CAL_REPORT for autosave to indicate if the compass had actually been saved, rather then being scheduled to be saved
-Remmove unused public interfaces
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.
The minimum version for gcc was supposed to be 4.9 for any platform.
However our build instructions are outdated. Remove the problematic
parts that use the sparse-endian.h header while we don't fix the setup
for windows.
- Correctly sort includes and add missing AP_Math.h
- Use anonymous struct for trim_registers in _load_trim_values,
renaming its members so they don't start with underscore
- Don't change _dig* values when we failed to read from sensor
- Add some blank lines
- Make _dig_* members be inside a _dig struct
- Use constrain_int32 instead of if/else chain
- s/time_us/time_usec/
- Construct raw_field with a single constructor in _update()
- Add missing copyright notice
- Group methods together in declaration
In addition:
- bbbmini, navio and navio2 can force HMC5843 backend to
be external.
- there was a typo in the backend name that's now fixed:
AK8953 vs AK8963
Minlure has an onboard compass (HMC5883L) as slave of MPU-6000, but also
allows the use of an external HMC5883L compass, which should be
connected to the lure's I2C port.
That rotation is done because of the "arrow" pointing the sensor direction in
3DR GPS+Compass kit (which isn't natural to the sensor orientation, by the
way). Thus, do that rotation only for external HMC5883L compasses.
That rotation is set for AP_COMPASS_TYPE_HMC5883L, because it's usually
the compass from 3DR GPS+Compass kit, which has an arrow pointing the
"correct" direction. That rotation should be done before rotate_field()
as it represents the "standard" position for the kit.
This patch also makes published values consistent with respect to rotation.
Before this, raw fields used in calibration were published with a
rotation different from the filtered field when _product_id is
AP_COMPASS_TYPE_HMC5883L.
And to the detection probe() method. That way we don't need to use a board
`#ifdef` inside the class code. Additionally, we make raspilot board use it.
Initialization was also changed a little bit so we don't try to
initialize 25 times. We rather use the same methods as in the
AP_InertialSensor drivers.
Also move up the call to is_zero() in read_raw so we don't set
_mag_[x|y|z] in case of failure.
These are not used. The only place where raw fields are used are in the
compass calibrator and we don't need to store them.
Additionally remove duplicated documentation about the meaning of the
functions to avoid them getting out of sync.
We still need the BusDriver (in some places called SerialBus) interface
since this driver can also be used on an AuxiliaryBus and that has a
different interface.
- Capitalize and rename constants
- Make clear what gain is applied in calibration and what is the
"normal" gain
- Make the separation between HMC5883L and HMC5843 explicit when it
makes sense to improve readability
- Remove spurious delay in calibrate function
We still need the BusDriver (in some places called SerialBus) interface
since this driver can also be used on an AuxiliaryBus and that has a
different interface.
Besides the usual conversion some more improvements:
- Use generic function to convert endianness
- Minor cleanups
- Reorder per-board ifdefs in compass instantiation: distinguish when
there's a default compass to when it should probe other compasses
In file included from /home/lucas/p/dronecode/ardupilot/libraries/AP_Compass/Compass.h:6:
/home/lucas/p/dronecode/ardupilot/libraries/AP_Compass/CompassCalibrator.h:86:5: warning: struct 'param_t' was previously declared as a class [-Wmismatched-tags]
struct param_t _params;
^
/home/lucas/p/dronecode/ardupilot/libraries/AP_Compass/CompassCalibrator.h:44:11: note: previous use is here
class param_t {
^
/home/lucas/p/dronecode/ardupilot/libraries/AP_Compass/CompassCalibrator.h:86:5: note: did you mean class here?
struct param_t _params;
^~~~~~
Not having data ready during initialization is normal. Don't print error
message since it can mislead people to think the compass was not
initialized successfully.
This is the only driver doing this, using the system_initializing() from
scheduler to log selectively. Remove the check together with removal of
unused wrapper methods to semaphore.
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.
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.
For all supported boards the maximum number of instances is 3. The
number of HIL_COMPASSES was already defined as 2 instead of 3, so this
is left as before.
We don't support HAL_CPU_CLASS <= HAL_CPU_CLASS_16 anymore. This makes
COMPASS_MAX_INSTANCES and COMPASS_MAX_BACKEND constant for all supported
boards.
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'.
offset parameter units are milligauss
User settable parameters should have a User category defined. Those that should never be set by a user should not have this.
The function rotate_field() can change the values axes and the function
correct_field() applies offsets (which are already in milligauss). Thus any
sensitivity adjustment must be done for two reasons:
(1) The offsets must be applied to the values already in milligauss;
(2) The factory sensitivity adjustment values are per axis, if any rotation
that switches axes is applied, that'll mess with the adjustment.
Experiments showed that before this patch the length of the mag field reported
quite different from the expected. After this patch, the same experiments
showed reasonable values.
This is part of the transition to make all mag field values be used in
milligauss. Additionally the value of _gain_multiple is adapted to the new way
we're using it and corrected accordingly to the datasheets.
The use of _gain_multiple is not necessary because the values of
expected_{x,yz} and _mag_{x,y,z} are both in sensor raw unit (i.e., lsbs).
That wasn't fixed before in order not to make APM users to recalibrate their
compasses.
the previous approach assumed a 1:1 mapping between compass backends
and compass instances, which isn't true on PX4.
It also only setup milligauss offsets on a set_and_save call, which is
not the only way offsets change
this adds a milligauss_ratio per instance, which is considerably
simpler
AK8963 is configured in 16-bit ADC mode which implies sensitivity of 0.15 uT/LSb. Knowing this fact we can convert the measurements to the proper units. The change will make users recalibrate their compasses.
From now on there's a pair get_field_milligauss() and
get_offsets_milligauss() that can make the transition to the common
units across all compasses easier.
This commit changes the way libraries headers are included in source files:
- If the header is in the same directory the source belongs to, so the
notation '#include ""' is used with the path relative to the directory
containing the source.
- If the header is outside the directory containing the source, then we use
the notation '#include <>' with the path relative to libraries folder.
Some of the advantages of such approach:
- Only one search path for libraries headers.
- OSs like Windows may have a better lookup time.
increase iterations to get good results from LM
better check for convergence, comparison with initial fitness is a better way to determine if convergence occurs, if fitness has not improved compared to initial fitness it means optimiser has failed.
Like was done to inertial AK8963 and inertial sensor move the decision
regarding the I2C bus to the caller. We don't allow changing the address
because apparently HMC5843 doesn't support different addresses.
Changing only the bus could be more easily done but this prepares the
ground for using HMC5843 on an AuxiliarBus.
The need for a more generic abstraction is evidenced by this commit: a
"SerialBus" in AP_HAL would be a nice addition rather than letting each
driver to provide its own. However the methods are a little bit
different from what we have in AK8963. It's safer to do the simple
conversion now and later on to add the abstraction changing everybody to
use it.
read() calls accumulate() which takes the lock by itself so we must
release it like we were doing before 669ae26 ("AP_Compass: encapsulated
calibration in HMC").
Just like was done for inertial sensor, different detect() functions
might need different arguments and passing a pointer to function here is
cumbersome. For example, it forces to have a method like "detect_i2c2"
rather than allowing hal.i2c2 to be passed as parameter.
This commit changes the way libraries headers are included in source files:
- If the header is in the same directory the source belongs to, so the
notation '#include ""' is used with the path relative to the directory
containing the source.
- If the header is outside the directory containing the source, then we use
the notation '#include <>' with the path relative to libraries folder.
Some of the advantages of such approach:
- Only one search path for libraries headers.
- OSs like Windows may have a better lookup time.
In case of error or zeroed data, the i2c semaphore wasn't given.
It happened at first startup on Bebop and caused a failure:
"PANIC: failed to take _bus->sem 100 times in a row..."
No need to repeat the same comments on 3 different places. Instead add a
single comment to explain how start_measurements() and read_raw() are
related.
Don't use a state machine in AK8963: the start_measurements() method
should be called only once. Even if there's a magnetic sensor overflow
the only thing we should do is to discard the new data.
This also moves the _collect_samples() method to be inside _update()
since it's the only place it should be called from, the one running on
the timer thread.
We need to check the AK8963's id before anything else Here we are
reordering the calls to _calibrate() and _check_id(). After that we
don't need to read and write again the AK8963_CNTL1 register.
While at it do some renames as well:
- _configure() -> _setup_mode(): since now there's a
_bus->configure() it became confusing what actually it's
doing.
- make error messages say what we were actually trying to do but
couldn't. Also remove PSTR since this is linux-only.
- start_conversion() -> start_measurements(): We are instructing the
bus to start to get the samples, not to tell the chip to start an
analog->digital conversion like in other sensors.
The MPUREG_PWR_MGMT_1 defines were used when we cared about the MPU9250
initialization code. Now all initialization is done by the MPU9250 class
itself, so remove these defines.
Also remove AK8983_SELFTEST_MAGNETIC_FIELD_ON that is never used and let
the defines always with 2 bytes to improve readability.