MAVSDK
MAVSDK copied to clipboard
mavlink
info: updated the message to obtain flight and autopilot info as per …
…the mavlink spec. The following messages are deprecated in the MAVLINK protocol
MAV_CMD_REQUEST_FLIGHT_INFORMATION MAV_CMD_REQUEST_AUTOPILOT_CAPABILITIES
The suggested way to get the same information is using the following command message: MAV_CMD_REQUEST_MESSAGE
with appropriate message ID for requested message which would be for flight information the following message: MAVLINK_MSG_ID_FLIGHT_INFORMATION
Note APM does not implement this yet. See Issue https://github.com/ArduPilot/ardupilot/issues/15217
and for autopilot version: MAVLINK_MSG_ID_AUTOPILOT_VERSION 148
This PR resolves this incompatibility with the MAVLINK protocol.
Happy to hear your thoughts :)
My concern with the fallback is this: what if the drone supports the new REQUEST_MESSAGE message, but the request is lost? MAVSDK will time out, not because it is unsupported, but because of an actual timeout. If MAVSDK now falls back to the old message and the drone does not support it, it will timeout again, this time because it is not supported. How do we handle that?
If a particular mavlink command is unsupported, does PX4 send an error about command not being supported? At least with APM i get this error thrown in the MAVSDK logs like this:
[09:59:06|Warn ] command unsupported (528). (mavlink_command_sender.cpp:198)
Maybe this could be used as deciding criteria on using fallback?
The "UNSUPPORTED" error code does exist, but as far as I can tell, it is not mandatory (at least I have seen many places where it is not used). So if you receive "UNSUPPORTED", you can conclude that it is not supported, but if you don't receive anything, you cannot make the difference between a timeout and an unsupported message (at least in PX4).
That essentially makes "UNSUPPORTED" useless for us, unfortunately.
My concern with the fallback is this: what if the drone supports the new REQUEST_MESSAGE message, but the request is lost? MAVSDK will time out, not because it is unsupported, but because of an actual timeout. If MAVSDK now falls back to the old message and the drone does not support it, it will timeout again, this time because it is not supported. How do we handle that?
We try harder :grin:, so wit 5 retries, we basically try both commands 3 times, hopefully one goes through.
That essentially makes "UNSUPPORTED" useless for us, unfortunately.
I disagree. We can test against PX4 v1.10 for instance and check, and if we get UNSUPPORTED, then we know, go to fallback. If we time out, then we try the fallback anyway, and then try the normal one again. UNSUPPORTED is nice, because it means we don't have to wait for the timeout, so we are much quicker.
I disagree. We can test against PX4 v1.10 for instance and check, and if we get UNSUPPORTED, then we know, go to fallback.
But is there a PX4 version that responds UNSUPPORTED to the newer REQUEST_MESSAGE instead of implementing it? And is it really worth removing the older message from the PX4 codebase, just for the sake of it?
I don't know, my feeling is that PX4 should support both (I don't see what we gain by removing the old one), and then MAVSDK has essentially two possibilities:
- Support both with a more or less advanced fallback mechanism.
- Just use the old one until it does not have to support an older version of PX4, at which point it can switch to the new one.
I would prefer the 1st option since that would allow us to be not waiting for the EOL of PX4 1.10 for which we probably don't have a timeline yet.
What about keeping the older message as the default one and only try the newer message if we receive UNSUPPORTED response from the autopilot.
PX4 1.10, 1.11 and 1.12 still support the older message so the fallback would not be triggered.
APM 4.1.2 does not support the older message so will receive UNSUPPORTED response which will trigger the fallback and the newer message would be used for further requests.
This would allow us to slowly transition to the newer message. What do you think?
APM 4.1.2 does not support the older message
Oh, I had not realized that. Then your suggestion makes complete sense to me:
What about keeping the older message as the default one and only try the newer message if we receive UNSUPPORTED response from the autopilot?
What about keeping the older message as the default one and only try the newer message if we receive UNSUPPORTED response from the autopilot?
What about the other way round? Try the newer message and fallback to the old one for PX4 v1.10? PX4 v1.10 is basically EOL but that's just my opinion. There is no support window or dates as far as I know, it's up to us MAVSDK maintainers what we choose to support. All I know is that there are still plenty forks around based on v1.10.
Try the newer message and fallback to the old one for PX4 v1.10?
My understanding is that PX4 v1.10 does not send UNSUPPORTED for the REQUEST_MESSAGE, does it? If it doesn't, then we have to rely on timeouts, which requires weird heuristics like "let's try this one 2 times, then the other one 2 times, then go back to the first one, then wait 10 seconds (who knows?), and during that time monitor the other mavlink messages to guess if it's more likely to be a timeout or an unsupported message". Of course most of the time it will work, so we will end up having heuristics that we believe work "fairly well" but without really having a clue how often it can result in problems, with convoluted code that will stay around for who knows how long, all that just to use a preferred message with no real benefit (because PX4 1.12 supports both, rights?).
Using the old one first seems less ambiguous: PX4 v1.10+ supports it, and when support is dropped in newer versions of PX4 we can make PX4 return "UNSUPPORTED", just like Ardupilot. That's still not perfect w.r.t. the mavlink specs (that don't enforce the use of UNSUPPORTED), but at least that seems reliable for the versions of PX4 and Ardupilot we want to support.
Using the old one first seems less ambiguous: PX4 v1.10+ supports it, and when support is dropped in newer versions of PX4 we can make PX4 return "UNSUPPORTED", just like Ardupilot. That's still not perfect w.r.t. the mavlink specs (that don't enforce the use of UNSUPPORTED), but at least that seems reliable for the versions of PX4 and Ardupilot we want to support.
I agree. Even though its not the perfect way to adopt the new message, its much easier to implement and maintain.
However it is also not easy to make sure that any future versions of PX4 would implement UNSUPPORTED for the old message so we might end up coming back to this issue again. Could it be an idea to decide on a release number (say v1.5.0) for MAVSDK which just supports the new message and not the old message? Such a raodmap would also better prepare such transitions of deprecated standard messages. PX4 v1.10.2 was released almost 26 months before and i can imagine only a very small number of users could be still using it at the moment.
PX4 v1.10.2 was released almost 26 months before and i can imagine only a very small number of users could be still using it at the moment.
For single users, I agree. However, many companies just for off PX4 and then don't necessarily merge back, or not often.
with convoluted code that will stay around for who knows how long, all that just to use a preferred message with no real benefit
The thing is that it's not just about this one. We have the same problem with camera commands.
To me, if we don't use the "right"/"standard" thing first, then we don't push in the right direction. For instance, if someone looks at the code, or develops by looking at what is sent, they will by default implement the old way, so then you can never get out of the rut.
My understanding is that PX4 v1.10 does not send UNSUPPORTED for the REQUEST_MESSAGE, does it?
Let's check that.
WARNING: strong opinion follows :laughing:
For instance, if someone looks at the code, or develops by looking at what is sent, they will by default implement the old way, so then you can never get out of the rut.
The typical way I expect deprecation to work is that for some update of the library, a function is marked as "deprecated" (meaning "it will disappear and break your build some day in the future"). During the transition period, the library still supports the deprecated way, and obviously supports the new way. After it has been decided that the transition period has been long enough (for some projects it takes many years), then the deprecated function is just removed. Code depending on the deprecated function just breaks.
In this particular case, that ship has sailed, because the message has been deprecated for many years already and Ardupilot has chosen to drop it. But luckily Ardupilot answers UNSUPPORTED (the fact that UNSUPPORTED is not mandatory is a fundamental flaw in MAVLink anyway, IMO), so we seem to have a chance to still support Ardupilot and PX4 by using the old message first, and falling back to the new message when UNSUPPORTED is sent. Some day, PX4 can decide to drop support for the old message (ideally by sending UNSUPPORTED, too), and at this point we can just drop the old message in MAVSDK. At this point, we break:
- The old systems that never got updated and still rely on the old message. But that's fine because we decided to drop them, and they had years to update. They can choose to update or just stay with an older MAVSDK, that's not our problem.
- The new systems that were implemented using messages that have been marked as deprecated for years. Those are screwed and need to update or stay with an older MAVSDK, and again that's not our problem.
But trying to "show by example" to developers who don't read the specs by implementing a solution that sometimes fails (though maybe rarely, who knows) by design is not a good idea IMO: I'd rather have those who "do it wrong" be screwed and have to put resources into updating their system than punishing those who "do it right" by having a flaky, convoluted detection system that may fail in some cases.
Those messages have been deprecated for as long as I have known PX4 (that's 5 years now), and I don't think I know a single drone from that era that is still being produced and sold and at the same time did not have a chance to update. So I would be fine dropping the old messages. Still I'm fine supporting both if we find a way, but I will always vote against heuristics that try to guess whether it was an actual timeout or if that's just an unsupported message :innocent:.
Allright, i did a rough implementation of the "try old message first and fallback to new message" idea.