Add MessageData trait to allow serialize ***_DATA structs into into MAVLinkV[1|2]MessageRaw

[qwerty19106]

Problem

Microcontrollrs firmware consumes very many FLASH (.text) bytes.

Below is a piece of real code which parses Mavlink message from MAVLinkV2MessageRaw. The MAVLinkV2MessageRaw previously received by using UART DMA.

Optimization settings:

[profile.dev]
lto = "thin"
debug = true
opt-level = 1

cargo +stable build --target thumbv7em-none-eabihf

Classic MavMessage::parse:

fn on_rx_message(
    raw_message: &mut MAVLinkV2MessageRaw,
    local_position_ned_data: &mut LOCAL_POSITION_NED_DATA,
    attitude_data: &mut ATTITUDE_DATA,
    term: &mut impl rtic::Mutex<T = TerminalChannel>,
) {
    use mavlink::Message;
    let message = match MavMessage::parse(
        MavlinkVersion::V2,
        raw_message.message_id(),
        raw_message.payload(),
    ) {
        Ok(msg) => msg,
        Err(err) => {
            panic!(
                "[px4::rx_message_task_impl] error on parsing msgid={}: {}\n",
                raw_message.message_id(),
                err
            );
        }
    };

    match message {
        // Сохраняем локальную оценку полетника
        MavMessage::LOCAL_POSITION_NED(data) => {
            *local_position_ned_data = data;
        }
        MavMessage::ATTITUDE(data) => {
            *attitude_data = data;
        }
        _ => {}
    }
}

rust-size central_controller
   text	   data	    bss	    dec	    hex	filename
 145480	    116	   5916	 151512	  24fd8	central_controller

readelf -Ws central_controller | grep FUNC | grep -i mavlink
   971: 0801ccb3    40 FUNC    LOCAL  HIDDEN     2 _ZN7mavlink5bytes5Bytes9remaining17hb4526a7982ef297aE.llvm.9430813786115405808
  1399: 0801cae3   202 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink23MAVLinkV2RawAsyncReader7on_byte17hffe9e03806712d8fE
  1400: 0801a69d  9286 FUNC    GLOBAL DEFAULT    2 _ZN64_$LT$mavlink..common..MavMessage$u20$as$u20$mavlink..Message$GT$5parse17ha725efeb88dcf02cE
  1401: 0801cbf1   194 FUNC    GLOBAL DEFAULT    2 _ZN66_$LT$mavlink..error..ParserError$u20$as$u20$core..fmt..Display$GT$3fmt17h9ebfbdf4e0f195e0E
  1463: 0800d2d1   100 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink5bytes5Bytes9get_array17h86a2ac969767ec95E
  1464: 0801ccdb   186 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink5bytes5Bytes9get_bytes17h9d6c0f982c03fba0E
  1466: 0800d335    92 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink5bytes5Bytes9get_array17he4f71f76bca764afE
  1467: 0800ddeb    88 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common16LOGGING_ACK_DATA5deser17hb452359d46dbfeddE
...
  1671: 0801a45f    94 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common32CHANGE_OPERATOR_CONTROL_ACK_DATA5deser17h892792ef790ee2f5E
  1672: 0801a4bd   180 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common32DATA_TRANSMISSION_HANDSHAKE_DATA5deser17h011d2ea63be5785cE
  1673: 0801a571   206 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common20NAMED_VALUE_INT_DATA5deser17hc18fab53ba8f5d9fE
  1674: 0801a63f    92 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common10DEBUG_DATA5deser17h8366233b06fb4d71E

Use ***_DATA::deser:

fn on_rx_message(
    raw_message: &mut MAVLinkV2MessageRaw,
    local_position_ned_data: &mut LOCAL_POSITION_NED_DATA,
    attitude_data: &mut ATTITUDE_DATA,
    term: &mut impl rtic::Mutex<T = TerminalChannel>,
) {
    // Парсим
    const LOCAL_POSITION_NED_ID: u32 = 32;
    const ATTITUDE_ID: u32 = 30;

    match raw_message.message_id() {
        LOCAL_POSITION_NED_ID => {
            match LOCAL_POSITION_NED_DATA::deser(MavlinkVersion::V2, raw_message.payload()) {
                Ok(data) => *local_position_ned_data = data,
                Err(err) => {
                    panic!(
                        "[px4::rx_message_task_impl] error on parsing msgid={}: {}\n",
                        raw_message.message_id(),
                        err
                    );
                }
            }
        }
        ATTITUDE_ID => {
            match ATTITUDE_DATA::deser(MavlinkVersion::V2, raw_message.payload()) {
                Ok(data) => {
                    *attitude_data = data;
                }
                Err(err) => {
                    panic!(
                        "[px4::rx_message_task_impl] error on parsing msgid={}: {}\n",
                        raw_message.message_id(),
                        err
                    );
                }
            }
        }
        _ => {}
    }
}

