* fix(uavcan): apply UAVCAN_BITRATE on FlexCAN again
58dfff0de7 turned the SIOCSCANBITRATE path off for i.MX RT, S32K and Kinetis,
which is every board in the tree that builds SocketCAN, so UAVCAN_BITRATE
stopped doing anything at all. It was written before db6f7ec304, and the crash
it works around is that commit's bug: the driver reports bit/s on NuttX 12 and
the caller still divided by 1000, so a 1 Mbit/s board asked its controller for
1000 bit/s on every boot.
Benched on an ARK FMU-v6XRT with the gate removed: 1 Mbit/s -> 500 kbit/s ->
1 Mbit/s round-trips with both DroneCAN nodes coming back OPERAT, buses
error-active, no fault. Requesting a rate the controller cannot reach is also
survivable now -- it is reported and the configured rate is kept, where before
a negative return from here made CanDriver::init() give up and DroneCAN never
started.
The %u for a uint32_t is fixed as well; nothing compiled this branch while the
gate was in place.
* fix(work_queue): size the uavcan and TTY work queues for NuttX 12
NuttX 12.12 costs a few hundred bytes of stack on the paths that go through
the file layer, and that is more than these two defaults left spare. Measured
against the same PX4 tree on 10.3.0:
wq:uavcan 2860/3728 -> 3184/3736 (ARK FMU-v6X, two DroneCAN GNSS nodes)
wq:ttyS4 1088/1704 -> 1432/1712 (ARK FMU-v6XRT, crsf_rc)
load_mon reports wq:ttyS4 low on stack with 280 bytes left, and wq:uavcan sits
at 85 % of its own. 4096 and 2048 restore the margin the 10.3.0 figures had.
* refactor(px4_work_queue): out-line ScheduledWorkItem constructor to save flash
The header-inline constructor zero-initialises the hrt_call member at
every derived work-item constructor across ~150 classes. Move the
definition (verbatim) into ScheduledWorkItem.cpp next to the
destructor; the derived constructors already pay a call into the base
constructor chain, so this only removes the duplicated inline stores.
Saves 1376 B of .text on px4_fmu-v6x_default.
Signed-off-by: Balduin <balduin@auterion.com>
* refactor(px4_work_queue): out-line WorkItem::ScheduleNow to save flash
The body was inlined at ~240 call sites; a plain call is smaller at
every one of them.
Saves 672 B FLASH on px4_fmu-v6x_default.
Signed-off-by: Balduin <balduin@auterion.com>
---------
Signed-off-by: Balduin <balduin@auterion.com>
Co-authored-by: Balduin <balduin@auterion.com>
Two related fixes for HRT callout queue corruption under lockstep SIH:
1. WorkQueue::Run() popped a WorkItem, released the work-queue lock,
then called work->Run(). If another thread called Deinit on the
item in that window, wq->Remove was a no-op (item already popped)
and Deinit returned while Run() was still executing. Run() could
then call ScheduleDelayed → hrt_call_after on an object that the
caller of Deinit has since destructed and reused via placement new
— the hrt_call metadata ends up living inside another object's
fields, which later overwrite the flink pointer and tear the hrt
callout queue.
Fix: WorkItem carries a _run_in_progress atomic flag set by
WorkQueue::Run around work->Run(). Deinit spins on that flag after
wq->Remove so it cannot return while a Run() is still executing.
WorkQueue captures its worker tid in the constructor (the ctor
runs on the worker thread) so Deinit can skip the self-wait when
called from inside Run() itself (e.g. should_exit() paths that
invoke ScheduleClear).
2. _hrt_lock was a px4_sem_t and hrt_call_invoke unlocked around the
callback so callbacks could re-enter hrt_call_*. That also exposed
the queue to other threads mid-invocation. Switch _hrt_lock to a
PTHREAD_MUTEX_RECURSIVE pthread_mutex and hold it across the
callback — matches NuttX's enter_critical_section() nesting
semantics, lets callbacks reschedule themselves safely, and
prevents concurrent queue manipulation.
Together these eliminate the HRT queue tearing observed in the
SIH-at-20x MAVSDK integration soak (torn flink chains with the tail
unreachable from the head; orphan nodes pointing into reused
memory).
Move the helper from the header into WorkQueue.cpp. Drop the
SignalWorkerThread() call from request_stop() — request_stop sets
_should_exit but the worker is woken up via the existing exit path
in Detach already, so the extra signal here is redundant.
WorkItemSingleShot::_sem is a signaling semaphore, disable PI for it.
Set CONFIG_DEBUG_ASSERTIONS=y and the kernel panics due to the semaphore
having no holder, disabling PI fixes this.
In NuttX protected build there are separate work queues in kernel and user sides.
pthreads are only available in user side, so use tasks and kthreads for
memory protected builds.
Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
When the last WorkItem is deleted, it is removed from a work queue and the
queue is being stopped. But, the queue itself might get deleted in the middle,
in a higher priority thread than where the WorkItem deletion was performed from
If the WorkQueue::Detach accesses the member variables after this, there is memory
corruption
This happens in particular when launching i2c or spi devices in
I2CSPIDriverBase::module_start:
- The "initializer" is deleted when the instance is not found and the iterator
while loop continues.
- The workqueue is deleted in the middle of "initializer" deletion when the
WorkQueueRunner returns.
This prevents deletion of the WorkQueue before the Detach has been finished,
in the specific case that the ::Detach triggers the deletion
Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
- the purpose is to ensure that every WorkItem (and WorkItems scheduled
by WorkItems) is allowed to run to completion every step
- per workqueue register a lockstep component whenever a work item is
added (if not already registered)
- once the work queue is empty unregister component
- ekf2 can now run in multi-instance mode (currently up to 9 instances)
- in multi mode all estimates are published to alternate topics (eg estimator_attitude instead of vehicle_attitude)
- new ekf2 selector runs in multi-instance mode to monitor and compare all instances, selecting a primary (eg N x estimator_attitude => vehicle_attitude)
- sensors module accel & gyro inconsistency checks are now relative to the mean of all instances, rather than the current primary (when active ekf2 selector is responsible for choosing primary accel & gyro)
- existing consumers of estimator_status must check estimator_selector_status to select current primary instance status
- ekf2 single instance mode is still fully supported and the default
Co-authored-by: Paul Riseborough <gncsolns@gmail.com>
- drivers/tone_alarm: move to ModuleBase and purge CDev (/dev/tone_alarm0)
- drivers/tone_alarm: only run on tune_control publication (or scheduled note) rather than continuously
- drivers/tone_alarm: use HRT to schedule tone stop (prevents potential disruption)
- msg/tune_control: add tune_id numbering
- systemcmds/tune_control: add "error" special case tune_id
- move all tune_control publication to new uORB::PublicationQueued<>
- start tone_alarm immediately after board defaults are loaded to fix potential startup issues
- for SITL (or other boards with no TONE output) print common messages (startup, error, etc)
- only record start time on first run rather than init
- increase name length
- round average interval to nearest microsecond
- basic formatting consistency (google style guide)
The existing behavior is unexpected: if the work item is already on the
runnable queue, it will still be triggered after a call to ScheduleClear().
This can lead to race conditions.
- this is a new module for temperature compensation that consolidates the functionality previously handled in the sensors module (calculating runtime thermal corrections) and the events module (online thermal calibration)
- by collecting this functionality into a single module we can optionally disable it on systems where it's not used and save some flash (if disabled at build time) or memory (disabled at run time)
Setting PTHREAD_EXPLICIT_SCHED was necessary on linux to create WQ threads with priorities relative to max, but unfortunately we can't rely pthread_attr_setinheritsched as it's dependent on system ulimit configuration or running privileged. Instead we can create the wq:manager at the maximum desired priority and allow each WQ thread to have a relative priority.
getLockGuard relies on copy elision to work correctly, which the compiler
is not required to do (only with C++17).
If no copy elision happens, the mutex ends up being unlocked twice, and the
CS is executed with the mutex unlocked.
The patch also ensures that the same pattern cannot be used again.
and remove the px4_ prefix, except for px4_config.h.
command to update includes:
for k in app.h atomic.h cli.h console_buffer.h defines.h getopt.h i2c.h init.h log.h micro_hal.h module.h module_params.h param.h param_macros.h posix.h sem.h sem.hpp shmem.h shutdown.h tasks.h time.h workqueue.h; do for i in $(grep -rl 'include <px4_'$k src platforms boards); do sed -i 's/#include <px4_'$k'/#include <px4_platform_common\/'$k/ $i; done; done
for in $(grep -rl 'include <px4_config.h' src platforms boards); do sed -i 's/#include <px4_config.h/#include <px4_platform_common\/px4_config.h'/ $i; done
Transitional headers for submodules are added (px4_{defines,log,time}.h)
This could happen in the following cases:
- IRQ/publisher rate is faster than the processing rate, and therefore
WorkQueue::Add is called at a higher rate
- a long-running or stuck task that blocks the work queue a long time
Both cases are not expected to happen under 'normal' circumstances (if the
system runs as expected).
* adds a work_queue systemcmd that will bring a tree view of all active work queues and work items
* WorkQueues now track attached WorkItems and will shutdown when the last WorkItem is detached
Script to update include paths:
for i in $(grep -rl 'include <px4_work_queue' src platforms); do sed -i 's/#include <px4_work_queue/#include <px4_platform_common\/px4_work_queue/' $i; done