Application protocol function codes: PDU length

[Tipgeber]

I believe there is an issue in src/codec/rtu/mod.rs in fn request_pdu_len. You may want to check the Modbus application protocol spec, sections 6.11 and 6.12, to make sure. IMHO, the function should look like this:

pub const fn request_pdu_len(adu_buf: &[u8]) -> Result<Option<usize>> {
    if adu_buf.len() < 2 {
        return Ok(None);
    }
    let fn_code = adu_buf[1];
    let len = match fn_code {
        0x01..=0x06 => Some(5),
        0x07 | 0x0B | 0x0C | 0x11 => Some(1),
        0x0F => {
            if adu_buf.len() > 6
                && u16::from_be_bytes([adu_buf[4], adu_buf[5]]) == adu_buf[6] as u16
            {
                Some(6 + (adu_buf[6] as usize))
            } else {
                // incomplete frame
                None
            }
        }
        0x10 => {
            if adu_buf.len() > 6
                && u16::from_be_bytes([adu_buf[4], adu_buf[5]]).saturating_mul(2)
                    == adu_buf[6] as u16
            {
                Some(6 + (adu_buf[6] as usize))
            } else {
                // incomplete frame
                None
            }
        }
        0x16 => Some(7),
        0x18 => Some(3),
        0x17 => {
            if adu_buf.len() > 10 {
                Some(10 + adu_buf[10] as usize)
            } else {
                // incomplete frame
                None
            }
        }
        _ => {
            return Err(Error::FnCode(fn_code));
        }
    };
    Ok(len)
}

Specifically, for function codes 0x0F and 0x10, adu_buf[6], not adu_buf[4], should be used to determine the PDU length. This change fixed an issue in my application. I'd appreciate it if you integrate it in your crate.

[flosse]

@Tipgeber thanks for reporting!

Would you mind to open a PR?

[domenicquirl]

Shouldn't the check for 0x0F (Write Multiple Coils) be divided by 8 (plus or minus 1), to reflect how the following values are bitpacked to 1 byte per 8 coils (with the 9th coil bumping the data part up to 2 bytes)?

So something like

            let quantity = u16::from_be_bytes([adu_buf[4], adu_buf[5]]);
            let expected_count = if quantity % 8 == 0 {
                quantity.saturating_div(8)
            } else {
                quantity.saturating_div(8) + 1
            };