mavlink / MAVSDK-Python

MAVSDK client for Python.
https://mavsdk.mavlink.io
BSD 3-Clause "New" or "Revised" License
311 stars 218 forks source link

Ardupilot: Missing log entry and mismatched entry.id when using log_files.get_entries() #532

Open EeMeng94 opened 1 year ago

EeMeng94 commented 1 year ago

I have encountered some weird behaviour when using drone.log_files.get_entries() with the code below:

#!/usr/bin/env python3

import asyncio
from mavsdk import System
import sys
import os

async def run():
    drone = System()
    await drone.connect(system_address="serial:///dev/ttyACM0")
    print("Waiting for drone to connect...")
    async for state in drone.core.connection_state():
        if state.is_connected:
            print(f"-- Connected to drone!")
            break
    await get_entries(drone)

async def get_entries(drone):
    entries = await drone.log_files.get_entries()
    for entry in entries:
        print(f"Log {entry.id} from {entry.date}, size: {entry.size_bytes}")
    return entries

if __name__ == "__main__":
    asyncio.get_event_loop().run_until_complete(run())

Executing the code yields the following result (truncated the middle part):

Waiting for drone to connect...
-- Connected to drone!
Log 0 from , size: 0
Log 1 from 1980-01-01T00:00:00Z, size: 81408
Log 2 from 1980-01-01T00:00:00Z, size: 399360
Log 3 from 1980-01-01T00:00:00Z, size: 399360
Log 4 from 1980-01-01T00:00:00Z, size: 399588
⋮
Log 135 from 2021-01-31T05:02:44Z, size: 8918016
Log 136 from 2021-02-21T01:20:42Z, size: 6872334
Log 137 from 1980-01-01T00:00:00Z, size: 567371
Log 138 from 2022-11-01T09:55:02Z, size: 2679413

The entries seems to have been shifted, causing the 1st entry to be empty (log 0), subsequent entries's log id to be incremented by 1 and the final entry to be missing.

Viewing the pixhawk unit on QGroundControl shows that there are logs with id 0-138 (a total of 139 logs): Screenshot from 2022-12-01 16-16-48 Screenshot from 2022-12-01 16-07-20

JonasVautherin commented 1 year ago

Looks like it could be a bug indeed! Would you be willing to have a look? I would start investigating here

EeMeng94 commented 1 year ago

Hi @JonasVautherin thanks for the reply. Unfortunately I am not able to commit to fixing it due to lack of time.

However it seems like it might have been an APM issue/feature. Based on the code used by QGroundControl, it seems like the 0th APM log entry is bogus. The 1st entry start from index of 1, and the final entry will be have an index of n (instead of n - 1). https://github.com/mavlink/qgroundcontrol/blob/master/src/AnalyzeView/LogDownloadController.cc#L160

JonasVautherin commented 1 year ago

Good to know! Thanks for reporting, and feel free to come back to it when you have time! If that's really an Ardupilot-specific well-known behavior, I think it may be easy to adapt the code for it, similar to e.g. here.

Haven't looked in details, but I could imagine something like this:

if (_parent->autopilot() == SystemImpl::Autopilot::ArduPilot) {
    // Set the index shifted by 1, maybe?
} else {
    // Set the index not shifted by 1
}
julianoes commented 1 year ago

Oh my god, another off by one difference for ArduPilot? That's just crazy.