search

PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.21k stars 13.38k forks source link

[Bug] New Septentrio Driver High CPU Usage #23289

Closed flyingthingsintothings closed 2 months ago

flyingthingsintothings commented 3 months ago

Describe the bug

When enabling the Septentrio driver, it consumes 40-75% of the CPU on the flight controller. This seems to be a bug caused by a commit that wasn't in the history of the PR that introduced the driver. When I compile the latest main branch with the Septentrio driver but with fcb479cd3d667e3625fca37111c524c2a04a08b6 reverted, everything works as expected. fcb479cd3d667e3625fca37111c524c2a04a08b6 wasn't in the history of the PR which is probably why the PR worked and the merged driver isn't working.

What I Tested

To Reproduce

  1. Compile 9c83f842bee11e3942e578377245b6a3fc9b5a7d (latest main branch as of writing) with Septentrio driver included
  2. Flash the build to a flight controller
  3. Run the Septentrio module (make sure the GPS module is disabled)
  4. Run top in the MAVLink console and see high CPU usage

Expected behavior

The version from the PR used around 0.200%. That would also be the expected usage.

Screenshot / Media

The bug

image

Flight Log

\/

Software Version

9c83f842bee11e3942e578377245b6a3fc9b5a7d (latest main branch)

Flight controller

Holybro Pixhawk 6X

Vehicle type

Multicopter

How are the different components wired up (including port information)

The flight controller can be attached to a Septentrio receiver but this isn't required to trigger the bug. Even with just the flight controller the bug happens. Mine isn't attached to a receiver.

Additional context

\/

MaEtUgR commented 3 months ago

We also found issues on main with MAVLink communication over UART. Not exactly understood yet but I commented here: https://github.com/PX4/PX4-Autopilot/pull/23248#issuecomment-2176357080

katzfey commented 3 months ago

I think it's because the timeout value for the readAtLeast call should be in microseconds, not milliseconds, in the file drivers/gnss/septentrio/septentrio.cpp.

katzfey commented 3 months ago

Or, readAtLeast timeout in SerialImpl.cpp should be changed to milliseconds, not microseconds.

katzfey commented 3 months ago

I propose that changing Serial to have a millisecond timeout instead of a microsecond timeout is better because there are other users such as CRSF and GPS that also assume it is in milliseconds.

katzfey commented 3 months ago

See https://github.com/PX4/PX4-Autopilot/pull/23296

flyingthingsintothings commented 2 months ago

I tested the latest main branch and it seems to be fixed. Thank you for the help.