Posts
4497
Following
316
Followers
475
Linux kernel hacker and maintainer etc.

OpenPGP: 3AB05486C7752FE1

My crate has the implementation now in 933 lines and not a lot of dependencies. It starts to be in a shape that is not a huge stretch to make it fully no_std even. Not the first priority tho but entirely possible.

After I have a similar state function for the receiver I’ll look at futures crate, which claims to be no_std compatible. I.e. idea would be to get yield once per iteration.

I’m not interested in async stuff for multi-threading. I’m interested single-threaded sequenced type of stuff, co-operative multitasking…

0
0
0

Jarkko Sakkinen

Edited 1 year ago

This should clarify how it is organized:

/// Map the previous frame type of the sender and incoming frame type of the
/// receiver to the next packet to be sent.
///
/// NOTE: ZRINIT is used here as a wait state, as the sender does not use it for
/// other purposes. Other than tat the states map to the packets that the sender
/// sends next.
const fn next_state(sender: Option<Type>, receiver: Type) -> Option<Type> {
    match (sender, receiver) {
        (None, Type::ZRINIT) => Some(Type::ZFILE),
        (None, _) => Some(Type::ZRQINIT),
        (Some(Type::ZRQINIT), Type::ZRINIT) => Some(Type::ZFILE),
        (Some(Type::ZFILE), Type::ZRPOS) => Some(Type::ZDATA),
        (Some(Type::ZFILE), Type::ZRINIT) => Some(Type::ZRINIT),
        (Some(Type::ZRINIT), Type::ZRPOS) => Some(Type::ZDATA),
        (Some(Type::ZDATA), Type::ZACK) => Some(Type::ZDATA),
        (Some(Type::ZDATA), Type::ZRPOS) => Some(Type::ZDATA),
        (Some(Type::ZDATA), Type::ZRINIT) => Some(Type::ZFIN),
        (Some(Type::ZFIN), Type::ZFIN) => None,
        (_, _) => None,
    }
}

No reason to add complexity with fsmentry. This is a nice clean const function.

1
0
0

Jarkko Sakkinen

#zmodem2 is a nice history lesson to develop:

    style: cleanup and fix cosmetic stuff

    1. This inherits from original `zmodem` crate: "ZLDE" is in-fact ZDLE,
       an acronym of "ZMODEM Data Link Escape" character.
    2. Fine-tune use statements.

    Link: https://wiki.synchro.net/ref:zmodem
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@iki.fi>

That link in the commit message is a great source of information on #zmodem.

#rustlang

0
0
1

After looking that for some time my state diagram for “sz” is simply this:

fsmentry::dsl! {
    #[derive(Debug)]
    pub Mode {
        WaitingForReceiver -> Starting;
        Starting -> Sending;
        Starting -> WaitingForPosition;
        Sending -> WaitingForPosition;
        WaitingForPosition -> Sending;
        Sending -> Ending;
    }
}

This is scales exactly of the normal serial transfer. ZCHALLENGE/ZCOMPL, “corporate zmodem” (yes it was a real concept back in the say) and running commands by the request of the sender are definitely out of scope. So pretty clean and understandable in the end when you carve deep enough…

1
0
0
Moving file through the serial post is tricky, not because of protocols but because mostly it is based on launching rz and sz. This indirection makes impossible to implement an user experience that people used to transferring file such as showing progress, canceling the transfer and such and so forth. I'm fed up with this difficulty so that's where the motivation comes from.
1
0
0

Jarkko Sakkinen

Edited 1 year ago
I was going to use `fsmentry` crate for the state machine but then i ended up to something way more simpler: https://github.com/jarkkojs/zmodem2/commit/86a4af491448cf30a718a43dd1e5998b5b1863c6
1
0
0

Jarkko Sakkinen

next step in my zmodem2 crate is to make send and receive asynchronous so that progress can be tracked. then i’m ready to integrate this to my other small project, tior. #zmodem #rustlang

1
1
3
@Dr_Emann Yeah, I was not doing that because I can but because I had to :-) Good lesson anyway with the type system, learned a lot even though the snippet is only few lines.
0
0
0
@Dr_Emann This was a good tip. I want to refactor this as a zero-copy thing overally but doing these primitives takes time because you really have to think through, I was going to add cast from bytes next. Saves time!
1
0
0

Jarkko Sakkinen

converted legacy hard coded test cases for frame to rstest in the #zmodem 2 crate:

#[cfg(test)]
mod tests {
    use crate::frame::*;

    #[rstest::rstest]
    #[case(Encoding::ZBIN, Type::ZRQINIT, &[ZPAD, ZLDE, Encoding::ZBIN as u8, 0, 0, 0, 0, 0, 0, 0])]
    #[case(Encoding::ZBIN32, Type::ZRQINIT, &[ZPAD, ZLDE, Encoding::ZBIN32 as u8, 0, 0, 0, 0, 0, 29, 247, 34, 198])]
    fn test_header(
        #[case] encoding: Encoding,
        #[case] frame_type: Type,
        #[case] expected: &[u8]
    ) {
        let header = Header::new(encoding, frame_type);

        let mut packet = vec![];
        new_frame(&header, &mut packet);

        assert_eq!(packet, expected);
    }
    #[rstest::rstest]
    #[case(Encoding::ZBIN, Type::ZRQINIT, &[1, 1, 1, 1], &[ZPAD, ZLDE, Encoding::ZBIN as u8, 0, 1, 1, 1, 1, 98, 148])]
    #[case(Encoding::ZHEX, Type::ZRQINIT, &[1, 1, 1, 1], &[ZPAD, ZPAD, ZLDE, Encoding::ZHEX as u8, b'0', b'0', b'0', b'1', b'0', b'1', b'0', b'1', b'0', b'1', 54, 50, 57, 52, b'\r', b'\n', XON])]
    fn test_header_with_flags(
        #[case] encoding: Encoding,
        #[case] frame_type: Type,
        #[case] flags: &[u8; 4],
        #[case] expected: &[u8]
    ) {
        let header = Header::new(encoding, frame_type).flags(flags);

        let mut packet = vec![];
        new_frame(&header, &mut packet);

        assert_eq!(packet, expected);
    }
}

