This centralized namespace header encourages centralizing things on
umbrella headers that are a pain to maintain. Force each part of
AP_HAL_Linux to include what is used.
While at it, do some whitespace cleanups and minor changes to adhere to
coding style.
This is selectable by a define and is never changed. Just remove
everything referencing it: we can come up with a better solution if it
is actually used later.
There's little point in having the Linux::AnalogIn just to implement and
empty interface. All implementations inside AP_HAL_Linux are already
inheriting directly from AP_HAL, so just remove it.
In a case ArduPilot is launched as a background process without
detaching with *nohup* like this ./arduplane -C /dev/ttyAMA0 ConsoleDevice
is created and an attempt to read from it is made. This yields in a stopped
process. This is an endeavour to overcome this problem.
Using factory method maked it easier to grasp the lifetime of all object
that get created and destroyed. Instead of spanning this thoughout whole
source file we have it nicely encapsulated in this a little horrendeous
_parseDevicePath that is of course to improve more.
Otherwise we're going to leak memory without any need.
Before this fix we've created ConsoleDevice 4 times in case -A switch hadn't been supplied,
but we hadn't ever deleted those. Now there's no memory leak here.
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
../../libraries/AP_HAL_Linux/Perf.cpp: In member function ‘void Linux::Perf::_debug_counters()’:
../../libraries/AP_HAL_Linux/Perf.cpp:85:36: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘uint64_t {aka long unsigned int}’ [-Wformat=]
c.name, c.count);
^
Test code for integration with another thread to pull data from internal
perf counters. Since we are using the timer thread here, there's no
retry mechanism and we only print that data can be corrupted.
Instead of creating a new object Perf_Lttng copying the necessaries
fields, just make a tighter integration with the internal perf counters
and re-use the same fields.
The idea is to leave the internal perf enabled all the time, like it is
in PX4, and then allow the integration with lttng on top. Next step
would be to runtime enable/disable only the perf counters we are
interested in.
This also changes the structure so it's easy to allow another thread to
pull data from the Perf object. A rw lock protects from addition of new
counters and an atomic unsigned int allows other threads to do a
lockless copy of the data.
In order for this to work the allocation was changed to use a single
memory pool instead of returning a calloc'ed data for each perf counter.
Since most of our counters are of ' elapsed' type, don't bother using a
smaller struct for the 'count' type
If the RPi version detection fails, the standard version is now RPi2/3 instead of RPi1.
I think this is useful, because the RPi1 is not really supported (performance reasons).
Like others, use HAVE_ prefix and name it HAVE_LTTNG_UST to be the same
name as exported by pkg-config While at it remove wrong comment with
_HELLO_TP_H.
Implementation of alloca() is very much architecture and compiler
dependent. Avoid the case in which it could return a non-aligned
pointer, which would mean Thread::_poison_stack() would do the wrong
thing.
../../libraries/AP_HAL_Linux/Thread.cpp: In member function ‘void Linux::Thread::_poison_stack()’:
../../libraries/AP_HAL_Linux/Thread.cpp:63:20: warning: declaration of ‘start’ shadows a member of 'this' [-Wshadow]
uint8_t *end, *start, *curr;
^
Sort include alphabetically and make them in order:
Main header
system headers
library headers
local headers
While reordering, change a include of endian.h to our sparse-endian.h
which is more reliant to toolchain changes.
Running the vehicles we check the stack size doesn't grow too much by
enabling the DEBUG_STACK in the scheduler. Even on 64bit boards the
stack is consistent around 4k. Just to be a little conservative, let it
be a little bit more that that: 256kB.
Since we have RT prio and we call mlock(), the memory for the stack of
each thread is locked in memory. This means we are effectively taking
that much memory. The default stack size varies per distro, but it's
common to have 8MB for 64 bit boards and 4MB for 32 bit boards. Here is
the output of ps -L -o 'comm,rtprio,rss $(pidof arducopter-quad)', showing the
RSS of arducopter-quad before and after this change:
Before:
COMMAND RTPRIO RSS
arducopter-quad 12 46960
sched-timer 15 46960
sched-uart 14 46960
sched-rcin 13 46960
sched-tonealarm 11 46960
sched-io 10 46960
After:
COMMAND RTPRIO RSS
arducopter-quad 12 7320
sched-timer 15 7320
sched-uart 14 7320
sched-rcin 13 7320
sched-tonealarm 11 7320
sched-io 10 7320
We don't need all the comments in the array declaration and we can
inline its declaration in the function call. This makes it easier to
copy it to other places.
This bug led to issues for us so it may help others to resolve it.
Currently, the AP_HAL_Linux RCInput::read(uint16_t*,uint8_t) function
only returns the first x nonzero channels. Once it hits a channel that
is set to zero, it stops and all remaining channels are returned as
zero, even if they are set. This causes discrepancies between the raw RC
input sent to the GCS and the RC input that is actually used on the
vehicle.
The fixes this issue and makes it behave exactly as it does on the
PX4_HAL code. We ran into this issue when sending rc_override messages
in which there were some channels set to zero.
0-length arrays are supported in C but forbidden in C++. GCC allows it
but clang is more strict:
../../libraries/AP_HAL_Linux/SPIDriver.cpp:75:35: fatal error: no matching constructor for initialization of 'Linux::SPIDeviceDriver [0]'
SPIDeviceDriver SPIDeviceManager::_device[0];
^
../../libraries/AP_HAL_Linux/SPIDriver.h:20:7: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided
class SPIDeviceDriver : public AP_HAL::SPIDeviceDriver {
^
../../libraries/AP_HAL_Linux/SPIDriver.h:20:7: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided
../../libraries/AP_HAL_Linux/SPIDriver.h:25:5: note: candidate constructor not viable: requires 9 arguments, but 0 were provided
SPIDeviceDriver(const char *name, uint16_t bus, uint16_t subdev, enum AP_HAL::SPIDeviceType type, uint8_t mode, uint8_t bitsPerWord, int16_t cs_pin, uint32_t lowspeed, uint32_t highspeed);
^
1 error generated.
This allows us to re-use SPIDevice from SPIDeviceDriver (the
to-become-SPIDeviceProperties) while the drivers are
converted. We create a fake device by calling the temporary
SPIDeviceManager::get_device() method passing the descriptor. The
transfer and assert logic is still using the old code.
Now we can interoperate SPIDeviceDriver with the ones based in
SPIDevice since they are going to use the same semaphore and bus.
The way this code is structured is a little bit different from the
SPIDriver implementation:
- We only open the bus once, no matter how many devices we have in it
- There's a single transfer() method which uses half-duplex mode
instead of full duplex. The reason is that for all cases in the
codebase we are using half-duplex transfers using the full-duplex
API, i.e. a single SPI msg with both tx and rx buffers. This is
cumbersome because the buffers need to be of the same size and the
receive buffer using an offset of the same length as the actux data
being written. This means the high level APIs need to copy buffers
around.
If later we have uses for a real full duplex case it's just a matter
of adding another transfer_fullduplex() method or something like
this.
- The methods are implemented in the SPIDevice class instead of having
proxy methods to SPIDeviceManager as is the case of SPIDriver
Also from now on we refer to the SPIDriver objects as "descriptors"
because they have the parameters of each device in the
SPIDeviceManager::devices[] table. When SPIDeviceDriver is completely
replaced we can rename them to SPIDeviceProperties.
Save in the manager the number of devices so it can be used in other
places like the SPIDevice implementation. This is a temporary storage
while we migrate to SPIDevice.
While at it use protected rather than private.
This allows us to re-use I2CDevice from I2CDriver while the drivers are
converted. We create a fake device with addr = 0 for each I2CDriver but
we only use the register/unregister logic. The transfer logic still uses
the methods from I2CDriver in order to use the right address.
Now we can interoperate I2CDevice drivers with the ones base in
I2CDriver since they are going to use the same semaphore and bus.
The I2CDriver constructors were changed to re-use the logic in I2CDevice
(it uses a number rather than an string) and the semaphore doesn't live
outside anymore, its embedded in the fake I2CDevice, as well as the
bus's file descritor.
This is a similar function to what we have in I2CDriver, but it can
receive a nullptr to recv or send. It will create 2 i2c_msg structs to
send and receive data to/from the I2C slave.
These are very similar to their counterparts in I2CDriver. The changes
were:
- Don't use fixed buffer with PATH_MAX length: allocate the string
- Change the interface to use std::vector so we can simplify the
implementation
Use pthread's barrier so we don't keep waking up threads with possibly
higher priority during initialization phase.
This also synchronizes all of them to a single point. With the previous
approach it was possible (but unlikely) that a thread hadn't reach the
synchronization point when main thread signalize "system initialized".