MAVSDK icon indicating copy to clipboard operation
MAVSDK copied to clipboard

Timesync adjustment for incoming messages is not implemented?

Open chengguizi opened this issue 2 years ago • 12 comments

Hi,

Thanks for maintaining this SDK, looking very promising!

However I am inspecting its implementation for PX4 integration. I realise the incoming messages from telemetry is NOT adjusted based on time offset estimated, Example here

https://github.com/mavlink/MAVSDK/blob/e7231dc8f390b96f4682e6810ac3d2fe554b5b8b/src/mavsdk/plugins/telemetry/telemetry_impl.cpp#L966

Cross reference to mavros, the correct behaviour should be something like this.

https://github.com/mavlink/mavros/blob/c035b8ff1ffb042db235648371f5d7d2e531fae4/mavros/src/plugins/imu.cpp#L425

Is this a missing feature? Anyway to work around this by applying the offset ourselves? I see the timesync data seems not exposed in the API. Thanks!

Related: https://github.com/mavlink/MAVSDK/issues/1033

chengguizi avatar Mar 09 '23 08:03 chengguizi

To add on, seems the filtering of time offset estimate is also missing

https://github.com/mavlink/mavros/blob/master/mavros/src/plugins/sys_time.cpp#L406

chengguizi avatar Mar 09 '23 08:03 chengguizi

Thanks for the words and nice write up!

Timesync itself should be implemented, however, you need to enable it for a system using enable_timesync(). However, I wasn't aware that we should be applying the offset. So you're saying that the offset should be included in all messages with a time_usec field, or just the IMU one?

Regarding filtering: I guess that would make the time sync work smoother, right? I would gladly take a pull request that implements and tests that.

julianoes avatar Mar 09 '23 20:03 julianoes

Hi @julianoes , thanks for the quick reply. Yes, from what I know working with PX4 systems and mavros, all incoming messages should have synchronisation offset applied.

Another example on local_position here https://github.com/mavlink/mavros/blob/c035b8ff1ffb042db235648371f5d7d2e531fae4/mavros/src/plugins/local_position.cpp#L163

Namely, the offset application should be done on the recipient, not the sender.

And yes, I am currently fixing it to test with PX4 system, but I can tell it gonna be quite breaking change to the codebase, particularly if we want to fix all the incoming messages. What I am currently doing:

  • expose the time offset from the system object, and apply the offset to the messages outside the Telemetry callback
  • for sending out information, I have to change code and make sure they are sending the monotonic clock instead.

If I am to do a PR, which version i should be working on? v1.4.12? I see the API is quite different from the master and I am not sure why.

chengguizi avatar Mar 10 '23 00:03 chengguizi

Btw, I would like to know if MAVSDK supports the operation to run MAVSDK app on two Ubuntu machines, and talk to each other in UDP? Aka, none of them are actual flight controllers / autopilot. Would like to have the timesync working there as well.

chengguizi avatar Mar 10 '23 00:03 chengguizi

I think given this changes the API it's probably best to do it for MAVSDK v2, so in the main branch.

What would be the point of two MAVSDK instances? Would one be the autopilot_server and the other one the client, or how would this work?

julianoes avatar Mar 10 '23 01:03 julianoes

@julianoes ok! Let me move to MAVSDK v2 on the main then.

Is the master branch consider stable? Or is there a stable release so far for MAVSDK v2? I need to test it on real PX4 systems, so can't take anything too unstable.

For two MAVSDK instances. The use case is following:

  • We might want two robotic system to communicate with each other over a (wireless) serial link. MAVSDK has the flexibility of doing TCP, UDP and serial which is great!
  • For such two systems, we may not always have autopilot. They can be embedded Linux system. Would be great to enjoy the infrastructure built by MAVSDK if all possible. For both data transmission, proper message packaging and timesync.

chengguizi avatar Mar 10 '23 03:03 chengguizi

Also, I cant seem to find any information about MAVSDK v2. What is the purpose / benefits for the main version upgrade?

chengguizi avatar Mar 10 '23 03:03 chengguizi

What is the purpose / benefits for the main version upgrade?

I can answer this :). It's really just semantic versioning. We have changed the MAVSDK API, and therefore the next release should increase the major version to indicate that it will break the build.

Is the master branch consider stable?

In terms of what it does with MAVLink, I would consider it "safe" (at least as safe as 1.4.x). In terms of API, it may still have small changes before v2 is released (since that's an opportunity to change the API :innocent:). So the biggest consequence is that if you work against main, maybe once in a while when you update, it will break your build and you will have to fix it (maybe a function name changed, a typo was fixed, something like that).

JonasVautherin avatar Mar 10 '23 10:03 JonasVautherin

* For such two systems, we may not always have autopilot. They can be embedded Linux system. Would be great to enjoy the infrastructure built by MAVSDK if all possible. For both data transmission, proper message packaging and timesync.

fwiw, yes, this is possible. Based on a few examples that @JonasVautherin made a few years ago, I made something similar. I use a server and client to communicate between a groundstation and a drone that does not use PX4 (DJI). For the same reason, I wanted to profit from the strength of MAVSDK/mavlink.

The code was from a PR branch I believe. Maybe @JonasVautherin can dig it up?

reedev avatar Mar 10 '23 11:03 reedev

The code was from a PR branch I believe. Maybe @JonasVautherin can dig it up?

Hmm I'd say it's unlikely to still work, that was a while ago (I would guess 2019). But with the action_server and telemetry_server (which did not exist back then), I think it should not be too hard to make something that looks like an autopilot.

JonasVautherin avatar Mar 10 '23 13:03 JonasVautherin

@JonasVautherin

For MAVSDK v2, it sounds great. I dont mind minor breaking of API over time, as long as the MAVLInk part is functioning without critical bugs / missing features compared to 1.4.x.

For the p2p communication over mavlink, using mavsdk codebase. I do find your suggestion on action_server and telemetry_server promising. Could not find much documentation, but the example code seems helpful

https://github.com/mavlink/MAVSDK/blob/main/examples/autopilot_server/autopilot_server.cpp

Basically i just need one computer act as a ground station and open its udp port, and the other one acts as autopilot / companion computer.

I am currently not sure which pair would work as p2p communication:

        enum class UsageType {
            Autopilot, /**< @brief SDK is used as an autopilot. */
            GroundStation, /**< @brief SDK is used as a ground station. */
            CompanionComputer, /**< @brief SDK is used as a companion computer on board the MAV. */
            Camera, /** < @brief SDK is used as a camera. */
            Custom /**< @brief the SDK is used in a custom configuration, no automatic ID will be
                      provided */
        };

Any suggestions?

chengguizi avatar Mar 11 '23 03:03 chengguizi

I would just try to use Autopilot for the "server" side, and GroundStation for the ground.

It should not matter too much though, as long as the components have a different sysid.

JonasVautherin avatar Mar 11 '23 10:03 JonasVautherin