Should be easier to refactor the legacy code now as there is less raw code that might be affected in tests.

#rustlang #rstest

0
0
1

Jarkko Sakkinen

Edited 1 year ago

I hope I got this right (safety-proprty), i.e. so that references are enforced to have equal life-time:

impl<'a> From<&'a Header> for &'a [u8] {
    fn from(value: &Header) -> Self {
        // SAFETY: out-of-boundary is not possible, given that the size constraint
        // exists in the struct definition. The lifetime parameter links the lifetime
        // of the header reference to the slice.
        unsafe { from_raw_parts((value as *const Header) as *const u8, size_of::<Header>()) }
    }
}

#rustlang

1
0
0

I first used transmute but that causes extra struct copy.

0
0
0

Jarkko Sakkinen

Edited 1 year ago

great implemented transmutable (with u8) enum for the frame type of the #ZMODEM transfer protocol header: https://github.com/jarkkojs/zmodem2/commit/c76316c2ecae097be03506e8ce19d61287e6468a

I can use this as a reference model for refactoring rest of the code base.

Next, I’ll replace encoding: u8 with a similar enum Encoding .

#rustlang

0
0
0

Jarkko Sakkinen

Known good crates for no_std Rust:

  1. spinning (used e.g. in #Enarx)
  2. heapless

Will be updated over time.

#note #rustlang

1
0
1

Jarkko Sakkinen

Edited 1 year ago

Interestingly enough for CRC16 byte array is construct reverse, i.e. big-endian so the final refurnished functions that pass the tests are:

use crc32::{Crc, CRC_16_XMODEM, CRC_32_ISO_HDLC};

const CRC16: Crc<u16> = Crc::<u16>::new(&CRC_16_XMODEM);
const CRC32: Crc<u32> = Crc::<u32>::new(&CRC_32_ISO_HDLC);

pub fn get_crc16(buf: &[u8], maybe_zcrc: Option<u8>) -> [u8; 2] {
    let mut digest = CRC16.digest();

    digest.update(buf);

    if let Some(zcrc) = maybe_zcrc {
        digest.update(&[zcrc]);
    }

    digest.finalize().to_be_bytes()
    // [(crc >> 8) as u8, (crc & 0xff) as u8]
}

pub fn get_crc32(buf: &[u8], maybe_zcrc: Option<u8>) -> [u8; 4] {
    let mut digest = CRC32.digest();

    digest.update(buf);

    if let Some(zcrc) = maybe_zcrc {
        digest.update(&[zcrc]);
    }

    // Assuming little-endian byte order, given that ZMODEM used to work on
    // VAX, which was a little-endian computer architecture:
    digest.finalize().to_le_bytes()
}

The manual conversion matches this so I guess this is right. No idea how it ended up like this in the history. Hooray, now I an finally open-code these convoluting functions by exporting CRC16 and CRC32 and using them directly in the call sites, mostly by just calling .checksum() (there was only one call site where you really need to use .update() and .finalize().

0
0
0

Jarkko Sakkinen

Edited 1 year ago

For 16-bit it was heck a lot of easier to pick the right one. There was CRC_16_XMODEM, and yes it did work.

0
0
0

Jarkko Sakkinen

Edited 1 year ago

As a bug fix: https://github.com/jarkkojs/zmodem2/commit/ba5c5bbb7d521fc6df6177d1dec2825ea137da14

This abandoned looking zmodem crate is fully working and has als lrzsz based test suite, so kudos to the author for making this initial version, even if it since has been forgotten. Working code is always better than clean code…

So my crate is called zmodem2 and I’m purely focusing right now to make it work best possible way for my serial tool but I might want to go “beyond zmodem” later on. Since there’s not over-crowded supply of zmodem tools, I’ll add subcommands send and receive directly to the command-line so that terminal session is not necessity to do a file transfer.

1
0
1

Jarkko Sakkinen

After trial and error, i.e. brute force going through crc crate algorithms, I can say that CRC32_32_ISO_HDLC is the correct variant for ZMODEM 32-bit transfers, i.e. cipher can be acquired by:

const CRC32: Crc<u32> = Crc::<u32>::new(&CRC_32_ISO_HDLC);

It is better to declare like this so that it gets compiled into .rodata and not initialized at run-time.

1
0
0

Pretty easy way to implement this strategy: avoid using Vec to the extremes. It is quite common in Rust programs that there’s tons of Vec instances, while in reality most of them are fixed arrays on their usage patterns. In addition heapless provides pretty nice set of structures for common tasks with a fixed amount data space.

0
0
0
Show older