mavlink / MAVSDK

API and library for MAVLink compatible systems written in C++17
https://mavsdk.mavlink.io
BSD 3-Clause "New" or "Revised" License
622 stars 507 forks source link

Building MAIN breaks Euler Angle Values - ArduCopter (Returns NaN but works in v1.4.16) #2143

Open OliverHeilmann opened 1 year ago

OliverHeilmann commented 1 year ago

Hello!

I have been following #2031 as I had the same issue with the GUIDED mode. I could not get a drone to take off without manually setting it to GUIDED mode first. Thankfully, this issue has been solved now, thanks dev team!

I followed the suggestion of building main from source and that has worked nicely so thank you. Unfortunately however, my previously working eulerAngle telemetry gets are now broken/ only return NaN values.

As the title suggests, this issue is raised for ArduCopter (and Rover, Plane etc.) and not for PX4 (which I have tried main and it works fine).

This is the code I used:

mavsdk::Telemetry::EulerAngle eulerAngle = telemetry->attitude_euler();

Using main I get the following results for euler angle params: Screenshot 2023-09-20 at 22 07 27

Using v1.4.16, I get the following results for euler angle params: Screenshot 2023-09-20 at 22 05 02

I suspect there is some code change that needs to be modified back to the v1.4.16 version?

Thanks for your help in advance!

julianoes commented 1 year ago

Possible. Can you check whether ArduPilot sends out https://mavlink.io/en/messages/common.html#ATTITUDE or https://mavlink.io/en/messages/common.html#ATTITUDE_QUATERNION and what the values are?

OliverHeilmann commented 1 year ago

Thanks for the speedy response. A few things to add here...

1) Using mavsdk::Telemetry::Quaternion quaternionAngle = telemetry->attitude_quaternion(); I printed the outputs and they also read NaN as illustrated in the picture below: Screenshot 2023-09-21 at 11 41 32 It is worth noting that these results are from the ArduPilot SITL environment. 2) I did the same test with physical drone hardware and I discovered that none of the telemetry data (including Lat/ Lon etc.) were sending at all. After opening Mission Planner, connecting to the drone, closing it again and then running my code, the other telemetry started sending (excluding the Euler and Quaternion angles as before). This reminds me of another Issue I raised a while back #1967. But to be clear, connecting is fine now, it's just the telemetry values returning NaN.

JonasVautherin commented 1 year ago

With Ardupilot, do you need to somehow enable some telemetry with set_rate or something?

OliverHeilmann commented 1 year ago

With Ardupilot, do you need to somehow enable some telemetry with set_rate or something?

@JonasVautherin hitting the nail on the head with the one sentence solution! 😆

I added the following two lines in before the checks and it worked! I have added this in my code in a suitable place but perhaps this is something that could be incorporated into MavSDK? Perhaps a PR... I'd be happy to test it out if you like.

telemetry->set_rate_attitude_euler(10.0);
telemetry->set_rate_attitude_quaternion(10.0);

Thanks for your help!

JonasVautherin commented 1 year ago

perhaps this is something that could be incorporated into MavSDK?

Yeah I wonder. It surely would be nice if it was consistent between PX4 and Ardupilot when using MAVSDK... Maybe MAVSDK should just set default rates for everything? @julianoes what do you think?

julianoes commented 1 year ago

I don't think it should. It depends on the link and what you're trying to do. Otherwise, you end up with way too much traffic on the link, depending on the link of course.

JonasVautherin commented 1 year ago

But how does it work then? If there is no default, then the client should always set it anyway, in order to get consistent behavior, right?

OliverHeilmann commented 1 year ago

How about setting a rate if/ when that specific telemetry type is requested? e.g.

mavsdk::Telemetry::EulerAngle euler = telemetry->attitude_euler();    // inside attitude_euler() method somewhere

Some check to see if a rate has already been set or something... You'd still have to choose a number of course but then the user can at least change it if they decide they want to. Would need to think about cases where user set_rate then calls get_telemetry to ensure rate isn't overwritten but that's doable right?

EDIT: Also, why was it working in v1.4.16? Do you remember what changes were made since then?

JonasVautherin commented 1 year ago

Actually the problem with MAVSDK setting a rate is that it will conflict with other clients. Say QGC sets some rate and then you start MAVSDK, then MAVSDK won't know about QGC and will just overwrite it.

That's a tricky problem :thinking:.

OliverHeilmann commented 1 year ago

Ok follow up observation here.

I've noticed that when I set set_rate_attitude_euler(...) but NOT set_rate_attitude_quaternion(...), I don't receive any telemetry for Euler or Quat angles. When I do the reverse (Quat and not Euler), I receive both Euler and Quat angles.

Perhaps this is a quirk for ArduPilot and the way Mavlink messages are interpreted, or perhaps this is a MavSDK issue? Not sure so thought I'd post here.

In regards to your other comment @JonasVautherin, I think it is reasonable to have some logic where you get_rate to check if a non-zero value has already been set, if so, don't overwrite it... could this solve your QGC problem description? Though, what this doesn't solve is the case where MavSDK sets a rate first and then QGC overwrites it right?

In any case, It would be good to actually get some telemetry if you are polling for it, regardless of rates. I wouldn't have posted this issue if I saw values returning for instance.

Actually the problem with MAVSDK setting a rate is that it will conflict with other clients. Say QGC sets some rate and then you start MAVSDK, then MAVSDK won't know about QGC and will just overwrite it.

That's a tricky problem 🤔.

JonasVautherin commented 1 year ago

Perhaps this is a quirk for ArduPilot and the way Mavlink messages are interpreted, or perhaps this is a MavSDK issue?

Sounds like something that MAVSDK should handle properly :+1:.

I think it is reasonable to have some logic where you get_rate

I am not sure if MAVLink provides any way of getting the rate, though :thinking:. Not sure if there is a solution to that (other than pushing the problem to the application developers, that they need to tune for their particular app, use-case and drone) :confused:.

OliverHeilmann commented 1 year ago

I am not sure if MAVLink provides any way of getting the rate, though 🤔. Not sure if there is a solution to that (other than pushing the problem to the application developers, that they need to tune for their particular app, use-case and drone) 😕.

I see MAV_CMD_SET_MESSAGE_INTERVAL and MAV_CMD_REQUEST_MESSAGE in the docs but, you're right, I don't see any way of getting current rate anywhere... interesting.

I am still wondering if it is clear to devs using MAVSDK with ArduPilot that they need to use set_rate to get the telemetry stream though?

Also wanted to raise a seg-fault issue here too as I also encountered it when I changed my MAVSDK version to main and built from source (which is when I also discovered the Euler telemetry issue discussed above as well). I suggested re-opening the issue but can create a new one if you think it is sensible to do so.