Creating UDP packet idiomatic

hi all,

I'm trying to create a UDP packet of [u8;36] with a fixed format ("DATA" + u8 + 0 0 0 + 8x f32 floats).

I've found a way by creating a vector and using push/append; but as i'm learning rust im trying to see if htere is a more ideomatic way. can you please help me to see if this makes sense, and how you would do it?

thanks so much,
Scott

async fn send_data_command(socket: UdpSocket, index: u8, values_floats: Vec<f32>) -> Result<()> {

    // create a udp packet [u8,32] of bytes
    // structure should be
    // "DATA" + index(byte) + 0 0 0 (3x zero byes) + 8 x floats (as bytes)
    // if not enough floats, make sure we add zeros

    if values_floats.len() > 8 {
        return Err(anyhow!("Error sending data command: cannot package more than 8 floats"));
    }

    let mut packet: Vec<u8> = Vec::with_capacity(36);
    b"DATA".map(|b| packet.push(b));
    packet.push(index);
    packet.extend(vec![0_u8, 0_u8, 0_u8]); // add the zero bytes

    for (f) in values_floats {
        f.to_le_bytes().map(|b| packet.push(b));
    }
    packet.resize(36, 0_u8); //make sure we always have 36 bytes, e.g. by adding zeros if there were be less than 8 floats

    let len = socket.send(&packet[..]).await?;

    if len == 36 {
        Ok(())
    } else {
        return Err(anyhow!("Error sending data command: more or less than 36 bytes send"));
    }
}

Since you are working with fixed-size packets, there isn't any need for any dynamic allocation. Especially not when you are working with a literal. Why are you even doing the vec![0, 0, 0] thing? Extend works with any iterable, you could have used an array just as well.

The abuse of array.map is very weird, too. Usually, mapping is assumed to be lazy (because it's done on iterators); until seeing the concrete types, I'd have assumed you had a laziness bug there.

Additionally, it would be better to decouple the serialization logic from the sending. There's no reason why this should be coupled with a socket per se. If you perform this decoupling, then you can remove the async-ness, too.

You also miscalculated the packet length. 4 + 4 + (8 * 4) = 40, not 36.

Altogether, a much better impl would be:

fn create_data_packet(index: u8, values: &[f32]) -> anyhow::Result<[u8; 40]> {
    anyhow::ensure!(
        values.len() <= 8,
        "Error sending data command: cannot package more than 8 floats",
    );

    let mut packet = [0_u8; 40];
    let index_u32 = u32::from(index);
    
    packet[0..4].copy_from_slice(b"DATA");
    packet[4..8].copy_from_slice(&index_u32.to_le_bytes());
    
    for (chunk, &value) in packet[8..].chunks_mut(4).zip(values) {
        chunk.copy_from_slice(&value.to_le_bytes());
    }

    Ok(packet)
}
2 Likes

Thank you - i learned a lot from this. i have tried packet[0..4] = b"DATA" before but got a compiler warning on that packet[0..4] size is not known at compile time. why does .copy_from_slice() does work here?

Why wouldn't it work? It's just a function call.

Assignment (ie., a move generated by the compiler) is a compile-time concept that requires that the size be known statically to the compiler. That's special. Calling a function on a slice, there isn't anything special about that.

copy_from_slice() is just a regular run-time function that is implemented using a loop at the lowest level, and it essentially performs element-by-element assignment. It's the same as

for (place, value) in packet[0..4].iter_mut().zip(*b"DATA") {
    *place = value;
}

which would have worked just as well (except for the handling of mismatched lengths, and the fact that you shouldn't spell that out manually every time if there's an equivalent, trivial-to-use function in std).

Thanks again. and then why is the size not know of packet[0..4]? We already defined its a [u8, 40], and why doesnt the compiler know for sure that packet[0..4] is a [u8;4]?

No. Indexing can be done with arbitrary values at runtime, hence it's simply not possible to base the type upon the value of the index. <[T] as Index<Range<usize>>::index() always returns [T], as evidenced by the pertinent associated type of the trait.

1 Like

The compiler might be certain, but what matters is that the compiler must always follow the type system's rules. The type system has no special cases for expressions that happen to represent ranges of constant length. So, the value of packet[some_range] has type [u8], not [u8; 4], because some_range could have any length.

If there were a sort of RangeWithFixedLength type then you could write

packet[RangeWithFixedLength::<4> { start: 0 }] = *b"DATA";

and both sides of the assignment would be arrays, but there currently is no such type. You can define one, if you want:[1]

struct RangeWithFixedLength<const LEN: usize> {
    start: usize
}

impl<const LEN: usize, T> std::ops::Index<RangeWithFixedLength<LEN>> for Vec<T> {
    type Output = [T; LEN];
    fn index(&self, range: RangeWithFixedLength<LEN>) -> &Self::Output { 
        (&self[range.start..][..LEN]).try_into().unwrap()
    }
}

impl<const LEN: usize, T> std::ops::IndexMut<RangeWithFixedLength<LEN>> for Vec<T> {
    fn index_mut(&mut self, range: RangeWithFixedLength<LEN>) -> &mut Self::Output { 
        (&mut self[range.start..][..LEN]).try_into().unwrap()
    }
}

fn main() {
    let mut packet = vec![0; 8];
    packet[RangeWithFixedLength::<4> { start: 0 }] = *b"DATA";
    dbg!(packet);
}

(Perhaps there's already a library that offers this; I haven't looked.)


  1. though ideally it would work through SliceIndex implementations (currently unstable) ↩ī¸Ž

3 Likes

Minor correction: it should be range.start..range.start + LEN :slight_smile:

Oops! Fixed. (That's what I get for only testing with start: 0.)

1 Like

Thanks a lot for your detailed answers, really helps me to learn!

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.