Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement recv_raw for Connection wrappers into mavlink-core #286

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

roby2014
Copy link

@roby2014 roby2014 commented Sep 24, 2024

(depends on #284)

So, my goal is to have some sort of way to fetch raw messages from the connection wrappers (AsyncMavConnection, MavConnection) directly, without having to do the extra-step of providing a type and converting between and vice-versa.

At the moment, recv will always use the generic M.

Another thing I was planning was to somehow allow connections without knowing the message type M, but this looks a bit harder to implement (and probably impossible because dialect must be known at compile-time?).

I've already implemented for AsyncMavConnection and its protocols (udp, tcp, direct-serial, file):

async fn recv_raw(&self) -> Result<MAVLinkRawMessage, crate::error::MessageReadError>;

I've also implemented a small example (mavlink-dump-raw-async), which already shows how this can be used with async.

I want to ask if this is a good idea and if possible, to open discussion on this topic here.

@roby2014 roby2014 force-pushed the recv_raw branch 2 times, most recently from 0cee34e to 4467ac4 Compare September 24, 2024 16:09
@roby2014
Copy link
Author

roby2014 commented Sep 25, 2024

Well 1 possible idea I have is to implement raw connectors, e.g: AsyncMavRawConnection and MavRawConnection instead of adding recv_raw (or keep recv_raw just for API design). This would not care about dialect and just tries to parse a MAVLink message. Obviously, the message check (crc calculation, deserialization, etc), would have to be done manually by the user).

@patrickelectric (and other maintainers) what do you think?

@pv42
Copy link
Contributor

pv42 commented Sep 25, 2024

So I had the idea of doing something like that:

pub trait MavRawConnection {
    fn recv_raw(&self) -> Result<MAVLinkVXMessageRaw>
    ...
}

and then have

pub trait MavConnection<M: Message> : MavRawConnection  {
    fn recv(&self) -> Result<(MavHeader, M), crate::error::MessageReadError> {
        M::parse(self.recv_raw())
    }
   ...
}

The issue is that MAVLinkVXMessageRaw does not exist and it probably would have to be a trait object.

@roby2014
Copy link
Author

roby2014 commented Sep 26, 2024

So I had the idea of doing something like that:

pub trait MavRawConnection {
    fn recv_raw(&self) -> Result<MAVLinkVXMessageRaw>
    ...
}

and then have

pub trait MavConnection<M: Message> : MavRawConnection  {
    fn recv(&self) -> Result<(MavHeader, M), crate::error::MessageReadError> {
        M::parse(self.recv_raw())
    }
   ...
}

The issue is that MAVLinkVXMessageRaw does not exist and it probably would have to be a trait object.

I still think you'll need a dialect that way since functions like read_v1_raw_message_async, etc... Depend on the generic M

P.S: My title might be misleading. My goal is to have Connection wrappers that do not depend on a dialect so you can just receive RAW data

@joaoantoniocardoso
Copy link
Contributor

So I had the idea of doing something like that:

pub trait MavRawConnection {
    fn recv_raw(&self) -> Result<MAVLinkVXMessageRaw>
    ...
}

and then have

pub trait MavConnection<M: Message> : MavRawConnection  {
    fn recv(&self) -> Result<(MavHeader, M), crate::error::MessageReadError> {
        M::parse(self.recv_raw())
    }
   ...
}

The issue is that MAVLinkVXMessageRaw does not exist and it probably would have to be a trait object.

I still think you'll need a dialect that way since functions like read_v1_raw_message_async, etc... Depend on the generic M

P.S: My title might be misleading. My goal is to have Connection wrappers that do not depend on a dialect so you can just receive RAW data

I think that'd only be possible if we don't check the CRC. For that, we could surely extract the part of the recv raw functions that deal with the message before the CRC, and expose that to the public lib API.

@roby2014
Copy link
Author

I think it would be great to have some sort of API to just receive/send mavlink messages, even if they are not CRC-validated or from a specific dialect. When the goal is to just "route" messages, I dont want to care about a dialect or version.

@joaoantoniocardoso
Copy link
Contributor

joaoantoniocardoso commented Oct 10, 2024

I think it would be great to have some sort of API to just receive/send mavlink messages, even if they are not CRC-validated or from a specific dialect. When the goal is to just "route" messages, I dont want to care about a dialect or version.

I think the main reason the CRC (and thus the knowledge of the message dialect) is necessary for us to create a message is because the protocol relies on a single magic byte to define the start of the message, and this byte can also exist within the rest of the packed, so unless we have another reliable algorithm to identify a message, if we follow the current parser idea, it would possibly discard valid messages while wrongly identifying messages.

One idea would be not to remove all bytes from each identified message from the buffer, but to only remove the bytes until the next magic byte, which would result in sending both invalid and valid messages, increasing bandwidth (no idea of how much). This assumes we can pass the responsibility of validating the messages entirely to the clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants