Hi, I would like to start a discussion about improving the MAVLink API. I frequently come across situations which are (at least with my knowledge) not easy to solve using this library.
Subscribing to a specific message
Often I want to receive a certain message somewhere in the code. This is a key feature in order to not have this one huge match statement where each and every message is processed. For this, it would be very nice to be able to subscribe to a message. I imagine an API like this:
impl MavConnectionHandler {
subscribe(&self, message: MavMessage)->std::sync::mpsc::Receiver<MavMessage>
subscribe_once(&self, message: MavMessage)->std::sync::mpsc::Receiver<MavMessage>
// call frequently to do the housework
spin(&self);
}Obviously this would require that someone is calling spin regularly. Another less no_std friendly approach would be
impl MavConnectionHandler {
/// spawns the MavConnectionHandler which takes care of handling subscriptions
new<M>(conn: Box<dyn MavConnection<M> + Sync + Send>)->(JoinHandle<()>, Arc<dyn MavConnectionHandler>)
subscribe(&self, message: MavMessage)->std::sync::mpsc::Receiver<MavMessage>
subscribe_once(&self, message: MavMessage)->std::sync::mpsc::Receiver<MavMessage>
}This however bears a big problem. AFAIK there is no way to talk about the type of a mavlink message without creating an instance of it. This brings us to my next pain point:
Allow a message type to be specified, without an Instance of the message
Well there is the message id, but that is by no means type safe (and IMO nothing we should rely on in an API). And yes, there is std::mem::discriminant, however that does require one to construct a value. If I want to name the type of MavMessage, the best option so far seems to be this:
let message_type = discriminant(&MavMessage::PARAM_VALUE(Default::default()));
Allow conversion of a MavMessage to one specific ..._DATA struct
I'm currently in the making of a nicer async API which operates on top of the MavConnection trait. AFAIK it is not possible to convert a MavMessage into on specific data structure. This is not an issue if one doesn't know what message the next recv() will yield. However, in my implementation I do know that. Consider the following example (which continues the last snippet):
/// currently. The thing which bothers me the most is that there is
/// no way of avoiding yet another level of indentation.
if let MavMessage::PARAM_VALUE(data) = wrappe_conn.request(&message_type).await {
}
/// maybe in the future? ^^
/// Notice that the request method can do the unwrapping internally,
/// as we already know that we will only receive the correct messages
let data = wrapped_conn.request(&message_type).await;If I understand correctly, the issue is simple: There is no way of extracting data out of an enum instance without mentioning the variant which contains said data. Maybe its possible to add a method like this to MavMessage:
/// Tries to convert an instance of the `MavMessage` enum into one of the `_DATA` structs
impl MavMessage {
into_inner(self)->Option<T>
}
Maybe it's also possible to exploit Any for the polymorphism needed in a generic receive function. A rough draft could be like this: For sure runtime reflections do work for this kind of stuff, but that would be a huge break in the API which will likely not pay off UX wise.
pub enum MavMessageType {
HEARTBEAT,
SYS_STATUS,
SYSTEM_TIME,
//...
}
impl MavMessage {
type(&self)->MavMessageType;
as_message<T>(&self)->T{
self.downcast_ref::<T>().clone()
}
// serialize, deserialize, ...
}Summary
I'm sure there is more room for improvement, but these are the points which have bitten me the most. My proposals are in no way ready and probably will even fail compilation, they solely are meant to be early drafts. Speaking of drafts, out of the need for my use cases I already implemented an async API wrapper for MavConnection in the last couple of days. I would be very happy if it eventually find's it's way back in the this crate, behind a feature gate which is disabled per default maybe? Currently you can only have a look into it here. It isn't feature complete, but it already works!
What do you think?
Is there a general desire to further refine the API?
Edit:
Some of my statements turned out to be questionable, thus I corrected them. Furthermore I added some more information and examples.
Edit 2:
I added some more information and again refine the examples.
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent