Sometimes (like in DataFlash) the size of the ring buffer will be
determined in run time and the object can have size zero until proper
initialization. When this was the case, an underflow in ::get_size would
mess with the initializing algorithm.
Another issue was that the 'new' operator could fail what was not being
handled. Now, we only set the size member after we are sure 'new'
successfully allocated memory.
Replace the previous not-implemented interface with a set of new methods
that can be resonably implemented:
- register_periodic_callback() now receives a functor returning bool
to easily allow "oneshot" timers
- adjust_periodic_callback() allows the caller to change the timer
for a specific handle. This way drivers like MS5611 can adjust the
timer depending on its state machine: the time to sample
temperature is smaller than the time to get a pressure sample
- add unregister_callback(): since we have an opaque pointer, we
can't tell the user to just delete it in order to unregister the
callback
This method is needed when we want to transfer both tx and rx at the
same time, as opposed to common cases for sensors in which they are like
in the I2C interface: half-duplex.
Minor changes to follow coding style and improve readability:
- sort headers
- move struct definition to compilation unit rather than header
- Add braces to if, for, etc
Volatile will provide protection to sequence re-ordering and guarantee
the variable is fetched from memory, but it won't provide the memory
barrier needed to ensure that no re-ordering (by either the compiler or
the CPU) will happen among other threads of execution
accessing the same variables.
For more info about this effect can be found on articles about
std::memory_order.
When using reserved(), the reserved memory cannot be read before it's
written, therefore we cannot update 'tail' until the caller of
reserved() is done writing.
To solve that, a method called 'commit()' was added so the caller can
inform that is done with the memory usage and is safe to update 'tail'.
The caller also has to inform the length that was actually written.
This solution was developed to work considering the usage context of
this class: 1 reader and 1 writer **only**.
Adds a method called `reserve()`, that will take a ByteBuffer::IoVec
array of at least two elements, and return the number of elements
filled out. 0 will be returned if `len` is over the total space of
the buffer; 1 will be returned if there's enough contiguous bytes in
the buffer; 2 will be returned if there are two non-contiguous blocks
of memory.
This method is suitable to be used with POSIX system calls such as
readv(), and is an optimization to not require temporary memory copies
while reading from a file descriptor.
Also modify the write() method to use reserve(), so that similar checks
are performed only in one place.
Modify ByteBuffer class to have a `peekiovec()` method, that takes in a
`struct IoVec` array (similar to `struct iovec` from POSIX), and a
number of bytes, and returns the number of elements from this array
that have been filled out. It is either 0 (buffer is empty), 1
(there's enough contiguous bytes to read that amount) or 2 (ring buffer
is wrapping around).
This enables using scatter-gather I/O (i.e. writev()), removing calls
to memcpy(). That's one call when no wrap-around is happening, and
two calls if it is.
Also, rewrite `ByteBuffer::peekbytes()` to use `peekiovec()`, so that
some of the checks performed by the former are not replicated in the
latter.
Many of our SPI and I2C sensors define the protocol of setting the most
significant bit of the register address in order to perform a read operation.
Thus, enable the use of a "read flag" that is ORed with the register's address.
Since this is an abstraction for general devices, it's a good idea to have zero
as the default value for that flag.
While at it, add documentation to read_registers().
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.
../../libraries/AP_HAL/examples/Printf/Printf.cpp:63:17: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if (ret != strlen(float_tests[i].result)) {
^
...
ardupilot/modules/gtest/include/gtest/gtest.h:1448:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
if (expected == actual) {
^
And similar ones.
../../libraries/AP_HAL/examples/UART_test/UART_test.cpp:13:28: warning: ‘uarts’ defined but not used [-Wunused-variable]
static AP_HAL::UARTDriver* uarts[] = {
^
This is very similar to std::unique_ptr, but doesn't require including
the <memory> header which pulls lots of c++ headers and cause problems
with nuttx headers. It's header-only. It contains an explanation on what
it solves, how to use and unit tests.
This commit adds support for OpticalFlow to MinnowBoardMax trying to
leave the OpticalFlow implementation as generic as it already is.
We had to add some format conversion and software crop to the cameras that
do not have this features.
Fix warning that reveals a real bug:
In file included from libraries/AP_HAL_Linux/UARTDriver.cpp:25:0:
libraries/AP_HAL_Linux/UARTDriver.cpp: In member function 'virtual bool Linux::UARTDriver::tx_pending()':
libraries/AP_HAL/utility/RingBuffer.h:21:35: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
#define BUF_EMPTY(buf) buf##_head == buf##_tail
^
libraries/AP_HAL_Linux/UARTDriver.cpp:355:13: note: in expansion of macro 'BUF_EMPTY'
return !BUF_EMPTY(_writebuf);
The problem is when there's a ! operator: without the parenthesis we would actually be doing
return !_write_buf_head == _write_buf_tail
which is not what we want.
This adds a macro for boards that need to have UARTs on the timer
thread instead of their own thread. This is needed for the qflight
port due to restrictions in the RPC API
This was previously used to allow to save a state in a SPIDriver so we
could synchronize the initialization of AP_Compass and
AP_InertialSensor.
It was only used by MPU9250 and is not used anymore since the move to
AuxiliaryBus initialization and it's not used anymore since c3dae6f
("AP_InertialSensor: MPU9250: Remove methods not used anymore")
The argument in init() is not used by any implementation. Add a second
method without it so the HAL implementation can used it instead. Later
the unused method will be removed.
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.
Only compiled on Bebop, the constructor will need to be modified to
pass the pwm chip number and to create a PWM_Sysfs instead of a PWM_Sysfs_Bebop
in case it is used on a mainline linux board
Currently, the default behaviour on linux boards tries to
write LED gpios with fixed values among them. There is no way
to declare that there are no LED GPIOs.
This commit moves the declaration of the LED Gpios in AP_HAL_Boards.h
and makes AP_Notify do nothing if no LED gpio was declared
For certain basic functionality, there aren't much benefit to be able to
vary the implementation easily at runtime. So instead of using virtual
functions, use regular functions that are "resolved" at link time. The
implementation of such functions is provided per board/platform.
Examples of functions that fit this include: getting the current
time (since boot), panic'ing, getting system information, rebooting.
These functions are less likely to benefit from the indirection provided
by virtual interfaces. For more complex hardware access APIs the
indirection makes more sense and ease the testing (when we have it!).
The idea is that instead of calling
hal.scheduler->panic("on the streets of london");
now use
AP_HAL::panic("on the streets of london");
A less important side-effect is that call-site code gets
smaller. Currently the compiler needs to get the hal, get the scheduler
pointer, get the right function pointer in the vtable for that
scheduler. And the call must include an extra parameter ("this"). Now it
will be just a function call, with the address resolved at link time.
This patch introduces the first functions that will be in the namespace,
further patches will implementations for each board and then switch the
call-sites. The extra init() function allow any initial setup needed for
the functions to work.
Remove the checks for HAL_CPU_CLASS > HAL_CPU_CLASS_16 and
HAL_CPU_CLASS >= HAL_CPU_CLASS_75. Corresponding dead code will be
removed on separate commits.
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.
Follow the following order for includes:
- Corresponding header file (if exists)
- System headers
- Other ArduPilot library headers
- "Local" headers (from the same library)
This was introduced with the HAL rework:
In file included from /p/ardupilot/libraries/AP_HAL/AP_HAL.h:11:0,
from /p/ardupilot/ArduCopter/Copter.h:35,
from /p/ardupilot/ArduCopter/ArduCopter.cpp:76:
/p/ardupilot/ArduCopter/ArduCopter.cpp: In function 'int ArduPilot_main(int, char* const*)':
/p/ardupilot/libraries/AP_HAL/AP_HAL_Main.h:11:26: warning: no previous declaration for 'int ArduPilot_main(int, char* const*)' [-Wmissing-declarations]
#define AP_MAIN __EXPORT ArduPilot_main
^
It's due to PX4 using that warning as opposed to Linux. Since it harmless, add
the prototype for everybody.
Since we are using uint8_t and uint16_t types we need to include the
correspondent system header. Otherwise it would depend on the include
order of who is including this particular header, causing failures as we
move headers around.
This is going to be used by vehicles that already have an object with
setup/loop functions. The vehicle object will just implement the
HAL::Callbacks interface.
Move the macros to a single place and reduce the variations not based on
board, but based on
- The name of the entry-point function, specified by AP_MAIN;
- Whether it contains argc/argv arguments or not.
The goal here is that programs (vehicles and examples) don't need to
include all possible boards to define a main function. Further patches
will change the programs.
Add run method, that encapsulate any mainloop logic on behalf of the
client code. The setup/loop functions are passed via a HAL::Callbacks
interface. The AP_HAL_MAIN() macro should be kept as trivial as
possible.
This interface should be implemented by the existing vehicle objects. To
make easy for the examples (that don't have the equivalent of vehicle
objects), a FunCallbacks was added to bridge to the functions directly.
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'.
This method is not used anymore since the introduction of channel map and
allowing motors to be enabled/disabled in AP_Motors.
Later we may introduce a method to write multiple values with a default
implementation that supports the channel and enable maps rather than
requiring all subclasses to implement this method.
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.
Add a deinit() counterpart. This is needed for some ports that require some deinitializtion logic. The default implementation is empty. I'm not sure whether we need to inforce it for all.
This adds the backend driver for LSM9DS0. This implementation is based on the
legacy driver coded by Víctor Mayoral Vilches (under folder LSM9DS0) and makes
some necessary adaptations and fixes in order to work properly. The legacy
driver folder was removed.
It both reduces flash size and move symbols to read-only sections.
The scheduler_tasks table is one known not to be in read-only section before due
to the FastDelegate implementation. Before and after this patch:
APMrover2 $ size APMrover2.elf{.old,}
text data bss dec hex filename
611406 4832 40920 657158 a0706 APMrover2.elf.old
609686 4824 38936 653446 9f886 APMrover2.elf
APMrover2 $ nm -C APMrover2.elf{.old,} |grep tasks
0000000000696f80 B Rover::scheduler_tasks
000000000047c440 R Rover::scheduler_tasks
As can be seen above, now the scheduler_tasks symbol is in a read-only data
section and in all of them we decreased the total size.
For APM2 we have a similar situation, but the table was already in text section
because it was using plain C pointers:
APMrover2 $ size APMrover2.elf{.old,}
text data bss dec hex filename
189518 1038 3494 194050 2f602 APMrover2.elf.old
189216 1038 3480 193734 2f4c6 APMrover2.elf
APMrover2 $ nm -C APMrover2.elf{.old,} |grep tasks
00001f92 T Rover::scheduler_tasks
00001f8a T Rover::scheduler_tasks
Now that we are using C++11 we can use variadic templates to simplify
the FastDelegate classes. It also simplifies moving away from the
FastDelegate implementation.
Start to add code behind APM_BUILD_FUNCTOR to support changing the
functor implementation (without breaking the build while the change is
not complete).
This is a Functor implementation that should cover the use cases we have
for FastDelegate. In contrary to the latter, it can be constructed at
compile time so the compiler can safely put it in a read-only section
which covers the cases in which we are not using it.
LukeMike
Murilo Belluzzo
Lucas De Marchi
hiro2233
Andrew Tridgell
Luiz Ywata
Leandro Pereira
Siddharth Bharat Purohit
Gustavo Jose de Sousa
Patrick J.P
Tom Pittenger
Randy Mackay
Niti Rohilla
Ricardo de Almeida Gonzaga
Staroselskii Georgii
Julien Beraud
Peter Barker
mirkix
Jonathan Challinger
Víctor Mayoral Vilches
Aaron Wang Shi
Caio Marcelo de Oliveira Filho
José Roberto de Souza
Víctor Mayoral Vilches
raspilot
Vladislav Zakharov
Eugene Shamaev
Max Basescu