chore: drop byteorder dependency

[paolobarbolini]

Feature or Problem

Drop the byteorder dependency, which could easily be replaced with functions from std and easier to read logic.

Related Issues

None

Release Information

next

Consumer Impact

One less dependency, and less unsafe code in the build

Testing

Read below

Unit Test(s)

None

Acceptance or Integration

None

Manual Verification

While manually verifying that the code is correct I've gotten even more confused at how this code works. Looking at the Go library the CRC is put at the end and read from the end, while here the CRC is written to the end and read from the start 🤷‍♂️. Are the tests wrong???

I've tried doing this and this seems to confirm my assumption:

diff --git a/src/crc.rs b/src/crc.rs
index 228880d..5dc5981 100644
--- a/src/crc.rs
+++ b/src/crc.rs
@@ -58,3 +58,23 @@ pub(crate) fn extract_crc(data: &mut Vec<u8>) -> Result<u16> {
     data.truncate(data.len() - 2);
     Ok(crc)
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::crc::crc16;
+
+    use super::{extract_crc, push_crc};
+
+    #[test]
+    fn im_confused() {
+        let raw_data = rand::random::<[u8; 32]>();
+        let crc = crc16(&raw_data);
+
+        let mut data = raw_data.to_vec();
+        push_crc(&mut data);
+
+        let crc2 = extract_crc(&mut data).unwrap();
+        assert_eq!(raw_data, data.as_slice());
+        assert_eq!(crc, crc2);
+    }
+}

[brooksmtownsend]

Thank you for filing this @paolobarbolini ! I'll take a look at this shortly to verify the behavior... seems weird.

[brooksmtownsend]

This functionality looks like a great replacement! I had one request to try and avoid an unwrap()

[brooksmtownsend]

I think your explanation is good enough that I can get over having an unwrap there 😉

[brooksmtownsend]

@paolobarbolini do you a need for this to be released? It's small enough I think it could be a patch.

[paolobarbolini]

Thanks for the merge. I'll see if I can send the second PR with the CRC moved to the end tomorrow.