Check data length

[anti-social]

Fixes index out of bounds error when parsing invalid data

[tact1m4n3]

I just pushed a heavily refactored version of the project. I've noticed that this new version does not panic when parsing invalid data. I might be wrong tho... I haven't done much testing yet

[anti-social]

I'd rather return an error instead of coercing to the maximum value:

let len = (u8::from_le(data[1]) as usize + 2).min(Packet::MAX_LENGTH)

So if you are OK with that I will refactor the PR

[anti-social]

I've noticed that this new version does not panic when parsing invalid data

It returns InvalidType { raw_type: 0 } error instead. So it perceives third byte in a buffer as a packet type. It does not panic due to a static buffer length.

[anti-social]

Have a question. Why do you wrap bytes from a buffer in some sites into u8::from_le? Technically it is a no-op for any architecture.

[tact1m4n3]

Hey. Yeah I agree with you that we should return an error. But don’t start refactoring just yet, because I plan to change a lot of things like today(or tomorrow) :))

[tact1m4n3]

Have a question. Why do you wrap bytes from a buffer in some sites into u8::from_le? Technically it is a no-op for any architecture.

Really? Aren’t there architectures using big endian?

[anti-social]

Really? Aren’t there architectures using big endian?

But it is a single byte. It only does things for number types like u16, i16, u32 etc. I think from_le, from_be, swap_bytes are just methods that all number types have for consistency.

[tact1m4n3]

I think endianness affects single bytes as well as it dictates if you read from left or right those 8 bits

[anti-social]

Hey. Yeah I agree with you that we should return an error. But don’t start refactoring just yet, because I plan to change a lot of things like today(or tomorrow) :))

Sorry, did not have time to read your comment

[anti-social]

I think endianness affects single bytes as well as it dictates if you read from left or right those 8 bits

Endianness is about a byte order, not bit

[tact1m4n3]

It’s fine as I haven’t changed anything yet

[anti-social]

And you can simply check it:

assert_eq!(u8::from_le(0xef), 0xef);
assert_eq!(u8::from_be(0xef), 0xef);

[anti-social]

Found an answer: https://github.com/rust-lang/rust/issues/44724#issuecomment-344188069

[tact1m4n3]

You’re right. I had it wrong :))

[tact1m4n3]

Then, as you said, there is no need for those

Have a question. Why do you wrap bytes from a buffer in some sites into u8::from_le? Technically it is a no-op for any architecture.

[tact1m4n3]

Is there anything more you would like to add? I was thinking of making a release soon.

[anti-social]

Is there anything more you would like to add? I was thinking of making a release soon.

I would like to have a possibility to process all addresses. For example to be able to sniff packets between a transmitter and a handset.

[anti-social]

We could have 2 constructors for the parser: new and new_with_address

[anti-social]

You can say we can create multiple parsers: one to parse packets from a handset and the other to parse data from a transmitter. But a handset and a transmitter use half-duplex uart via single wire.

[tact1m4n3]

I went for a single address since that address would already be known to the user. If a parser parses packets with multiple addresses then we need a way of returning that address. maybe return it in a tuple with the packet

[tact1m4n3]

Something like this

[tact1m4n3]

Actually maybe it is better to create a new method like next_packet_with_address for that. What do you think?

[anti-social]

Actually maybe it is better to create a new method like next_packet_with_address for that. What do you think?

TBH I don't like adding a lot of similar methods

[anti-social]

Returning tuple is OK for me

[tact1m4n3]

alright.