Fix rtu request pdu length calculation #20
Conversation
|
Currently, I have neither a device to test this nor the time to look into the specifications. |
AsgerWenneb
force-pushed
the
bugfix/request-pdu-length
branch
from
1ad8054 to
50578af
Compare
|
Sorry for the late response. The branch should be rebased on the latest changes now. I've added some tests in the same style as existing tests. |
src/error.rs
Outdated
| /// Protocol not Modbus | ||
| ProtocolNotModbus(u16), | ||
| /// Length Mismatch | ||
| QuantityBytesMismatch(u16, u8, u16), |
There was a problem hiding this comment.
What do you think of this:
enum Error {
// ..
QuantityBytesMismatch {
quantity: u16,
bytes: u16,
bytes_expected: u16
}
}There was a problem hiding this comment.
I think the named variables are a great call.
Is it on purpose that bytes is a u16? I can change that as well, but it would need to be cast from u8 for all current instantiations.
There was a problem hiding this comment.
I think the named variables are a great call.
yes, we should change it also in the other variants.
Is it on purpose that
bytesis au16?
Well, the data type used for counting bytes should be the same (In this case, probably u8 after all.)
Unfortunately, I'm very busy right now, but I'll take a closer look at your PR soon.
| if adu_buf.len() > 4 { | ||
| Some(6 + adu_buf[4] as usize) | ||
| 0x0F => { | ||
| if adu_buf.len() > 6 { |
There was a problem hiding this comment.
Here we can do
let number_of_bytes = adu_buf.get(6)?;2 participants
PR to fix the request PDU length calculation mentioned in #12.
My application doesn't use coils but I've attempted to implement the fix for FC 0x0F as well. I have not looked at the TCP implementation at all (I have no experience with it). An error was also added for when the quantity and bytes fields don't match up.