Without lockstep the actual monotonic clock of the host computer is
used. Therefore, this time is likely much more than 20 seconds and the
check if the boot complete happened in time will fail immediately.
Therefore, it probably makes more sense to count the time from the first
mavlink creation.
The asinf function is NaN outside of -1 to 1. Therefore, it probably
makes sense to constrain the input to prevent NaN setpoints further down
the line.
This was added to enable a Pixhawk to be used as an RC input for e.g.
SITL. As far as we're aware of that's not really used. However, sending
this can cause issues if multiple Pixhawks are in the same network.
Also, is uses up some of the MAVLink bandwidth.
Therefore, it's probably best to remove that feature for now.
My suggestion is to re-configure stale bot so it just adds the label
stale but does not close the issue/PR.
This enables us to look for outdated issues using the stale label and
close them.
This brings these advantages:
- No more endless re-opening of PRs and issues which is not fun (and I
will eventually give up on).
- Less stalebot comment noise. (Because it is handy to look for all
issues/PRs without any comments to see what is unanswered).
- I'm not listed as "participating" in all the issues/PRs just because
I was just fighting the stalebot.
- Less notifications meaning we can see anything in the noise.
This fixes the regression where we saw this in SITL:
ERROR [mavlink] system boot did not complete in 20 seconds
The reason was that the stream subscription requests were not checked
because the while loop was not running yet because mavlink was not
booted completely.
By moving the stream subscription requests into a function we can run
them even if we don't run the rest of the loop.
This changes two things:
- This adds "-beta" as a valid tag suffix.
- This changes "rc" without "-" to "-rc". "rc" without is now described
as deprecated and with "-rc" is preferred.
When we send a command to any sysid or any compid, we need to match an
ack from a specific sysid or compid. If we don't do that, we keep
sending retransmissions and eventually time out.
The int32 param COM_ARM_AUTH is mapped to a packed struct. However, this
struct was not actually packed (anymore) and therefore the values were
applied incorrectly.
I fixed this by applying the packed attribute. By using a union with a
int32_t I could rid of the warning about address-of-packed-member.
This fixes the terrifying case where the drone disarms in-air just
because it receives a MAVLink disarm command. We now check param2 for a
magic number which enforces arming/disarming.
This is added to the mavlink protocol in:
https://github.com/mavlink/mavlink/pull/1162
In order to be consistent between Gazebo and jMAVSim we should be using
the env variable PX4_SIM_SPEED_FACTOR in the same way. This enables
vscode to set the speed using the env variable.
This changes the enums used for various VTOL states to enum classes
which makes them type-safe and should avoid shadowing.
This change was motivated by a Clang warning about shadowing of the
enum const TRANSITION_TO_FW which was declared twice, once in
vtol_type.h and once in standard.h.
This change only removes the shadowing but presumably these enums could
be cleaned up and consolidated further.
This moves the hysteresis test out of the systemlib and makes it its own
small library. Since it still depends on hrt_absolute_time this does not
link yet. My attempt to get all link dependencies together failed.
This tries to make the visibility.h header clearer and better
structured. We don't actually need the ifdef for lockstep enabled or
disabled because that's now handled elsewhere.
This makes sure we add the lockstep_scheduler_test even if the
ENABLE_LOCKSTEP_SCHEDULER is not set to yes. This means the
lockstep_scheduler is not used for SITL but the CMakeLists.txt file
still used and the test added.
GCC 9 complained about stringop-truncation which is a cautionary message
to prevent using strncpy with non-null terminated strings.
We can fix this by copying one byte less than the destination size and
then manually adding the null termination, as we already do.
When simulating with lockstep we can raise the speed by setting the env
variable `PX4_SIM_SPEED_FACTOR`. Some inputs like RC, MAVLink heartbeats
from a ground station, or offboard controls via MAVLink are still at the
normal speed which leads to timeouts getting detected in PX4.
To work around this issue we can automatically multiply the timeout
parameters by the speed factor.
This fixes the IDs of multi UAVs started with ROS/Gazebo.
Previously the 3 vehicles were displayed in QGC as Vehicle 2, 3, 4.
With this change it is more intuitive Vehicle 1, 2, 3 and this is
also consistent with the way it is documented and how it is in
jMAVSim.
When sih (simulation in hardware) was compiled for Snapdragon, we got
the linking problem below. I'm not sure why, supposedly the symbol
`double sin(double)` is missing.
undefined PLT symbol _LSin (705) /libpx4.so (symbol.c:303)
This way we prevent a big dip due to TECS in altitude when the
transition happens at 10 m/s already. Apparently the rule of thumb
is to set this transition speed at the same as airspeed cruise speed.
Includes:
- Fix for vibration on ground for standard_vtol.
- Noise for diff pressure / airspeed on Gazebo side.
- Fix for macOS magnetometer plugin linking error.
By declaring the params above the navigator modes inheriting from
mission block we make sure the params in navigator are set before
read in the constructor of mission block.
This should fix the case where the driver initializes even though the
device is not found. The change changes the behavior to return ERROR
if the whoami call fails several times instead of returning OK.
Also, the reset() and thus probe() calls are moved before initializing
the ringbuffer and device name.
This fixes build races which happened if "Unix Makefiles" instead of
ninja-build was used as the cmake backend.
For any dependencies of commands on files we need to create a target.
Otherwise, if "Unix Makefiles" are used as the generator the commands
are run in parallel on the different files which often can lead to
races or redundancies in our build.
A nice write-up can be found here:
https://samthursfield.wordpress.com/2015/11/21/
cmake-dependencies-between-targets-and-files-and-custom-commands/#
custom-commands-and-parallel-make
When using QGC and/or the Dronecode SDK it was possible to get in a
state where the two mavlink instances were both publishing their last
heartbeat_time and cause commander to consistently toggle between
data link lost and regained. With this fix, we only ever look at the
very last heartbeat time and therefore seem to avoid this issue.
This is a workaround for the write timeout that we have seen for some
host computers trying to flash the firmware.
We don't know the root cause of the problem but we do observed the
following:
- For blocking writes with timeout (Pyserial write_timeout=0.5):
write() throws SerialTimeoutException. In systrace we see that the
select() call after write waiting for the write to be finished hangs
and finally times out.
- For blocking writes without timeout (Pyserial write_timeout=None):
write() hangs indefinitely. In systrace we see that the
select() call after write waiting for the write to be finished hangs.
- For non-blocking writes:
write() works but flush() hangs. In systrace we see that
ioctl(fd, TCSBRK, 1) which is (correctly) triggered by termios tcdrain
hangs.
Inspecting USB traffic using usbmon, we can see that the data which is
written actually seems to be sent and looking at responses from the
Pixhawk bootloader and the timings it looks like all the data has
arrived.
This workaround uses non-blocking writes without flushing and this
seemed to prevent the issue from happening so far.
Debugging was done in collaboration with Beat Küng and David Sidrane.
The replay functionality was broken with lockstep. This is an interim
fix for the replay functionality.
In the longer term it would be nice to leverage the lockstep speedup
for the replay.
If we dont explicitly check for pyserial, we can have the case where the
import works but the Serial object creation fails. However, we don't see
this because we have this huge try/catch block which swallows
everything.
It turns out that we can fix the unit tests of the lockstep_scheduler
just by checking if `passed_lock` is not `nullptr`.
Without this check, the unit tests segfaulted.
This updates the jMAVSim submodule which includes a fix for HITL.
In order to fix HITL, a CLI argument `-lockstep` was required to enable
lockstep. This has now been added to the command in jmavsim_run.sh.
By using the uORB::Subscription API we use a separate subscription
rather than `orb_copy` on the existing file descriptor used in
sensors through `px4_poll`.
This fixes a very peculiar problem that we observed in SITL in CI for
fixedwing. The events were as follows:
1. `sensors` does `px4_poll` on the gyro topic (as normal), and gets
the latest sample using `orb_copy`.
2. A parameter update happens when the mag is initialized and triggers
`VotedSensorsUpdate::parameters_update()` where `orb_copy` happens
before the main loop in `sensors` has started a `px4_poll`.
3. `sensors` now does the `px4_poll`, however waits indefinitely because
it has already copied the latest sample.
Also, the `px4_poll` will never time out because in lockstep the
simulator waits for the next actuator control message which it never
gets and therefore it never sends the next sensor message with a new
timestamp to advance the time.
This only happens for fixedwing because there is only one "uORB path"
through the system unlike for multicopter where a gyro sample can get
picked up by either `sensors` or directly `mc_att_control`, so the
system can survive if `sensors` has "drops".
This removes the confusing ugly magic number of 233 introduced as part
of https://github.com/PX4/Firmware/pull/7670.
Also, the convoluted if is cleaned up using 3 separate bools with some
comments to explain what's going on.
This should not change anything function-wise except that the flight
controller could now potentially also use system ID 233 and not break
forwarding.
This avoids casting errors. An alternative to `std::round` would be to
add 0.5 before casting for positive values and subtract 0.5 before
casting for negative values.
The API of cond_timedwait was wrong. It used return -1 and set errno
instead of returning the error as specified for pthread_cond_timedwait
which it tries to mock.
It turns out that `sendto` does not work for TCP on Cygwin-Windows,
instead we need to use `send`. This required some refactoring since we
need to have the internet protocol and the port stored as a member
variable.
This should fix that lockstep SITL simulation was not working on
Windows.
It seems that the hysteresis test fails every now and then, presumably
due to timing issues. The tests needs some improvements, e.g. isolating
it from the system time.
It turns out the noise of the airspeed depends on the least significant
bit of time which is now always 0 because time is quite regular with
lockstep. Therefore, we need a better source for the randomness. I chose
to use the sign of the y-gyro and it seems to work.
Julian Oes