rust-size central_controller
   text	   data	    bss	    dec	    hex	filename
  79636	    116	   5916	  85668	  14ea4	central_controller

readelf -Ws central_controller | grep FUNC | grep -i mavlink
   631: 0800d5cf    40 FUNC    LOCAL  HIDDEN     2 _ZN7mavlink5bytes5Bytes9remaining17hb4526a7982ef297aE.llvm.9430813786115405808
  1043: 0800d369   150 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common23LOCAL_POSITION_NED_DATA5deser17h68899f00b39cc897E
  1044: 0800d3ff   202 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink23MAVLinkV2RawAsyncReader7on_byte17hffe9e03806712d8fE
  1045: 0800d2d3   150 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink6common13ATTITUDE_DATA5deser17h8a14d6e1ba98e6baE
  1046: 0800d50d   194 FUNC    GLOBAL DEFAULT    2 _ZN66_$LT$mavlink..error..ParserError$u20$as$u20$core..fmt..Display$GT$3fmt17h9ebfbdf4e0f195e0E
  1105: 0800d277    92 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink5bytes5Bytes9get_array17he4f71f76bca764afE
  1106: 0800d5f7   186 FUNC    GLOBAL DEFAULT    2 _ZN7mavlink5bytes5Bytes9get_bytes17h9d6c0f982c03fba0E

If I add message serialization then

rust-size central_controller
   text	   data	    bss	    dec	    hex	filename
 217988	    116	   5916	 224020	  36b14	central_controller

Note that I can not use ***_DATA for serialization in current API.

P.S. Typical FLASH size for microcontrollers (Blue Pill STM32F103C8T6) is 64 KB.
My code with receiving and transmitting Mavlink messages consumes 218KB (Mavlink ***_DATA::ser and ***_DATA::deser consume ~135KB).

Solution

I propose to add this API:

pub trait MessageData: Sized {
    type Message: Message;

    const ID: u32;
    const NAME: &'static str;
    const EXTRA_CRC: u8;
    const ENCODED_LEN: usize;

    fn ser(&self, version: MavlinkVersion, payload: &mut [u8]) -> usize;
    fn deser(version: MavlinkVersion, payload: &[u8]) -> Result<Self, ParserError>;
}

impl MAVLinkV1MessageRaw {
    ...
    pub fn serialize_message_data<D: MessageData>(&mut self, header: MavHeader, message_data: &D);
}

impl MAVLinkV2MessageRaw {
    ...
    pub fn serialize_message_data<D: MessageData>(&mut self, header: MavHeader, message_data: &D);
}

It only new API, no breaking changes (with little exception, see questions).
It allows serialize and parse ***_DATA structs without using Message (for example mavlink::common::MavMessage). Thus compiler will be generate only code for limited series of ***_DATA structs.

Unresolved questions

• Do rename ***_DATA:: ser to serialize or serialize_payload?
• Do rename MessageData::ENCODED_LEN to MessageData::SERIALIZED_LEN?
• I remove MAVLinkV[1|2]MessageRaw::calculate_crc as unnecessary. This is right?

[qwerty19106]

It is ready for review.

[patrickelectric]

@qwerty19106 I'll try to put a better look on it next week.

[qwerty19106]

As variant, I can place associated constants into MessageMeta struct, as proposed #156 by @GabrielDertoni.

pub struct MessageMeta {
    pub id: u32,
    pub name: &'static str,
    pub extra_crc: u8,
    pub encoded_len: u8,
}

pub trait MessageData: Sized {
    type Message: Message;

    const META: MessageMeta;

    fn ser(&self, version: MavlinkVersion, payload: &mut [u8]) -> usize;
    fn deser(version: MavlinkVersion, payload: &[u8]) -> Result<Self, ParserError>;
}

But I don't see the need for MessageMeta::default field.

[GabrielDertoni]

I agree that it is best to leave out MessageMeta::default. Instead, we should add

pub trait MessageData: Sized {
    const DEFAULT: Self::Message;
    /*...*/
}

this default constant is already going to be used by #160, even better to put it on the trait! Then we don't have to wait for const Default to become a thing and also don't have to deal with MessageMeta having a big default field with a generic.

I am strongly in favor of MessageMeta since you can strictly do more stuff with a struct than you can with a trait in this case.

[qwerty19106]

@patrickelectric Сould you take the time to review this?

[patrickelectric]

Hi @qwerty19106, Sorry, I did allocate some time to the project, but was mostly to fix the doc build failing in docs.rs.
I'll put time to review this before the end of the week.

[qwerty19106]

Rebased