From 81f7447c5204fb38a0c9c5031ba0c6304d17ccb8 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Tue, 15 Oct 2019 23:10:15 +0100
Subject: [PATCH 01/23] Refactor PacketIdentifier from u16 to NonZeroU16.

This matches the MQTT spec which mandates non-zero pids. Mqttrs will now error out when trying to
encode/decode a PacketIdentifier of 0, like for example mosquitto does. It also reduces the memory
usage of `Option<PacketIdentifier>`.

Fixes #14

Using PacketIdentifier is now more verbose, but mqttrs user won't be affected as much as mqttrs
itself. While we're breaking the API, it's tempting to shorten `PacketIdentifier` to `Pid`, as we've
already got `QoS` and will hopefully soone get `QoSPid`.

PacketIdentifyer::new() is pretty much a TryInto trait, but using that would have raised the minimum
rust version requirement for little convenience.
---
 src/codec_test.rs   | 41 +++++++++++++++++++++++------------------
 src/decoder.rs      | 20 ++++++++++----------
 src/decoder_test.rs | 34 +++++++++++++---------------------
 src/encoder.rs      | 10 +++++-----
 src/encoder_test.rs | 42 +++++++++++++++---------------------------
 src/publish.rs      |  4 ++--
 src/subscribe.rs    | 14 ++++++--------
 src/utils.rs        | 18 ++++++++++++++++--
 8 files changed, 90 insertions(+), 93 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 0d8cf31..d36af21 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -13,6 +13,11 @@ prop_compose! {
         QoS::from_u8(qos).unwrap()
     }
 }
+prop_compose! {
+    fn stg_pid()(pid in 1..std::u16::MAX) -> PacketIdentifier {
+        PacketIdentifier::new(pid).unwrap()
+    }
+}
 prop_compose! {
     fn stg_subtopic()(topic_path in stg_topic(), qos in stg_qos()) -> SubscribeTopic {
         SubscribeTopic { topic_path, qos }
@@ -58,7 +63,7 @@ prop_compose! {
 prop_compose! {
     fn stg_publish()(dup in bool::ANY,
                      qos in 0u8..3,
-                     pid in u16::ANY,
+                     pid in stg_pid(),
                      retain in bool::ANY,
                      topic_name in stg_topic(),
                      payload in vec(0u8..255u8, 1..300)) -> Packet {
@@ -66,48 +71,48 @@ prop_compose! {
                                 qos: QoS::from_u8(qos).unwrap(),
                                 retain,
                                 topic_name,
-                                pid: if qos == 0 { None } else { Some(PacketIdentifier(pid)) },
+                                pid: if qos == 0 { None } else { Some(pid) },
                                 payload})
     }
 }
 prop_compose! {
-    fn stg_puback()(pid in u16::ANY) -> Packet {
-        Packet::Puback(PacketIdentifier(pid))
+    fn stg_puback()(pid in stg_pid()) -> Packet {
+        Packet::Puback(pid)
     }
 }
 prop_compose! {
-    fn stg_pubrec()(pid in u16::ANY) -> Packet {
-        Packet::Pubrec(PacketIdentifier(pid))
+    fn stg_pubrec()(pid in stg_pid()) -> Packet {
+        Packet::Pubrec(pid)
     }
 }
 prop_compose! {
-    fn stg_pubrel()(pid in u16::ANY) -> Packet {
-        Packet::Puback(PacketIdentifier(pid))
+    fn stg_pubrel()(pid in stg_pid()) -> Packet {
+        Packet::Puback(pid)
     }
 }
 prop_compose! {
-    fn stg_pubcomp()(pid in u16::ANY) -> Packet {
-        Packet::PubComp(PacketIdentifier(pid))
+    fn stg_pubcomp()(pid in stg_pid()) -> Packet {
+        Packet::PubComp(pid)
     }
 }
 prop_compose! {
-    fn stg_subscribe()(pid in u16::ANY, topics in vec(stg_subtopic(), 0..20)) -> Packet {
-        Packet::Subscribe(Subscribe{pid: PacketIdentifier(pid), topics})
+    fn stg_subscribe()(pid in stg_pid(), topics in vec(stg_subtopic(), 0..20)) -> Packet {
+        Packet::Subscribe(Subscribe{pid: pid, topics})
     }
 }
 prop_compose! {
-    fn stg_suback()(pid in u16::ANY, return_codes in vec(stg_subretcode(), 0..300)) -> Packet {
-        Packet::SubAck(Suback{pid: PacketIdentifier(pid), return_codes})
+    fn stg_suback()(pid in stg_pid(), return_codes in vec(stg_subretcode(), 0..300)) -> Packet {
+        Packet::SubAck(Suback{pid: pid, return_codes})
     }
 }
 prop_compose! {
-    fn stg_unsubscribe()(pid in u16::ANY, topics in vec(stg_topic(), 0..20)) -> Packet {
-        Packet::UnSubscribe(Unsubscribe{pid:PacketIdentifier(pid), topics})
+    fn stg_unsubscribe()(pid in stg_pid(), topics in vec(stg_topic(), 0..20)) -> Packet {
+        Packet::UnSubscribe(Unsubscribe{pid:pid, topics})
     }
 }
 prop_compose! {
-    fn stg_unsuback()(pid in u16::ANY) -> Packet {
-        Packet::UnSubAck(PacketIdentifier(pid))
+    fn stg_unsuback()(pid in stg_pid()) -> Packet {
+        Packet::UnSubAck(pid)
     }
 }
 prop_compose! {
diff --git a/src/decoder.rs b/src/decoder.rs
index cd8e3c4..b1129b5 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -35,18 +35,18 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, io::Erro
             &header,
             &mut buffer.split_to(header.len()),
         )?)),
-        PacketType::Puback => Ok(Packet::Puback(PacketIdentifier(
+        PacketType::Puback => Ok(Packet::Puback(PacketIdentifier::new(
             buffer.split_to(2).into_buf().get_u16_be(),
-        ))),
-        PacketType::Pubrec => Ok(Packet::Pubrec(PacketIdentifier(
+        )?)),
+        PacketType::Pubrec => Ok(Packet::Pubrec(PacketIdentifier::new(
             buffer.split_to(2).into_buf().get_u16_be(),
-        ))),
-        PacketType::Pubrel => Ok(Packet::Pubrel(PacketIdentifier(
+        )?)),
+        PacketType::Pubrel => Ok(Packet::Pubrel(PacketIdentifier::new(
             buffer.split_to(2).into_buf().get_u16_be(),
-        ))),
-        PacketType::PubComp => Ok(Packet::PubComp(PacketIdentifier(
+        )?)),
+        PacketType::PubComp => Ok(Packet::PubComp(PacketIdentifier::new(
             buffer.split_to(2).into_buf().get_u16_be(),
-        ))),
+        )?)),
         PacketType::Subscribe => Ok(Packet::Subscribe(Subscribe::from_buffer(
             &mut buffer.split_to(header.len()),
         )?)),
@@ -56,9 +56,9 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, io::Erro
         PacketType::UnSubscribe => Ok(Packet::UnSubscribe(Unsubscribe::from_buffer(
             &mut buffer.split_to(header.len()),
         )?)),
-        PacketType::UnSubAck => Ok(Packet::UnSubAck(PacketIdentifier(
+        PacketType::UnSubAck => Ok(Packet::UnSubAck(PacketIdentifier::new(
             buffer.split_to(2).into_buf().get_u16_be(),
-        ))),
+        )?)),
     }
 }
 /* This will read the header of the stream */
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index 7b01b5b..c52bc0f 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -124,7 +124,7 @@ fn test_publish() {
             assert_eq!(p.retain, true);
             assert_eq!(p.qos, QoS::ExactlyOnce);
             assert_eq!(p.topic_name, "a/b");
-            assert_eq!(p.pid.unwrap(), PacketIdentifier(10));
+            assert_eq!(p.pid, Some(PacketIdentifier::new(10).unwrap()));
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
         _ => panic!("Should not be None"),
@@ -136,11 +136,9 @@ fn test_pub_ack() {
     let mut data = BytesMut::from(vec![0b01000000, 0b00000010, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::Puback(a)) => {
-            assert_eq!(a.0, 10);
-        }
+        Some(Packet::Puback(a)) => assert_eq!(a.get(), 10),
         _ => panic!(),
-    }
+    };
 }
 
 #[test]
@@ -148,11 +146,9 @@ fn test_pub_rec() {
     let mut data = BytesMut::from(vec![0b01010000, 0b00000010, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::Pubrec(a)) => {
-            assert_eq!(a.0, 10);
-        }
+        Some(Packet::Pubrec(a)) => assert_eq!(a.get(), 10),
         _ => panic!(),
-    }
+    };
 }
 
 #[test]
@@ -160,11 +156,9 @@ fn test_pub_rel() {
     let mut data = BytesMut::from(vec![0b01100010, 0b00000010, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::Pubrel(a)) => {
-            assert_eq!(a.0, 10);
-        }
+        Some(Packet::Pubrel(a)) => assert_eq!(a.get(), 10),
         _ => panic!(),
-    }
+    };
 }
 
 #[test]
@@ -172,11 +166,9 @@ fn test_pub_comp() {
     let mut data = BytesMut::from(vec![0b01110000, 0b00000010, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::PubComp(a)) => {
-            assert_eq!(a.0, 10);
-        }
+        Some(Packet::PubComp(a)) => assert_eq!(a.get(), 10),
         _ => panic!(),
-    }
+    };
 }
 
 #[test]
@@ -188,7 +180,7 @@ fn test_subscribe() {
     let d = decoder::decode(&mut data).unwrap();
     match d {
         Some(Packet::Subscribe(s)) => {
-            assert_eq!(s.pid.0, 10);
+            assert_eq!(s.pid.get(), 10);
             let t = SubscribeTopic {
                 topic_path: "a/b".to_string(),
                 qos: QoS::AtMostOnce,
@@ -206,7 +198,7 @@ fn test_suback() {
     let d = decoder::decode(&mut data).unwrap();
     match d {
         Some(Packet::SubAck(s)) => {
-            assert_eq!(s.pid.0, 10);
+            assert_eq!(s.pid.get(), 10);
             assert_eq!(
                 s.return_codes[0],
                 SubscribeReturnCodes::Success(QoS::ExactlyOnce)
@@ -224,7 +216,7 @@ fn test_unsubscribe() {
     let d = decoder::decode(&mut data).unwrap();
     match d {
         Some(Packet::UnSubscribe(a)) => {
-            assert_eq!(a.pid.0, 10);
+            assert_eq!(a.pid.get(), 10);
             assert_eq!(a.topics[0], 'a'.to_string());
         }
         _ => panic!(),
@@ -237,7 +229,7 @@ fn test_unsub_ack() {
     let d = decoder::decode(&mut data).unwrap();
     match d {
         Some(Packet::UnSubAck(p)) => {
-            assert_eq!(p.0, 10);
+            assert_eq!(p.get(), 10);
         }
         _ => panic!(),
     }
diff --git a/src/encoder.rs b/src/encoder.rs
index 96ab7d5..b5f9576 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -13,7 +13,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.0);
+            buffer.put_u16_be(pid.get());
             Ok(())
         }
         Packet::Pubrec(pid) => {
@@ -21,7 +21,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.0);
+            buffer.put_u16_be(pid.get());
             Ok(())
         }
         Packet::Pubrel(pid) => {
@@ -29,7 +29,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.0);
+            buffer.put_u16_be(pid.get());
             Ok(())
         }
         Packet::PubComp(pid) => {
@@ -37,7 +37,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.0);
+            buffer.put_u16_be(pid.get());
             Ok(())
         }
         Packet::Subscribe(subscribe) => subscribe.to_buffer(buffer),
@@ -48,7 +48,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.0);
+            buffer.put_u16_be(pid.get());
             Ok(())
         }
         Packet::PingReq => {
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index 26aa7e9..dc40f13 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -53,7 +53,7 @@ fn test_publish() {
         qos: QoS::ExactlyOnce,
         retain: true,
         topic_name: "asdf".to_string(),
-        pid: Some(PacketIdentifier(10)),
+        pid: Some(PacketIdentifier::new(10).unwrap()),
         payload: vec!['h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -70,7 +70,7 @@ fn test_publish() {
 
 #[test]
 fn test_puback() {
-    let packet = Packet::Puback(PacketIdentifier(19));
+    let packet = Packet::Puback(PacketIdentifier::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -84,7 +84,7 @@ fn test_puback() {
 
 #[test]
 fn test_pubrec() {
-    let packet = Packet::Pubrec(PacketIdentifier(19));
+    let packet = Packet::Pubrec(PacketIdentifier::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -98,7 +98,7 @@ fn test_pubrec() {
 
 #[test]
 fn test_pubrel() {
-    let packet = Packet::Pubrel(PacketIdentifier(19));
+    let packet = Packet::Pubrel(PacketIdentifier::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -113,7 +113,7 @@ fn test_pubrel() {
 
 #[test]
 fn test_pubcomp() {
-    let packet = Packet::PubComp(PacketIdentifier(19));
+    let packet = Packet::PubComp(PacketIdentifier::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -132,7 +132,7 @@ fn test_subscribe() {
         qos: QoS::ExactlyOnce,
     };
     let packet = Subscribe {
-        pid: PacketIdentifier(345),
+        pid: PacketIdentifier::new(345).unwrap(),
         topics: vec![stopic],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -150,16 +150,14 @@ fn test_subscribe() {
 fn test_suback() {
     let return_code = SubscribeReturnCodes::Success(QoS::ExactlyOnce);
     let packet = Suback {
-        pid: PacketIdentifier(12321),
+        pid: PacketIdentifier::new(12321).unwrap(),
         return_codes: vec![return_code],
     };
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&Packet::SubAck(packet), &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::SubAck(_c)) => {
-            assert!(true);
-        }
+        Some(Packet::SubAck(_c)) => assert!(true),
         _ => assert!(false),
     }
 }
@@ -167,30 +165,26 @@ fn test_suback() {
 #[test]
 fn test_unsubscribe() {
     let packet = Unsubscribe {
-        pid: PacketIdentifier(12321),
+        pid: PacketIdentifier::new(12321).unwrap(),
         topics: vec!["a/b".to_string()],
     };
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&Packet::UnSubscribe(packet), &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::UnSubscribe(_c)) => {
-            assert!(true);
-        }
+        Some(Packet::UnSubscribe(_c)) => assert!(true),
         _ => assert!(false),
     }
 }
 
 #[test]
 fn test_unsuback() {
-    let packet = Packet::UnSubAck(PacketIdentifier(19));
+    let packet = Packet::UnSubAck(PacketIdentifier::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::UnSubAck(_c)) => {
-            assert!(true);
-        }
+        Some(Packet::UnSubAck(_c)) => assert!(true),
         _ => assert!(false),
     }
 }
@@ -201,9 +195,7 @@ fn test_ping_req() {
     let _ = encoder::encode(&Packet::PingReq, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::PingReq) => {
-            assert!(true);
-        }
+        Some(Packet::PingReq) => assert!(true),
         _ => assert!(false),
     }
 }
@@ -214,9 +206,7 @@ fn test_ping_resp() {
     let _ = encoder::encode(&Packet::PingResp, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::PingResp) => {
-            assert!(true);
-        }
+        Some(Packet::PingResp) => assert!(true),
         _ => assert!(false),
     }
 }
@@ -227,9 +217,7 @@ fn test_disconnect() {
     let _ = encoder::encode(&Packet::Disconnect, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
     match decoded {
-        Some(Packet::Disconnect) => {
-            assert!(true);
-        }
+        Some(Packet::Disconnect) => assert!(true),
         _ => assert!(false),
     }
 }
diff --git a/src/publish.rs b/src/publish.rs
index 6357ce8..d074e74 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -19,7 +19,7 @@ impl Publish {
         let pid = if header.qos()? == QoS::AtMostOnce {
             None
         } else {
-            Some(PacketIdentifier(buffer.split_to(2).into_buf().get_u16_be()))
+            Some(PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?)
         };
 
         let payload = buffer.to_vec();
@@ -58,7 +58,7 @@ impl Publish {
 
         // Pid
         if self.qos != QoS::AtMostOnce {
-            buffer.put_u16_be(self.pid.unwrap().0 as u16);
+            buffer.put_u16_be(self.pid.unwrap().get());
         }
 
         // Payload
diff --git a/src/subscribe.rs b/src/subscribe.rs
index 7838a6c..b5cc98f 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -42,7 +42,7 @@ pub struct Unsubscribe {
 
 impl Subscribe {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier(buffer.split_to(2).into_buf().get_u16_be());
+        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = utils::read_string(buffer);
@@ -65,7 +65,7 @@ impl Subscribe {
         encoder::write_length(length, buffer)?;
 
         // Pid
-        buffer.put_u16_be(self.pid.0);
+        buffer.put_u16_be(self.pid.get());
 
         // Topics
         for topic in &self.topics {
@@ -79,7 +79,7 @@ impl Subscribe {
 
 impl Unsubscribe {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier(buffer.split_to(2).into_buf().get_u16_be());
+        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = utils::read_string(buffer);
@@ -90,7 +90,6 @@ impl Unsubscribe {
 
     pub fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), io::Error>{
         let header_u8 : u8 = 0b10100010;
-        let PacketIdentifier(pid) = self.pid;
         let mut length = 2;
         for topic in &self.topics{
             length += 2 + topic.len();
@@ -98,7 +97,7 @@ impl Unsubscribe {
 
         buffer.put(header_u8);
         encoder::write_length(length, buffer)?;
-        buffer.put_u16_be(pid as u16);
+        buffer.put_u16_be(self.pid.get());
         for topic in&self.topics{
             encoder::write_string(topic.as_ref(), buffer)?;
         }
@@ -108,7 +107,7 @@ impl Unsubscribe {
 
 impl Suback {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier(buffer.split_to(2).into_buf().get_u16_be());
+        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
         let mut return_codes: Vec<SubscribeReturnCodes> = Vec::new();
         while buffer.len() != 0 {
             let code = buffer.split_to(1).into_buf().get_u8();
@@ -123,12 +122,11 @@ impl Suback {
     }
     pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         let header_u8: u8 = 0b10010000;
-        let PacketIdentifier(pid) = self.pid;
         let length = 2 + self.return_codes.len();
 
         buffer.put(header_u8);
         encoder::write_length(length, buffer)?;
-        buffer.put_u16_be(pid);
+        buffer.put_u16_be(self.pid.get());
         for rc in &self.return_codes {
             buffer.put(rc.to_u8());
         }
diff --git a/src/utils.rs b/src/utils.rs
index 554ded3..64f0be4 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,11 +1,25 @@
 use bytes::{Buf, BytesMut, IntoBuf};
-use std::io;
+use std::{io, num::NonZeroU16};
 
 /// Packet Identifier, for ack purposes.
 ///
 /// Note that the spec disallows a pid of 0 ([MQTT-2.3.1-1] for mqtt3, [MQTT-2.2.1-3] for mqtt5).
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct PacketIdentifier(pub u16);
+pub struct PacketIdentifier(NonZeroU16);
+impl PacketIdentifier {
+    pub fn new(u: u16) -> Result<Self, io::Error> {
+        match NonZeroU16::new(u) {
+            Some(nz) => Ok(PacketIdentifier(nz)),
+            None => Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                "Zero PacketIdentifier",
+            )),
+        }
+    }
+    pub fn get(self) -> u16 {
+        self.0.get()
+    }
+}
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Protocol {

From b055854499e8ae26fc5da15dd784aa4cada2431c Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 16 Oct 2019 00:11:22 +0100
Subject: [PATCH 02/23] Refactor: Use PacketIdentifier::{from_buffer,
 to_buffer}.

DRY.
---
 src/decoder.rs   | 65 ++++++++++++++++++++----------------------------
 src/encoder.rs   | 10 ++++----
 src/publish.rs   |  6 ++---
 src/subscribe.rs | 12 ++++-----
 src/utils.rs     |  8 +++++-
 5 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/src/decoder.rs b/src/decoder.rs
index b1129b5..1ff9fec 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,6 +1,6 @@
 use crate::MULTIPLIER;
 use crate::*;
-use bytes::{Buf, BytesMut, IntoBuf};
+use bytes::BytesMut;
 use std::io;
 
 #[allow(dead_code)]
@@ -20,46 +20,35 @@ pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, io::Error> {
 }
 
 fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, io::Error> {
-    let t = header.packet();
-    match t {
-        PacketType::PingReq => Ok(Packet::PingReq),
-        PacketType::PingResp => Ok(Packet::PingResp),
-        PacketType::Disconnect => Ok(Packet::Disconnect),
-        PacketType::Connect => Ok(Packet::Connect(Connect::from_buffer(
-            &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::Connack => Ok(Packet::Connack(Connack::from_buffer(
-            &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::Publish => Ok(Packet::Publish(Publish::from_buffer(
+    Ok(match header.packet() {
+        PacketType::PingReq => Packet::PingReq,
+        PacketType::PingResp => Packet::PingResp,
+        PacketType::Disconnect => Packet::Disconnect,
+        PacketType::Connect => {
+            Packet::Connect(Connect::from_buffer(&mut buffer.split_to(header.len()))?)
+        }
+        PacketType::Connack => {
+            Packet::Connack(Connack::from_buffer(&mut buffer.split_to(header.len()))?)
+        }
+        PacketType::Publish => Packet::Publish(Publish::from_buffer(
             &header,
             &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::Puback => Ok(Packet::Puback(PacketIdentifier::new(
-            buffer.split_to(2).into_buf().get_u16_be(),
-        )?)),
-        PacketType::Pubrec => Ok(Packet::Pubrec(PacketIdentifier::new(
-            buffer.split_to(2).into_buf().get_u16_be(),
-        )?)),
-        PacketType::Pubrel => Ok(Packet::Pubrel(PacketIdentifier::new(
-            buffer.split_to(2).into_buf().get_u16_be(),
-        )?)),
-        PacketType::PubComp => Ok(Packet::PubComp(PacketIdentifier::new(
-            buffer.split_to(2).into_buf().get_u16_be(),
-        )?)),
-        PacketType::Subscribe => Ok(Packet::Subscribe(Subscribe::from_buffer(
-            &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::SubAck => Ok(Packet::SubAck(Suback::from_buffer(
-            &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::UnSubscribe => Ok(Packet::UnSubscribe(Unsubscribe::from_buffer(
+        )?),
+        PacketType::Puback => Packet::Puback(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::Pubrec => Packet::Pubrec(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::Pubrel => Packet::Pubrel(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::PubComp => Packet::PubComp(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::Subscribe => {
+            Packet::Subscribe(Subscribe::from_buffer(&mut buffer.split_to(header.len()))?)
+        }
+        PacketType::SubAck => {
+            Packet::SubAck(Suback::from_buffer(&mut buffer.split_to(header.len()))?)
+        }
+        PacketType::UnSubscribe => Packet::UnSubscribe(Unsubscribe::from_buffer(
             &mut buffer.split_to(header.len()),
-        )?)),
-        PacketType::UnSubAck => Ok(Packet::UnSubAck(PacketIdentifier::new(
-            buffer.split_to(2).into_buf().get_u16_be(),
-        )?)),
-    }
+        )?),
+        PacketType::UnSubAck => Packet::UnSubAck(PacketIdentifier::from_buffer(buffer)?),
+    })
 }
 /* This will read the header of the stream */
 fn read_header(buffer: &mut BytesMut) -> Option<(Header, usize)> {
diff --git a/src/encoder.rs b/src/encoder.rs
index b5f9576..292a869 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -13,7 +13,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.get());
+            pid.to_buffer(buffer);
             Ok(())
         }
         Packet::Pubrec(pid) => {
@@ -21,7 +21,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.get());
+            pid.to_buffer(buffer);
             Ok(())
         }
         Packet::Pubrel(pid) => {
@@ -29,7 +29,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.get());
+            pid.to_buffer(buffer);
             Ok(())
         }
         Packet::PubComp(pid) => {
@@ -37,7 +37,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.get());
+            pid.to_buffer(buffer);
             Ok(())
         }
         Packet::Subscribe(subscribe) => subscribe.to_buffer(buffer),
@@ -48,7 +48,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            buffer.put_u16_be(pid.get());
+            pid.to_buffer(buffer);
             Ok(())
         }
         Packet::PingReq => {
diff --git a/src/publish.rs b/src/publish.rs
index d074e74..2114011 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,5 +1,5 @@
 use crate::{encoder, utils, Header, PacketIdentifier, QoS};
-use bytes::{Buf, BufMut, BytesMut, IntoBuf};
+use bytes::{BufMut, BytesMut};
 use std::io;
 
 #[derive(Debug, Clone, PartialEq)]
@@ -19,7 +19,7 @@ impl Publish {
         let pid = if header.qos()? == QoS::AtMostOnce {
             None
         } else {
-            Some(PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?)
+            Some(PacketIdentifier::from_buffer(buffer)?)
         };
 
         let payload = buffer.to_vec();
@@ -58,7 +58,7 @@ impl Publish {
 
         // Pid
         if self.qos != QoS::AtMostOnce {
-            buffer.put_u16_be(self.pid.unwrap().get());
+            self.pid.unwrap().to_buffer(buffer);
         }
 
         // Payload
diff --git a/src/subscribe.rs b/src/subscribe.rs
index b5cc98f..7461637 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -42,7 +42,7 @@ pub struct Unsubscribe {
 
 impl Subscribe {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
+        let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = utils::read_string(buffer);
@@ -65,7 +65,7 @@ impl Subscribe {
         encoder::write_length(length, buffer)?;
 
         // Pid
-        buffer.put_u16_be(self.pid.get());
+        self.pid.to_buffer(buffer);
 
         // Topics
         for topic in &self.topics {
@@ -79,7 +79,7 @@ impl Subscribe {
 
 impl Unsubscribe {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
+        let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = utils::read_string(buffer);
@@ -97,7 +97,7 @@ impl Unsubscribe {
 
         buffer.put(header_u8);
         encoder::write_length(length, buffer)?;
-        buffer.put_u16_be(self.pid.get());
+        self.pid.to_buffer(buffer);
         for topic in&self.topics{
             encoder::write_string(topic.as_ref(), buffer)?;
         }
@@ -107,7 +107,7 @@ impl Unsubscribe {
 
 impl Suback {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let pid = PacketIdentifier::new(buffer.split_to(2).into_buf().get_u16_be())?;
+        let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut return_codes: Vec<SubscribeReturnCodes> = Vec::new();
         while buffer.len() != 0 {
             let code = buffer.split_to(1).into_buf().get_u8();
@@ -126,7 +126,7 @@ impl Suback {
 
         buffer.put(header_u8);
         encoder::write_length(length, buffer)?;
-        buffer.put_u16_be(self.pid.get());
+        self.pid.to_buffer(buffer);
         for rc in &self.return_codes {
             buffer.put(rc.to_u8());
         }
diff --git a/src/utils.rs b/src/utils.rs
index 64f0be4..0183cf4 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,4 +1,4 @@
-use bytes::{Buf, BytesMut, IntoBuf};
+use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::{io, num::NonZeroU16};
 
 /// Packet Identifier, for ack purposes.
@@ -19,6 +19,12 @@ impl PacketIdentifier {
     pub fn get(self) -> u16 {
         self.0.get()
     }
+    pub(crate) fn from_buffer(buf: &mut BytesMut) -> Result<Self, io::Error> {
+        Self::new(buf.split_to(2).into_buf().get_u16_be())
+    }
+    pub(crate) fn to_buffer(self, buf: &mut BytesMut) {
+        buf.put_u16_be(self.get())
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]

From 213d8c4f76e052251ec476499f83f98ae8fd56c2 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 16 Oct 2019 01:03:39 +0100
Subject: [PATCH 03/23] refactor: Merge `Publish.{qos,pid}` into
 `Publish.qospid`.

This avoids the gotcha (both for mqttrs and its users) of setting AtMostOnce with a pid, or
AtLeastonce/ExactlyOnce without one.

Fixes #11

Breaking change.
---
 src/codec_test.rs   |  9 ++++++---
 src/decoder_test.rs | 11 +++++------
 src/encoder_test.rs |  5 ++---
 src/lib.rs          |  2 +-
 src/publish.rs      | 33 ++++++++++++++++++---------------
 src/utils.rs        | 24 +++++++++++++++++++-----
 6 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index d36af21..26c07f1 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -62,16 +62,19 @@ prop_compose! {
 }
 prop_compose! {
     fn stg_publish()(dup in bool::ANY,
-                     qos in 0u8..3,
+                     qos in stg_qos(),
                      pid in stg_pid(),
                      retain in bool::ANY,
                      topic_name in stg_topic(),
                      payload in vec(0u8..255u8, 1..300)) -> Packet {
         Packet::Publish(Publish{dup,
-                                qos: QoS::from_u8(qos).unwrap(),
+                                qospid: match qos {
+                                    QoS::AtMostOnce => QosPid::AtMostOnce,
+                                    QoS::AtLeastOnce => QosPid::AtLeastOnce(pid),
+                                    QoS::ExactlyOnce => QosPid::ExactlyOnce(pid),
+                                },
                                 retain,
                                 topic_name,
-                                pid: if qos == 0 { None } else { Some(pid) },
                                 payload})
     }
 }
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index c52bc0f..2de80d3 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -1,8 +1,8 @@
 #![allow(unused_imports)]
 
 use crate::{
-    decoder, Connack, ConnectReturnCode, Packet, PacketIdentifier, QoS, SubscribeReturnCodes,
-    SubscribeTopic,
+    decoder, Connack, ConnectReturnCode, Packet, PacketIdentifier, QoS, QosPid,
+    SubscribeReturnCodes, SubscribeTopic,
 };
 use bytes::BytesMut;
 
@@ -102,7 +102,7 @@ fn test_publish() {
         Some(Packet::Publish(p)) => {
             assert_eq!(p.dup, false);
             assert_eq!(p.retain, false);
-            assert_eq!(p.qos, QoS::AtMostOnce);
+            assert_eq!(p.qospid, QosPid::AtMostOnce);
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
@@ -112,7 +112,7 @@ fn test_publish() {
         Some(Packet::Publish(p)) => {
             assert_eq!(p.dup, true);
             assert_eq!(p.retain, false);
-            assert_eq!(p.qos, QoS::AtMostOnce);
+            assert_eq!(p.qospid, QosPid::AtMostOnce);
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
@@ -122,9 +122,8 @@ fn test_publish() {
         Some(Packet::Publish(p)) => {
             assert_eq!(p.dup, true);
             assert_eq!(p.retain, true);
-            assert_eq!(p.qos, QoS::ExactlyOnce);
+            assert_eq!(p.qospid, QosPid::from_u8u16(2, 10).unwrap());
             assert_eq!(p.topic_name, "a/b");
-            assert_eq!(p.pid, Some(PacketIdentifier::new(10).unwrap()));
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
         _ => panic!("Should not be None"),
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index dc40f13..2297236 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -1,7 +1,7 @@
 #[allow(unused_imports)]
 use crate::{
     decoder, encoder, Connack, Connect, ConnectReturnCode, Packet, PacketIdentifier, Protocol,
-    Publish, QoS, Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe,
+    Publish, QoS, QosPid, Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe,
 };
 
 #[allow(unused_imports)]
@@ -50,10 +50,9 @@ fn test_connack() {
 fn test_publish() {
     let packet = Publish {
         dup: false,
-        qos: QoS::ExactlyOnce,
+        qospid: QosPid::from_u8u16(2, 10).unwrap(),
         retain: true,
         topic_name: "asdf".to_string(),
-        pid: Some(PacketIdentifier::new(10).unwrap()),
         payload: vec!['h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8],
     };
     let mut buffer = BytesMut::with_capacity(1024);
diff --git a/src/lib.rs b/src/lib.rs
index 4ad13f3..e1d84a1 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -21,7 +21,7 @@ pub use crate::{
     packet::Packet,
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
-    utils::{ConnectReturnCode, LastWill, PacketIdentifier, Protocol, QoS},
+    utils::{ConnectReturnCode, LastWill, PacketIdentifier, Protocol, QoS, QosPid},
 };
 
 const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
diff --git a/src/publish.rs b/src/publish.rs
index 2114011..b6160c1 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,14 +1,13 @@
-use crate::{encoder, utils, Header, PacketIdentifier, QoS};
+use crate::{encoder, utils, Header, PacketIdentifier, QoS, QosPid};
 use bytes::{BufMut, BytesMut};
 use std::io;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Publish {
     pub dup: bool,
-    pub qos: QoS,
+    pub qospid: QosPid,
     pub retain: bool,
     pub topic_name: String,
-    pub pid: Option<PacketIdentifier>,
     pub payload: Vec<u8>,
 }
 
@@ -16,26 +15,28 @@ impl Publish {
     pub fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let topic_name = utils::read_string(buffer);
 
-        let pid = if header.qos()? == QoS::AtMostOnce {
-            None
-        } else {
-            Some(PacketIdentifier::from_buffer(buffer)?)
+        let qospid = match header.qos()? {
+            QoS::AtMostOnce => QosPid::AtMostOnce,
+            QoS::AtLeastOnce => QosPid::AtLeastOnce(PacketIdentifier::from_buffer(buffer)?),
+            QoS::ExactlyOnce => QosPid::ExactlyOnce(PacketIdentifier::from_buffer(buffer)?),
         };
 
         let payload = buffer.to_vec();
         Ok(Publish {
             dup: header.dup(),
-            qos: header.qos()?,
+            qospid,
             retain: header.retain(),
             topic_name,
-            pid,
             payload,
         })
     }
     pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         // Header
-        let mut header_u8: u8 = 0b00110000 as u8;
-        header_u8 |= (self.qos.to_u8()) << 1;
+        let mut header_u8: u8 = match self.qospid {
+            QosPid::AtMostOnce => 0b00110000,
+            QosPid::AtLeastOnce(_) => 0b00110010,
+            QosPid::ExactlyOnce(_) => 0b00110100,
+        };
         if self.dup {
             header_u8 |= 0b00001000 as u8;
         };
@@ -46,8 +47,8 @@ impl Publish {
 
         // Length: topic (2+len) + pid (0/2) + payload (len)
         let length = self.topic_name.len()
-            + match self.qos {
-                QoS::AtMostOnce => 2,
+            + match self.qospid {
+                QosPid::AtMostOnce => 2,
                 _ => 4,
             }
             + self.payload.len();
@@ -57,8 +58,10 @@ impl Publish {
         encoder::write_string(self.topic_name.as_ref(), buffer)?;
 
         // Pid
-        if self.qos != QoS::AtMostOnce {
-            self.pid.unwrap().to_buffer(buffer);
+        match self.qospid {
+            QosPid::AtMostOnce => (),
+            QosPid::AtLeastOnce(pid) => pid.to_buffer(buffer),
+            QosPid::ExactlyOnce(pid) => pid.to_buffer(buffer),
         }
 
         // Payload
diff --git a/src/utils.rs b/src/utils.rs
index 0183cf4..6aeeb96 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -10,10 +10,7 @@ impl PacketIdentifier {
     pub fn new(u: u16) -> Result<Self, io::Error> {
         match NonZeroU16::new(u) {
             Some(nz) => Ok(PacketIdentifier(nz)),
-            None => Err(io::Error::new(
-                io::ErrorKind::InvalidData,
-                "Zero PacketIdentifier",
-            )),
+            None => Err(io::Error::new(io::ErrorKind::InvalidData, "Pid == 0")),
         }
     }
     pub fn get(self) -> u16 {
@@ -52,7 +49,7 @@ impl QoS {
             0 => Ok(QoS::AtMostOnce),
             1 => Ok(QoS::AtLeastOnce),
             2 => Ok(QoS::ExactlyOnce),
-            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "")),
+            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "Qos > 2")),
         }
     }
     #[inline]
@@ -61,6 +58,23 @@ impl QoS {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum QosPid {
+    AtMostOnce,
+    AtLeastOnce(PacketIdentifier),
+    ExactlyOnce(PacketIdentifier),
+}
+impl QosPid {
+    pub fn from_u8u16(qos: u8, pid: u16) -> Result<Self, io::Error> {
+        match qos {
+            0 => Ok(QosPid::AtMostOnce),
+            1 => Ok(QosPid::AtLeastOnce(PacketIdentifier::new(pid)?)),
+            2 => Ok(QosPid::ExactlyOnce(PacketIdentifier::new(pid)?)),
+            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "Qos > 2")),
+        }
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum ConnectReturnCode {
     Accepted,

From 4ef316a1001e5b8992237f3a7837699ecd317148 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 16 Oct 2019 11:28:53 +0100
Subject: [PATCH 04/23] Refactor: Connect.password and
 Connect.last_will.message are Vec<u8>, not String.

As per MQTT spec.

Fixes #10

Breaking change.

Along with the type change, code was reorganized a bit, in particular 'use' statements and moving
read functions from utils to encoder, matching the write functions' location.
---
 src/codec_test.rs |  2 +-
 src/connect.rs    | 30 +++++++++++++++---------------
 src/decoder.rs    | 17 ++++++++++++++---
 src/encoder.rs    | 11 +++++++----
 src/publish.rs    |  8 ++++----
 src/subscribe.rs  | 16 ++++++++--------
 src/utils.rs      |  9 ++-------
 7 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 26c07f1..20c24f9 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -51,7 +51,7 @@ prop_compose! {
                                   clean_session,
                                   last_will: None,
                                   username,
-                                  password })
+                                  password: password.map(|p| p.as_bytes().to_vec()) })
     }
 }
 prop_compose! {
diff --git a/src/connect.rs b/src/connect.rs
index dd6983b..3c4e56d 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -1,4 +1,4 @@
-use crate::{encoder, utils, ConnectReturnCode, LastWill, Protocol, QoS};
+use crate::{decoder::*, encoder::*, ConnectReturnCode, LastWill, Protocol, QoS};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::io;
 
@@ -10,7 +10,7 @@ pub struct Connect {
     pub clean_session: bool,
     pub last_will: Option<LastWill>,
     pub username: Option<String>,
-    pub password: Option<String>,
+    pub password: Option<Vec<u8>>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -21,18 +21,18 @@ pub struct Connack {
 
 impl Connect {
     pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let protocol_name = utils::read_string(buffer);
+        let protocol_name = read_string(buffer);
         let protocol_level = buffer.split_to(1).into_buf().get_u8();
         let protocol = Protocol::new(&protocol_name, protocol_level).unwrap();
 
         let connect_flags = buffer.split_to(1).into_buf().get_u8();
         let keep_alive = buffer.split_to(2).into_buf().get_u16_be();
 
-        let client_id = utils::read_string(buffer);
+        let client_id = read_string(buffer);
 
         let last_will = if connect_flags & 0b100 != 0 {
-            let will_topic = utils::read_string(buffer);
-            let will_message = utils::read_string(buffer);
+            let will_topic = read_string(buffer);
+            let will_message = read_bytes(buffer);
             let will_qod = QoS::from_u8((connect_flags & 0b11000) >> 3).unwrap();
             Some(LastWill {
                 topic: will_topic,
@@ -45,13 +45,13 @@ impl Connect {
         };
 
         let username = if connect_flags & 0b10000000 != 0 {
-            Some(utils::read_string(buffer))
+            Some(read_string(buffer))
         } else {
             None
         };
 
         let password = if connect_flags & 0b01000000 != 0 {
-            Some(utils::read_string(buffer))
+            Some(read_bytes(buffer))
         } else {
             None
         };
@@ -100,23 +100,23 @@ impl Connect {
 
         //NOTE: putting data into buffer.
         buffer.put(header_u8);
-        encoder::write_length(length, buffer)?;
-        encoder::write_string(self.protocol.name(), buffer)?;
+        write_length(length, buffer)?;
+        write_string(self.protocol.name(), buffer)?;
         buffer.put(self.protocol.level());
         buffer.put(connect_flags);
         buffer.put_u16_be(self.keep_alive);
-        encoder::write_string(self.client_id.as_ref(), buffer)?;
+        write_string(self.client_id.as_ref(), buffer)?;
 
         if let Some(last_will) = &self.last_will {
-            encoder::write_string(last_will.topic.as_ref(), buffer)?;
-            encoder::write_string(last_will.message.as_ref(), buffer)?;
+            write_string(last_will.topic.as_ref(), buffer)?;
+            write_bytes(&last_will.message, buffer)?;
         };
 
         if let Some(username) = &self.username {
-            encoder::write_string(username.as_ref(), buffer)?;
+            write_string(username.as_ref(), buffer)?;
         };
         if let Some(password) = &self.password {
-            encoder::write_string(password.as_ref(), buffer)?;
+            write_bytes(password, buffer)?;
         };
         //NOTE: END
         Ok(())
diff --git a/src/decoder.rs b/src/decoder.rs
index 1ff9fec..ce8f2a2 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,9 +1,8 @@
 use crate::MULTIPLIER;
 use crate::*;
-use bytes::BytesMut;
+use bytes::{Buf, BytesMut, IntoBuf};
 use std::io;
 
-#[allow(dead_code)]
 pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, io::Error> {
     if let Some((header, header_size)) = read_header(buffer) {
         if buffer.len() >= header.len() + header_size {
@@ -50,7 +49,8 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, io::Erro
         PacketType::UnSubAck => Packet::UnSubAck(PacketIdentifier::from_buffer(buffer)?),
     })
 }
-/* This will read the header of the stream */
+
+/// Read the header of the stream
 fn read_header(buffer: &mut BytesMut) -> Option<(Header, usize)> {
     if buffer.len() > 1 {
         let header_u8 = buffer.get(0).unwrap();
@@ -85,3 +85,14 @@ fn read_length(buffer: &BytesMut, mut pos: usize) -> Option<(usize, usize)> {
     }
     Some((len as usize, pos))
 }
+
+// FIXME: Result<String,...>
+pub fn read_string(buffer: &mut BytesMut) -> String {
+    String::from_utf8(read_bytes(buffer)).expect("Non-utf8 string")
+}
+
+// FIXME: This can panic if the packet is malformed
+pub fn read_bytes(buffer: &mut BytesMut) -> Vec<u8> {
+    let length = buffer.split_to(2).into_buf().get_u16_be();
+    buffer.split_to(length as usize).to_vec()
+}
diff --git a/src/encoder.rs b/src/encoder.rs
index 292a869..5709f85 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -2,7 +2,6 @@ use crate::{Packet, MAX_PAYLOAD_SIZE};
 use bytes::{BufMut, BytesMut};
 use std::io;
 
-#[allow(dead_code)]
 pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
     match packet {
         Packet::Connect(connect) => connect.to_buffer(buffer),
@@ -90,8 +89,12 @@ pub fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::Error>
     Ok(())
 }
 
-pub fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), io::Error> {
-    buffer.put_u16_be(string.len() as u16);
-    buffer.put_slice(string.as_bytes());
+pub fn write_bytes(bytes: &[u8], buffer: &mut BytesMut) -> Result<(), io::Error> {
+    buffer.put_u16_be(bytes.len() as u16);
+    buffer.put_slice(bytes);
     Ok(())
 }
+
+pub fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    write_bytes(string.as_bytes(), buffer)
+}
diff --git a/src/publish.rs b/src/publish.rs
index b6160c1..7c8f906 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,4 +1,4 @@
-use crate::{encoder, utils, Header, PacketIdentifier, QoS, QosPid};
+use crate::{decoder::*, encoder::*, Header, PacketIdentifier, QoS, QosPid};
 use bytes::{BufMut, BytesMut};
 use std::io;
 
@@ -13,7 +13,7 @@ pub struct Publish {
 
 impl Publish {
     pub fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let topic_name = utils::read_string(buffer);
+        let topic_name = read_string(buffer);
 
         let qospid = match header.qos()? {
             QoS::AtMostOnce => QosPid::AtMostOnce,
@@ -52,10 +52,10 @@ impl Publish {
                 _ => 4,
             }
             + self.payload.len();
-        encoder::write_length(length, buffer)?;
+        write_length(length, buffer)?;
 
         // Topic
-        encoder::write_string(self.topic_name.as_ref(), buffer)?;
+        write_string(self.topic_name.as_ref(), buffer)?;
 
         // Pid
         match self.qospid {
diff --git a/src/subscribe.rs b/src/subscribe.rs
index 7461637..e7fbcba 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -1,4 +1,4 @@
-use crate::{encoder, utils, PacketIdentifier, QoS};
+use crate::{decoder::*, encoder::*, PacketIdentifier, QoS};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::io;
 
@@ -45,7 +45,7 @@ impl Subscribe {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
-            let topic_path = utils::read_string(buffer);
+            let topic_path = read_string(buffer);
             let qos = QoS::from_u8(buffer.split_to(1).into_buf().get_u8())?;
             let topic = SubscribeTopic { topic_path, qos };
             topics.push(topic);
@@ -62,14 +62,14 @@ impl Subscribe {
         for topic in &self.topics {
             length += topic.topic_path.len() + 2 + 1;
         }
-        encoder::write_length(length, buffer)?;
+        write_length(length, buffer)?;
 
         // Pid
         self.pid.to_buffer(buffer);
 
         // Topics
         for topic in &self.topics {
-            encoder::write_string(topic.topic_path.as_ref(), buffer)?;
+            write_string(topic.topic_path.as_ref(), buffer)?;
             buffer.put(topic.qos.to_u8());
         }
 
@@ -82,7 +82,7 @@ impl Unsubscribe {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
-            let topic_path = utils::read_string(buffer);
+            let topic_path = read_string(buffer);
             topics.push(topic_path);
         }
         Ok(Unsubscribe { pid, topics })
@@ -96,10 +96,10 @@ impl Unsubscribe {
         }
 
         buffer.put(header_u8);
-        encoder::write_length(length, buffer)?;
+        write_length(length, buffer)?;
         self.pid.to_buffer(buffer);
         for topic in&self.topics{
-            encoder::write_string(topic.as_ref(), buffer)?;
+            write_string(topic.as_ref(), buffer)?;
         }
         Ok(())
     }
@@ -125,7 +125,7 @@ impl Suback {
         let length = 2 + self.return_codes.len();
 
         buffer.put(header_u8);
-        encoder::write_length(length, buffer)?;
+        write_length(length, buffer)?;
         self.pid.to_buffer(buffer);
         for rc in &self.return_codes {
             buffer.put(rc.to_u8());
diff --git a/src/utils.rs b/src/utils.rs
index 6aeeb96..3ad20c1 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -19,6 +19,7 @@ impl PacketIdentifier {
     pub(crate) fn from_buffer(buf: &mut BytesMut) -> Result<Self, io::Error> {
         Self::new(buf.split_to(2).into_buf().get_u16_be())
     }
+    // FIXME: Result<(), io::Error>
     pub(crate) fn to_buffer(self, buf: &mut BytesMut) {
         buf.put_u16_be(self.get())
     }
@@ -88,7 +89,7 @@ pub enum ConnectReturnCode {
 #[derive(Debug, Clone, PartialEq)]
 pub struct LastWill {
     pub topic: String,
-    pub message: String,
+    pub message: Vec<u8>,
     pub qos: QoS,
     pub retain: bool,
 }
@@ -147,9 +148,3 @@ impl ConnectReturnCode {
         }
     }
 }
-
-pub fn read_string(buffer: &mut BytesMut) -> String {
-    let length = buffer.split_to(2).into_buf().get_u16_be();
-    let byts = buffer.split_to(length as usize);
-    return String::from_utf8(byts.to_vec()).unwrap().to_string();
-}

From dd469713fa76e74227748b5edd7ee7e23ef94475 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 16 Oct 2019 12:18:23 +0100
Subject: [PATCH 05/23] Refactor: Tweak API visibility.

Hopefully I got the balance right, but we can always reexport missing APIs if this cull was too
eager.

One note about PacketType: it got moved to `packet.rs` and can now be obtained from a `Packet`
enum. Most end-users will just `macth` on `Packet` variants, but it can be interesting to match a
single type or to gather stats about all types.
---
 src/connect.rs   |  8 +++---
 src/decoder.rs   | 10 ++++---
 src/encoder.rs   |  9 ++++---
 src/header.rs    | 68 ++----------------------------------------------
 src/lib.rs       | 10 +++----
 src/packet.rs    | 64 +++++++++++++++++++++++++++++++++++++++++++++
 src/publish.rs   |  6 ++---
 src/subscribe.rs | 14 +++++-----
 src/utils.rs     | 11 +++++---
 9 files changed, 104 insertions(+), 96 deletions(-)

diff --git a/src/connect.rs b/src/connect.rs
index 3c4e56d..8242432 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -20,7 +20,7 @@ pub struct Connack {
 }
 
 impl Connect {
-    pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let protocol_name = read_string(buffer);
         let protocol_level = buffer.split_to(1).into_buf().get_u8();
         let protocol = Protocol::new(&protocol_name, protocol_level).unwrap();
@@ -68,7 +68,7 @@ impl Connect {
             clean_session,
         })
     }
-    pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         let header_u8: u8 = 0b00010000;
         let mut length: usize = 6 + 1 + 1; //NOTE: protocol_name(6) + protocol_level(1) + flags(1);
         let mut connect_flags: u8 = 0b00000000;
@@ -124,7 +124,7 @@ impl Connect {
 }
 
 impl Connack {
-    pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let flags = buffer.split_to(1).into_buf().get_u8();
         let return_code = buffer.split_to(1).into_buf().get_u8();
         Ok(Connack {
@@ -132,7 +132,7 @@ impl Connack {
             code: ConnectReturnCode::from_u8(return_code)?,
         })
     }
-    pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         let header_u8 = 0b00100000 as u8;
         let length = 2 as u8;
         let mut flags = 0b00000000 as u8;
diff --git a/src/decoder.rs b/src/decoder.rs
index ce8f2a2..1915627 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,8 +1,10 @@
-use crate::MULTIPLIER;
-use crate::*;
+use crate::{header::Header, *};
 use bytes::{Buf, BytesMut, IntoBuf};
 use std::io;
 
+/// Decode network bytes into a [Packet] enum.
+///
+/// [Packet]: ../enum.Packet.html
 pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, io::Error> {
     if let Some((header, header_size)) = read_header(buffer) {
         if buffer.len() >= header.len() + header_size {
@@ -87,12 +89,12 @@ fn read_length(buffer: &BytesMut, mut pos: usize) -> Option<(usize, usize)> {
 }
 
 // FIXME: Result<String,...>
-pub fn read_string(buffer: &mut BytesMut) -> String {
+pub(crate) fn read_string(buffer: &mut BytesMut) -> String {
     String::from_utf8(read_bytes(buffer)).expect("Non-utf8 string")
 }
 
 // FIXME: This can panic if the packet is malformed
-pub fn read_bytes(buffer: &mut BytesMut) -> Vec<u8> {
+pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Vec<u8> {
     let length = buffer.split_to(2).into_buf().get_u16_be();
     buffer.split_to(length as usize).to_vec()
 }
diff --git a/src/encoder.rs b/src/encoder.rs
index 5709f85..72219fa 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -2,6 +2,9 @@ use crate::{Packet, MAX_PAYLOAD_SIZE};
 use bytes::{BufMut, BytesMut};
 use std::io;
 
+/// Encode a [Packet] enum into a buffer.
+///
+/// [Packet]: ../enum.Packet.html
 pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
     match packet {
         Packet::Connect(connect) => connect.to_buffer(buffer),
@@ -68,7 +71,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
     }
 }
 
-pub fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::Error> {
     if len > MAX_PAYLOAD_SIZE {
         return Err(io::Error::new(
             io::ErrorKind::PermissionDenied,
@@ -89,12 +92,12 @@ pub fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::Error>
     Ok(())
 }
 
-pub fn write_bytes(bytes: &[u8], buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub(crate) fn write_bytes(bytes: &[u8], buffer: &mut BytesMut) -> Result<(), io::Error> {
     buffer.put_u16_be(bytes.len() as u16);
     buffer.put_slice(bytes);
     Ok(())
 }
 
-pub fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub(crate) fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), io::Error> {
     write_bytes(string.as_bytes(), buffer)
 }
diff --git a/src/header.rs b/src/header.rs
index d8195b4..057a688 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -1,72 +1,8 @@
-use crate::QoS;
+use crate::{PacketType, QoS};
 use std::io;
 
-#[derive(Debug, Copy, Clone, PartialEq)]
-pub enum PacketType {
-    Connect,
-    Connack,
-    Publish,
-    Puback,
-    Pubrec,
-    Pubrel,
-    PubComp,
-    Subscribe,
-    SubAck,
-    UnSubscribe,
-    UnSubAck,
-    PingReq,
-    PingResp,
-    Disconnect,
-}
-impl PacketType {
-    #[inline]
-    pub fn from_hd(hd: u8) -> Result<PacketType, io::Error> {
-        Self::from_u8(hd >> 4)
-    }
-    pub fn to_u8(&self) -> u8 {
-        match *self {
-            PacketType::Connect => 1,
-            PacketType::Connack => 2,
-            PacketType::Publish => 3,
-            PacketType::Puback => 4,
-            PacketType::Pubrec => 5,
-            PacketType::Pubrel => 6,
-            PacketType::PubComp => 7,
-            PacketType::Subscribe => 8,
-            PacketType::SubAck => 9,
-            PacketType::UnSubscribe => 10,
-            PacketType::UnSubAck => 11,
-            PacketType::PingReq => 12,
-            PacketType::PingResp => 13,
-            PacketType::Disconnect => 14,
-        }
-    }
-    pub fn from_u8(byte: u8) -> Result<Self, io::Error> {
-        match byte {
-            1 => Ok(PacketType::Connect),
-            2 => Ok(PacketType::Connack),
-            3 => Ok(PacketType::Publish),
-            4 => Ok(PacketType::Puback),
-            5 => Ok(PacketType::Pubrec),
-            6 => Ok(PacketType::Pubrel),
-            7 => Ok(PacketType::PubComp),
-            8 => Ok(PacketType::Subscribe),
-            9 => Ok(PacketType::SubAck),
-            10 => Ok(PacketType::UnSubscribe),
-            11 => Ok(PacketType::UnSubAck),
-            12 => Ok(PacketType::PingReq),
-            13 => Ok(PacketType::PingResp),
-            14 => Ok(PacketType::Disconnect),
-            _ => Err(io::Error::new(
-                io::ErrorKind::InvalidInput,
-                "Unsupported packet type".to_string(),
-            )),
-        }
-    }
-}
-
 #[derive(Debug, Clone, PartialEq)]
-pub struct Header {
+pub(crate) struct Header {
     hd: u8,
     packet_type: PacketType,
     len: usize,
diff --git a/src/lib.rs b/src/lib.rs
index e1d84a1..cab2e2b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,4 +1,6 @@
 mod connect;
+mod decoder;
+mod encoder;
 mod header;
 mod packet;
 mod publish;
@@ -12,13 +14,11 @@ mod decoder_test;
 #[cfg(test)]
 mod encoder_test;
 
-pub mod decoder;
-pub mod encoder;
-
 pub use crate::{
     connect::{Connack, Connect},
-    header::{Header, PacketType},
-    packet::Packet,
+    decoder::decode,
+    encoder::encode,
+    packet::{Packet, PacketType},
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
     utils::{ConnectReturnCode, LastWill, PacketIdentifier, Protocol, QoS, QosPid},
diff --git a/src/packet.rs b/src/packet.rs
index 29b7a25..9889a41 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,4 +1,5 @@
 use crate::{Connack, Connect, PacketIdentifier, Publish, Suback, Subscribe, Unsubscribe};
+use std::io;
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum Packet {
@@ -17,3 +18,66 @@ pub enum Packet {
     PingResp,
     Disconnect,
 }
+impl Packet {
+    pub fn get_type(&self) -> PacketType {
+        match self {
+            Packet::Connect(_) => PacketType::Connect,
+            Packet::Connack(_) => PacketType::Connack,
+            Packet::Publish(_) => PacketType::Publish,
+            Packet::Puback(_) => PacketType::Puback,
+            Packet::Pubrec(_) => PacketType::Pubrec,
+            Packet::Pubrel(_) => PacketType::Pubrel,
+            Packet::PubComp(_) => PacketType::PubComp,
+            Packet::Subscribe(_) => PacketType::Subscribe,
+            Packet::SubAck(_) => PacketType::SubAck,
+            Packet::UnSubscribe(_) => PacketType::UnSubscribe,
+            Packet::UnSubAck(_) => PacketType::UnSubAck,
+            Packet::PingReq => PacketType::PingReq,
+            Packet::PingResp => PacketType::PingResp,
+            Packet::Disconnect => PacketType::Disconnect,
+        }
+    }
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+pub enum PacketType {
+    Connect,
+    Connack,
+    Publish,
+    Puback,
+    Pubrec,
+    Pubrel,
+    PubComp,
+    Subscribe,
+    SubAck,
+    UnSubscribe,
+    UnSubAck,
+    PingReq,
+    PingResp,
+    Disconnect,
+}
+impl PacketType {
+    #[inline]
+    pub(crate) fn from_hd(hd: u8) -> Result<Self, io::Error> {
+        match hd >> 4 {
+            1 => Ok(PacketType::Connect),
+            2 => Ok(PacketType::Connack),
+            3 => Ok(PacketType::Publish),
+            4 => Ok(PacketType::Puback),
+            5 => Ok(PacketType::Pubrec),
+            6 => Ok(PacketType::Pubrel),
+            7 => Ok(PacketType::PubComp),
+            8 => Ok(PacketType::Subscribe),
+            9 => Ok(PacketType::SubAck),
+            10 => Ok(PacketType::UnSubscribe),
+            11 => Ok(PacketType::UnSubAck),
+            12 => Ok(PacketType::PingReq),
+            13 => Ok(PacketType::PingResp),
+            14 => Ok(PacketType::Disconnect),
+            _ => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "Unsupported packet type",
+            )),
+        }
+    }
+}
diff --git a/src/publish.rs b/src/publish.rs
index 7c8f906..44832dd 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,4 +1,4 @@
-use crate::{decoder::*, encoder::*, Header, PacketIdentifier, QoS, QosPid};
+use crate::{decoder::*, encoder::*, header::Header, PacketIdentifier, QoS, QosPid};
 use bytes::{BufMut, BytesMut};
 use std::io;
 
@@ -12,7 +12,7 @@ pub struct Publish {
 }
 
 impl Publish {
-    pub fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let topic_name = read_string(buffer);
 
         let qospid = match header.qos()? {
@@ -30,7 +30,7 @@ impl Publish {
             payload,
         })
     }
-    pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         // Header
         let mut header_u8: u8 = match self.qospid {
             QosPid::AtMostOnce => 0b00110000,
diff --git a/src/subscribe.rs b/src/subscribe.rs
index e7fbcba..89e7fd7 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -14,7 +14,7 @@ pub enum SubscribeReturnCodes {
     Failure,
 }
 impl SubscribeReturnCodes {
-    pub fn to_u8(&self) -> u8 {
+    pub(crate) fn to_u8(&self) -> u8 {
         match *self {
             SubscribeReturnCodes::Failure => 0x80,
             SubscribeReturnCodes::Success(qos) => qos.to_u8(),
@@ -41,7 +41,7 @@ pub struct Unsubscribe {
 }
 
 impl Subscribe {
-    pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
@@ -53,7 +53,7 @@ impl Subscribe {
         Ok(Subscribe { pid, topics })
     }
 
-    pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         let header_u8: u8 = 0b10000010;
         buffer.put(header_u8);
 
@@ -78,7 +78,7 @@ impl Subscribe {
 }
 
 impl Unsubscribe {
-    pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
@@ -88,7 +88,7 @@ impl Unsubscribe {
         Ok(Unsubscribe { pid, topics })
     }
 
-    pub fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), io::Error>{
+    pub(crate) fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), io::Error>{
         let header_u8 : u8 = 0b10100010;
         let mut length = 2;
         for topic in &self.topics{
@@ -106,7 +106,7 @@ impl Unsubscribe {
 }
 
 impl Suback {
-    pub fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut return_codes: Vec<SubscribeReturnCodes> = Vec::new();
         while buffer.len() != 0 {
@@ -120,7 +120,7 @@ impl Suback {
         }
         Ok(Suback { return_codes, pid })
     }
-    pub fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
         let header_u8: u8 = 0b10010000;
         let length = 2 + self.return_codes.len();
 
diff --git a/src/utils.rs b/src/utils.rs
index 3ad20c1..034bcaf 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -3,7 +3,10 @@ use std::{io, num::NonZeroU16};
 
 /// Packet Identifier, for ack purposes.
 ///
-/// Note that the spec disallows a pid of 0 ([MQTT-2.3.1-1] for mqtt3, [MQTT-2.2.1-3] for mqtt5).
+/// The spec ([MQTT-2.3.1-1], [MQTT-2.2.1-3]) disallows a pid of 0.
+///
+/// [MQTT-2.3.1-1]: https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718025
+/// [MQTT-2.2.1-3]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901026
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct PacketIdentifier(NonZeroU16);
 impl PacketIdentifier {
@@ -54,7 +57,7 @@ impl QoS {
         }
     }
     #[inline]
-    pub fn from_hd(hd: u8) -> Result<QoS, io::Error> {
+    pub(crate) fn from_hd(hd: u8) -> Result<QoS, io::Error> {
         Self::from_u8((hd & 0b110) >> 1)
     }
 }
@@ -109,14 +112,14 @@ impl Protocol {
         }
     }
 
-    pub fn name(&self) -> &'static str {
+    pub(crate) fn name(&self) -> &'static str {
         match self {
             &Protocol::MQIsdp(_) => "MQIsdp",
             &Protocol::MQTT(_) => "MQTT",
         }
     }
 
-    pub fn level(&self) -> u8 {
+    pub(crate) fn level(&self) -> u8 {
         match self {
             &Protocol::MQIsdp(level) => level,
             &Protocol::MQTT(level) => level,

From 9c89b1c8ac7f54b5f33801a3283f4b608c7f544f Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 16 Oct 2019 13:55:45 +0100
Subject: [PATCH 06/23] refactor: simplify `Protocol` enum.

There's really no need for fancyness when we only support two possible values. Clearer API, more
straightforward code. Also, moved the enum to `header.rs` and added some docstrings.

Breaking change.
---
 src/codec_test.rs |  2 +-
 src/connect.rs    |  3 +--
 src/header.rs     | 38 ++++++++++++++++++++++++++++++++++++++
 src/lib.rs        |  3 ++-
 src/utils.rs      | 36 ------------------------------------
 5 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 20c24f9..cae24ae 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -45,7 +45,7 @@ prop_compose! {
                      clean_session in bool::ANY,
                      username in stg_optstr(),
                      password in stg_optstr()) -> Packet {
-        Packet::Connect(Connect { protocol: Protocol::MQTT(4),
+        Packet::Connect(Connect { protocol: Protocol::MQTT311,
                                   keep_alive,
                                   client_id,
                                   clean_session,
diff --git a/src/connect.rs b/src/connect.rs
index 8242432..7f34bf1 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -101,8 +101,7 @@ impl Connect {
         //NOTE: putting data into buffer.
         buffer.put(header_u8);
         write_length(length, buffer)?;
-        write_string(self.protocol.name(), buffer)?;
-        buffer.put(self.protocol.level());
+        self.protocol.to_buffer(buffer)?;
         buffer.put(connect_flags);
         buffer.put_u16_be(self.keep_alive);
         write_string(self.client_id.as_ref(), buffer)?;
diff --git a/src/header.rs b/src/header.rs
index 057a688..4533651 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -1,4 +1,5 @@
 use crate::{PacketType, QoS};
+use bytes::{BufMut, BytesMut};
 use std::io;
 
 #[derive(Debug, Clone, PartialEq)]
@@ -37,6 +38,43 @@ impl Header {
     }
 }
 
+/// Protocol version sent at connection.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Protocol {
+    /// [MQTT 3.1.1] is the most commonly implemented version. [MQTT 5] isn't yet supported my by
+    /// `mqttrs`.
+    ///
+    /// [MQTT 3.1.1]: https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html
+    /// [MQTT 5]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html
+    MQTT311,
+    /// MQIsdp, aka SCADA are pre-standardisation names of MQTT. It should mostly conform to MQTT
+    /// 3.1.1, but you should watch out for implementation discrepancies. `Mqttrs` handles it like
+    /// standard MQTT 3.1.1.
+    MQIsdp,
+}
+impl Protocol {
+    pub(crate) fn new(name: &str, level: u8) -> Result<Protocol, io::Error> {
+        match (name, level) {
+            ("MQIsdp", 3) => Ok(Protocol::MQIsdp),
+            ("MQTT", 4) => Ok(Protocol::MQTT311),
+            _ => Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("Unsupported protocol {:?} {}", name, level),
+            )),
+        }
+    }
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+        match self {
+            Protocol::MQTT311 => {
+                Ok(buffer.put_slice(&[0u8, 4, 'M' as u8, 'Q' as u8, 'T' as u8, 'T' as u8, 4]))
+            }
+            Protocol::MQIsdp => Ok(buffer.put_slice(&[
+                0u8, 4, 'M' as u8, 'Q' as u8, 'i' as u8, 's' as u8, 'd' as u8, 'p' as u8, 4,
+            ])),
+        }
+    }
+}
+
 /* TESTS */
 #[cfg(test)]
 mod tests {
diff --git a/src/lib.rs b/src/lib.rs
index cab2e2b..9fadec0 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -18,10 +18,11 @@ pub use crate::{
     connect::{Connack, Connect},
     decoder::decode,
     encoder::encode,
+    header::Protocol,
     packet::{Packet, PacketType},
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
-    utils::{ConnectReturnCode, LastWill, PacketIdentifier, Protocol, QoS, QosPid},
+    utils::{ConnectReturnCode, LastWill, PacketIdentifier, QoS, QosPid},
 };
 
 const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
diff --git a/src/utils.rs b/src/utils.rs
index 034bcaf..5d84cfc 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -28,12 +28,6 @@ impl PacketIdentifier {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum Protocol {
-    MQIsdp(u8),
-    MQTT(u8),
-}
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum QoS {
     AtMostOnce,
@@ -97,36 +91,6 @@ pub struct LastWill {
     pub retain: bool,
 }
 
-impl Protocol {
-    pub fn new(name: &str, level: u8) -> Result<Protocol, io::Error> {
-        match name {
-            "MQIsdp" => match level {
-                3 => Ok(Protocol::MQIsdp(3)),
-                _ => Err(io::Error::new(io::ErrorKind::InvalidData, "")),
-            },
-            "MQTT" => match level {
-                4 => Ok(Protocol::MQTT(4)),
-                _ => Err(io::Error::new(io::ErrorKind::InvalidData, "")),
-            },
-            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "")),
-        }
-    }
-
-    pub(crate) fn name(&self) -> &'static str {
-        match self {
-            &Protocol::MQIsdp(_) => "MQIsdp",
-            &Protocol::MQTT(_) => "MQTT",
-        }
-    }
-
-    pub(crate) fn level(&self) -> u8 {
-        match self {
-            &Protocol::MQIsdp(level) => level,
-            &Protocol::MQTT(level) => level,
-        }
-    }
-}
-
 impl ConnectReturnCode {
     pub fn to_u8(&self) -> u8 {
         match *self {

From 93d88fda486a6318fafda89e3d618500a3835cd3 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Thu, 17 Oct 2019 14:54:50 +0100
Subject: [PATCH 07/23] Improve error checking and reporting.

The main change is to turn all (?) "buffer too small to read/write" panics into clean
`std::io::Error`s with an associated reason string. There are also some misc error checked, like
non-utf8 strings.

I haven't reviewed every "unwrap()", just added some proptests and fixed some obvious places. It'd
be great to find any remaining panic (if any) using automated testing rather than code review.
---
 src/codec_test.rs | 16 +++++++++++++
 src/connect.rs    | 25 +++++++++++---------
 src/decoder.rs    | 60 ++++++++++++++++++++++++++++++++++++++++-------
 src/encoder.rs    | 59 +++++++++++++++++++++++++++++-----------------
 src/header.rs     | 14 +++++------
 src/lib.rs        |  1 -
 src/packet.rs     |  8 +++----
 src/publish.rs    | 13 +++++-----
 src/subscribe.rs  | 31 +++++++++++++-----------
 src/utils.rs      | 32 ++++++++++++++-----------
 10 files changed, 171 insertions(+), 88 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index cae24ae..da638d7 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -1,6 +1,7 @@
 use crate::*;
 use bytes::BytesMut;
 use proptest::{bool, collection::vec, num::*, prelude::*};
+use std::io::ErrorKind;
 
 // Proptest strategies to generate packet elements
 prop_compose! {
@@ -163,6 +164,21 @@ macro_rules! impl_proptests {
                 encoded.split_off(encoded.len() - 1);
                 let decoded = decoder::decode(&mut encoded).unwrap();
                 prop_assert!(decoded.is_none(), "partial decode {:?} -> {:?}", encoded, decoded);
+
+                // Check that encoding into a small buffer fails cleanly
+                buf.clear();
+                buf.split_off(encoded.len()+1);
+                prop_assert!(encoder::encode(&pkt.clone(), &mut buf).is_ok(), "exact buffer capacity {}", buf.capacity());
+                loop {
+                    buf.clear();
+                    buf.split_off(buf.capacity()-1);
+                    prop_assert_eq!(ErrorKind::WriteZero,
+                                    encoder::encode(&pkt.clone(), &mut buf).unwrap_err().kind(),
+                                    "small buffer capacity {}/{}", buf.capacity(), encoded.len());
+                    if buf.capacity() == 0 {
+                        break;
+                    }
+                }
             }
         }
     };
diff --git a/src/connect.rs b/src/connect.rs
index 7f34bf1..421f56a 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -1,6 +1,6 @@
 use crate::{decoder::*, encoder::*, ConnectReturnCode, LastWill, Protocol, QoS};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
-use std::io;
+use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Connect {
@@ -20,19 +20,19 @@ pub struct Connack {
 }
 
 impl Connect {
-    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let protocol_name = read_string(buffer);
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
+        let protocol_name = read_string(buffer)?;
         let protocol_level = buffer.split_to(1).into_buf().get_u8();
         let protocol = Protocol::new(&protocol_name, protocol_level).unwrap();
 
         let connect_flags = buffer.split_to(1).into_buf().get_u8();
         let keep_alive = buffer.split_to(2).into_buf().get_u16_be();
 
-        let client_id = read_string(buffer);
+        let client_id = read_string(buffer)?;
 
         let last_will = if connect_flags & 0b100 != 0 {
-            let will_topic = read_string(buffer);
-            let will_message = read_bytes(buffer);
+            let will_topic = read_string(buffer)?;
+            let will_message = read_bytes(buffer)?;
             let will_qod = QoS::from_u8((connect_flags & 0b11000) >> 3).unwrap();
             Some(LastWill {
                 topic: will_topic,
@@ -45,13 +45,13 @@ impl Connect {
         };
 
         let username = if connect_flags & 0b10000000 != 0 {
-            Some(read_string(buffer))
+            Some(read_string(buffer)?)
         } else {
             None
         };
 
         let password = if connect_flags & 0b01000000 != 0 {
-            Some(read_bytes(buffer))
+            Some(read_bytes(buffer)?)
         } else {
             None
         };
@@ -68,7 +68,8 @@ impl Connect {
             clean_session,
         })
     }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
+
         let header_u8: u8 = 0b00010000;
         let mut length: usize = 6 + 1 + 1; //NOTE: protocol_name(6) + protocol_level(1) + flags(1);
         let mut connect_flags: u8 = 0b00000000;
@@ -97,6 +98,7 @@ impl Connect {
             length += last_will.topic.len();
             length += 4;
         };
+        check_remaining(buffer, length+1)?;
 
         //NOTE: putting data into buffer.
         buffer.put(header_u8);
@@ -123,7 +125,7 @@ impl Connect {
 }
 
 impl Connack {
-    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
         let flags = buffer.split_to(1).into_buf().get_u8();
         let return_code = buffer.split_to(1).into_buf().get_u8();
         Ok(Connack {
@@ -131,7 +133,8 @@ impl Connack {
             code: ConnectReturnCode::from_u8(return_code)?,
         })
     }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
+        check_remaining(buffer, 4)?;
         let header_u8 = 0b00100000 as u8;
         let length = 2 as u8;
         let mut flags = 0b00000000 as u8;
diff --git a/src/decoder.rs b/src/decoder.rs
index 1915627..7a58441 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,11 +1,11 @@
 use crate::{header::Header, *};
 use bytes::{Buf, BytesMut, IntoBuf};
-use std::io;
+use std::io::{Error, ErrorKind};
 
 /// Decode network bytes into a [Packet] enum.
 ///
 /// [Packet]: ../enum.Packet.html
-pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, io::Error> {
+pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, Error> {
     if let Some((header, header_size)) = read_header(buffer) {
         if buffer.len() >= header.len() + header_size {
             //NOTE: Check if buffer has, header bytes + remaining length bytes in buffer.
@@ -20,7 +20,7 @@ pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, io::Error> {
     }
 }
 
-fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, io::Error> {
+fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
     Ok(match header.packet() {
         PacketType::PingReq => Packet::PingReq,
         PacketType::PingResp => Packet::PingResp,
@@ -88,13 +88,55 @@ fn read_length(buffer: &BytesMut, mut pos: usize) -> Option<(usize, usize)> {
     Some((len as usize, pos))
 }
 
-// FIXME: Result<String,...>
-pub(crate) fn read_string(buffer: &mut BytesMut) -> String {
-    String::from_utf8(read_bytes(buffer)).expect("Non-utf8 string")
+pub(crate) fn read_string(buffer: &mut BytesMut) -> Result<String, Error> {
+    String::from_utf8(read_bytes(buffer)?)
+        .map_err(|_| Error::new(ErrorKind::InvalidData, "Non-utf8 string"))
 }
 
-// FIXME: This can panic if the packet is malformed
-pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Vec<u8> {
+pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
     let length = buffer.split_to(2).into_buf().get_u16_be();
-    buffer.split_to(length as usize).to_vec()
+    if length as usize > buffer.len() {
+        Err(Error::new(ErrorKind::InvalidData, "length > buffer.len()"))
+    } else {
+        Ok(buffer.split_to(length as usize).to_vec())
+    }
+}
+
+
+#[cfg(test)]
+mod test {
+    use crate::decode;
+    use bytes::BytesMut;
+    use std::io::ErrorKind;
+
+    #[test]
+    fn non_utf8_string() {
+        let mut data = BytesMut::from(vec![
+            0b00110000, 10, // type=Publish, remaining_len=10
+            0x00, 0x03, 'a' as u8, '/' as u8, 0xc0 as u8, // Topic with Invalid utf8
+            'h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8, // payload
+        ]);
+        assert_eq!(
+            ErrorKind::InvalidData,
+            decode(&mut data).unwrap_err().kind()
+        );
+    }
+
+    /// Validity of remaining_len is tested exhaustively elsewhere, this is for inner lengths, which
+    /// are rarer.
+    #[test]
+    fn inner_length_too_long() {
+        let mut data = BytesMut::from(vec![
+            0b00010000, 20, // Connect packet, remaining_len=20
+            0x00, 0x04, 'M' as u8, 'Q' as u8, 'T' as u8, 'T' as u8, 0x04,
+            0b01000000, // +password
+            0x00, 0x0a, // keepalive 10 sec
+            0x00, 0x04, 't' as u8, 'e' as u8, 's' as u8, 't' as u8, // client_id
+            0x00, 0x03, 'm' as u8, 'q' as u8, // password with invalid length
+        ]);
+        assert_eq!(
+            ErrorKind::InvalidData,
+            decode(&mut data).unwrap_err().kind()
+        );
+    }
 }
diff --git a/src/encoder.rs b/src/encoder.rs
index 72219fa..7429fbc 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -1,69 +1,72 @@
-use crate::{Packet, MAX_PAYLOAD_SIZE};
+use crate::Packet;
 use bytes::{BufMut, BytesMut};
-use std::io;
+use std::io::{Error, ErrorKind};
 
 /// Encode a [Packet] enum into a buffer.
 ///
 /// [Packet]: ../enum.Packet.html
-pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
     match packet {
         Packet::Connect(connect) => connect.to_buffer(buffer),
         Packet::Connack(connack) => connack.to_buffer(buffer),
         Packet::Publish(publish) => publish.to_buffer(buffer),
         Packet::Puback(pid) => {
+            check_remaining(buffer, 4)?;
             let header_u8 = 0b01000000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            pid.to_buffer(buffer);
-            Ok(())
+            pid.to_buffer(buffer)
         }
         Packet::Pubrec(pid) => {
+            check_remaining(buffer, 4)?;
             let header_u8 = 0b01010000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            pid.to_buffer(buffer);
-            Ok(())
+            pid.to_buffer(buffer)
         }
         Packet::Pubrel(pid) => {
+            check_remaining(buffer, 4)?;
             let header_u8 = 0b01100000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            pid.to_buffer(buffer);
-            Ok(())
+            pid.to_buffer(buffer)
         }
         Packet::PubComp(pid) => {
+            check_remaining(buffer, 3)?;
             let header_u8 = 0b01110000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            pid.to_buffer(buffer);
-            Ok(())
+            pid.to_buffer(buffer)
         }
         Packet::Subscribe(subscribe) => subscribe.to_buffer(buffer),
         Packet::SubAck(suback) => suback.to_buffer(buffer),
         Packet::UnSubscribe(unsub) => unsub.to_buffer(buffer),
         Packet::UnSubAck(pid) => {
+            check_remaining(buffer, 4)?;
             let header_u8 = 0b10110000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
-            pid.to_buffer(buffer);
-            Ok(())
+            pid.to_buffer(buffer)
         }
         Packet::PingReq => {
+            check_remaining(buffer, 2)?;
             buffer.put(0b11000000 as u8);
             buffer.put(0b00000000 as u8);
             Ok(())
         }
         Packet::PingResp => {
+            check_remaining(buffer, 2)?;
             buffer.put(0b11010000 as u8);
             buffer.put(0b00000000 as u8);
             Ok(())
         }
         Packet::Disconnect => {
+            check_remaining(buffer, 2)?;
             buffer.put(0b11100000 as u8);
             buffer.put(0b00000000 as u8);
             Ok(())
@@ -71,13 +74,25 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), io::Error> {
     }
 }
 
-pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::Error> {
-    if len > MAX_PAYLOAD_SIZE {
-        return Err(io::Error::new(
-            io::ErrorKind::PermissionDenied,
-            "data size too big",
-        ));
-    };
+/// Check wether buffer has `len` bytes of write capacity left. Use this to return a clean
+/// Result::Err instead of panicking.
+pub(crate) fn check_remaining(buffer: &BytesMut, len: usize) -> Result<(), Error> {
+    if buffer.remaining_mut() < len {
+        Err(Error::new(ErrorKind::WriteZero, "buffer full"))
+    } else {
+        Ok(())
+    }
+}
+
+/// http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718023
+pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), Error> {
+    match len {
+        0..=127 => check_remaining(buffer, len + 1)?,
+        128..=16383 => check_remaining(buffer, len + 2)?,
+        16384..=2097151 => check_remaining(buffer, len + 3)?,
+        2097152..=268435455 => check_remaining(buffer, len + 4)?,
+        _ => return Err(Error::new(ErrorKind::PermissionDenied, "data too big")),
+    }
     let mut done = false;
     let mut x = len;
     while !done {
@@ -92,12 +107,12 @@ pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), io::
     Ok(())
 }
 
-pub(crate) fn write_bytes(bytes: &[u8], buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub(crate) fn write_bytes(bytes: &[u8], buffer: &mut BytesMut) -> Result<(), Error> {
     buffer.put_u16_be(bytes.len() as u16);
     buffer.put_slice(bytes);
     Ok(())
 }
 
-pub(crate) fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), io::Error> {
+pub(crate) fn write_string(string: &str, buffer: &mut BytesMut) -> Result<(), Error> {
     write_bytes(string.as_bytes(), buffer)
 }
diff --git a/src/header.rs b/src/header.rs
index 4533651..ea1e297 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -1,6 +1,6 @@
 use crate::{PacketType, QoS};
 use bytes::{BufMut, BytesMut};
-use std::io;
+use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) struct Header {
@@ -10,7 +10,7 @@ pub(crate) struct Header {
 }
 
 impl Header {
-    pub fn new(hd: u8, len: usize) -> Result<Header, io::Error> {
+    pub fn new(hd: u8, len: usize) -> Result<Header, Error> {
         Ok(Header {
             hd,
             len,
@@ -29,7 +29,7 @@ impl Header {
         (self.hd & 0b1000) != 0
     }
     #[inline]
-    pub fn qos(&self) -> Result<QoS, io::Error> {
+    pub fn qos(&self) -> Result<QoS, Error> {
         QoS::from_hd(self.hd)
     }
     #[inline]
@@ -53,17 +53,17 @@ pub enum Protocol {
     MQIsdp,
 }
 impl Protocol {
-    pub(crate) fn new(name: &str, level: u8) -> Result<Protocol, io::Error> {
+    pub(crate) fn new(name: &str, level: u8) -> Result<Protocol, Error> {
         match (name, level) {
             ("MQIsdp", 3) => Ok(Protocol::MQIsdp),
             ("MQTT", 4) => Ok(Protocol::MQTT311),
-            _ => Err(io::Error::new(
-                io::ErrorKind::InvalidData,
+            _ => Err(Error::new(
+                ErrorKind::InvalidData,
                 format!("Unsupported protocol {:?} {}", name, level),
             )),
         }
     }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
         match self {
             Protocol::MQTT311 => {
                 Ok(buffer.put_slice(&[0u8, 4, 'M' as u8, 'Q' as u8, 'T' as u8, 'T' as u8, 4]))
diff --git a/src/lib.rs b/src/lib.rs
index 9fadec0..5a1b74d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -26,4 +26,3 @@ pub use crate::{
 };
 
 const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
-const MAX_PAYLOAD_SIZE: usize = 268435455;
diff --git a/src/packet.rs b/src/packet.rs
index 9889a41..6256d87 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,5 +1,5 @@
 use crate::{Connack, Connect, PacketIdentifier, Publish, Suback, Subscribe, Unsubscribe};
-use std::io;
+use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum Packet {
@@ -58,7 +58,7 @@ pub enum PacketType {
 }
 impl PacketType {
     #[inline]
-    pub(crate) fn from_hd(hd: u8) -> Result<Self, io::Error> {
+    pub(crate) fn from_hd(hd: u8) -> Result<Self, Error> {
         match hd >> 4 {
             1 => Ok(PacketType::Connect),
             2 => Ok(PacketType::Connack),
@@ -74,8 +74,8 @@ impl PacketType {
             12 => Ok(PacketType::PingReq),
             13 => Ok(PacketType::PingResp),
             14 => Ok(PacketType::Disconnect),
-            _ => Err(io::Error::new(
-                io::ErrorKind::InvalidInput,
+            _ => Err(Error::new(
+                ErrorKind::InvalidInput,
                 "Unsupported packet type",
             )),
         }
diff --git a/src/publish.rs b/src/publish.rs
index 44832dd..62e40f1 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,6 +1,6 @@
 use crate::{decoder::*, encoder::*, header::Header, PacketIdentifier, QoS, QosPid};
 use bytes::{BufMut, BytesMut};
-use std::io;
+use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Publish {
@@ -12,8 +12,8 @@ pub struct Publish {
 }
 
 impl Publish {
-    pub(crate) fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, io::Error> {
-        let topic_name = read_string(buffer);
+    pub(crate) fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, Error> {
+        let topic_name = read_string(buffer)?;
 
         let qospid = match header.qos()? {
             QoS::AtMostOnce => QosPid::AtMostOnce,
@@ -30,7 +30,7 @@ impl Publish {
             payload,
         })
     }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
         // Header
         let mut header_u8: u8 = match self.qospid {
             QosPid::AtMostOnce => 0b00110000,
@@ -43,6 +43,7 @@ impl Publish {
         if self.retain {
             header_u8 |= 0b00000001 as u8;
         };
+        check_remaining(buffer, 1)?;
         buffer.put(header_u8);
 
         // Length: topic (2+len) + pid (0/2) + payload (len)
@@ -60,8 +61,8 @@ impl Publish {
         // Pid
         match self.qospid {
             QosPid::AtMostOnce => (),
-            QosPid::AtLeastOnce(pid) => pid.to_buffer(buffer),
-            QosPid::ExactlyOnce(pid) => pid.to_buffer(buffer),
+            QosPid::AtLeastOnce(pid) => pid.to_buffer(buffer)?,
+            QosPid::ExactlyOnce(pid) => pid.to_buffer(buffer)?,
         }
 
         // Payload
diff --git a/src/subscribe.rs b/src/subscribe.rs
index 89e7fd7..049afe8 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -1,6 +1,6 @@
 use crate::{decoder::*, encoder::*, PacketIdentifier, QoS};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
-use std::io;
+use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct SubscribeTopic {
@@ -41,11 +41,11 @@ pub struct Unsubscribe {
 }
 
 impl Subscribe {
-    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
-            let topic_path = read_string(buffer);
+            let topic_path = read_string(buffer)?;
             let qos = QoS::from_u8(buffer.split_to(1).into_buf().get_u8())?;
             let topic = SubscribeTopic { topic_path, qos };
             topics.push(topic);
@@ -53,8 +53,9 @@ impl Subscribe {
         Ok(Subscribe { pid, topics })
     }
 
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
         let header_u8: u8 = 0b10000010;
+        check_remaining(buffer, 1)?;
         buffer.put(header_u8);
 
         // Length: pid(2) + topic.for_each(2+len + qos(1))
@@ -65,7 +66,7 @@ impl Subscribe {
         write_length(length, buffer)?;
 
         // Pid
-        self.pid.to_buffer(buffer);
+        self.pid.to_buffer(buffer)?;
 
         // Topics
         for topic in &self.topics {
@@ -78,26 +79,27 @@ impl Subscribe {
 }
 
 impl Unsubscribe {
-    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
-            let topic_path = read_string(buffer);
+            let topic_path = read_string(buffer)?;
             topics.push(topic_path);
         }
         Ok(Unsubscribe { pid, topics })
     }
 
-    pub(crate) fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), io::Error>{
+    pub(crate) fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), Error>{
         let header_u8 : u8 = 0b10100010;
         let mut length = 2;
         for topic in &self.topics{
             length += 2 + topic.len();
         }
-
+        check_remaining(buffer, 1)?;
         buffer.put(header_u8);
+
         write_length(length, buffer)?;
-        self.pid.to_buffer(buffer);
+        self.pid.to_buffer(buffer)?;
         for topic in&self.topics{
             write_string(topic.as_ref(), buffer)?;
         }
@@ -106,7 +108,7 @@ impl Unsubscribe {
 }
 
 impl Suback {
-    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
         let pid = PacketIdentifier::from_buffer(buffer)?;
         let mut return_codes: Vec<SubscribeReturnCodes> = Vec::new();
         while buffer.len() != 0 {
@@ -120,13 +122,14 @@ impl Suback {
         }
         Ok(Suback { return_codes, pid })
     }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), io::Error> {
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
         let header_u8: u8 = 0b10010000;
         let length = 2 + self.return_codes.len();
-
+        check_remaining(buffer, 1)?;
         buffer.put(header_u8);
+
         write_length(length, buffer)?;
-        self.pid.to_buffer(buffer);
+        self.pid.to_buffer(buffer)?;
         for rc in &self.return_codes {
             buffer.put(rc.to_u8());
         }
diff --git a/src/utils.rs b/src/utils.rs
index 5d84cfc..395a8d8 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,5 +1,9 @@
+use crate::encoder::check_remaining;
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
-use std::{io, num::NonZeroU16};
+use std::{
+    io::{Error, ErrorKind},
+    num::NonZeroU16,
+};
 
 /// Packet Identifier, for ack purposes.
 ///
@@ -10,21 +14,21 @@ use std::{io, num::NonZeroU16};
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct PacketIdentifier(NonZeroU16);
 impl PacketIdentifier {
-    pub fn new(u: u16) -> Result<Self, io::Error> {
+    pub fn new(u: u16) -> Result<Self, Error> {
         match NonZeroU16::new(u) {
             Some(nz) => Ok(PacketIdentifier(nz)),
-            None => Err(io::Error::new(io::ErrorKind::InvalidData, "Pid == 0")),
+            None => Err(Error::new(ErrorKind::InvalidData, "Pid == 0")),
         }
     }
     pub fn get(self) -> u16 {
         self.0.get()
     }
-    pub(crate) fn from_buffer(buf: &mut BytesMut) -> Result<Self, io::Error> {
+    pub(crate) fn from_buffer(buf: &mut BytesMut) -> Result<Self, Error> {
         Self::new(buf.split_to(2).into_buf().get_u16_be())
     }
-    // FIXME: Result<(), io::Error>
-    pub(crate) fn to_buffer(self, buf: &mut BytesMut) {
-        buf.put_u16_be(self.get())
+    pub(crate) fn to_buffer(self, buf: &mut BytesMut) -> Result<(), Error> {
+        check_remaining(buf, 2)?;
+        Ok(buf.put_u16_be(self.get()))
     }
 }
 
@@ -42,16 +46,16 @@ impl QoS {
             QoS::ExactlyOnce => 2,
         }
     }
-    pub fn from_u8(byte: u8) -> Result<QoS, io::Error> {
+    pub fn from_u8(byte: u8) -> Result<QoS, Error> {
         match byte {
             0 => Ok(QoS::AtMostOnce),
             1 => Ok(QoS::AtLeastOnce),
             2 => Ok(QoS::ExactlyOnce),
-            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "Qos > 2")),
+            _ => Err(Error::new(ErrorKind::InvalidData, "Qos > 2")),
         }
     }
     #[inline]
-    pub(crate) fn from_hd(hd: u8) -> Result<QoS, io::Error> {
+    pub(crate) fn from_hd(hd: u8) -> Result<QoS, Error> {
         Self::from_u8((hd & 0b110) >> 1)
     }
 }
@@ -63,12 +67,12 @@ pub enum QosPid {
     ExactlyOnce(PacketIdentifier),
 }
 impl QosPid {
-    pub fn from_u8u16(qos: u8, pid: u16) -> Result<Self, io::Error> {
+    pub fn from_u8u16(qos: u8, pid: u16) -> Result<Self, Error> {
         match qos {
             0 => Ok(QosPid::AtMostOnce),
             1 => Ok(QosPid::AtLeastOnce(PacketIdentifier::new(pid)?)),
             2 => Ok(QosPid::ExactlyOnce(PacketIdentifier::new(pid)?)),
-            _ => Err(io::Error::new(io::ErrorKind::InvalidData, "Qos > 2")),
+            _ => Err(Error::new(ErrorKind::InvalidData, "Qos > 2")),
         }
     }
 }
@@ -103,7 +107,7 @@ impl ConnectReturnCode {
         }
     }
 
-    pub fn from_u8(byte: u8) -> Result<ConnectReturnCode, io::Error> {
+    pub fn from_u8(byte: u8) -> Result<ConnectReturnCode, Error> {
         match byte {
             0 => Ok(ConnectReturnCode::Accepted),
             1 => Ok(ConnectReturnCode::RefusedProtocolVersion),
@@ -111,7 +115,7 @@ impl ConnectReturnCode {
             3 => Ok(ConnectReturnCode::ServerUnavailable),
             4 => Ok(ConnectReturnCode::BadUsernamePassword),
             5 => Ok(ConnectReturnCode::NotAuthorized),
-            _ => Err(io::Error::new(io::ErrorKind::InvalidInput, "")),
+            _ => Err(Error::new(ErrorKind::InvalidInput, "ConnectReturnCode > 5")),
         }
     }
 }

From 131a865270a2c7125f1281b06f2a1af8eb47805d Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Fri, 18 Oct 2019 16:05:10 +0100
Subject: [PATCH 08/23] Refactor: various qos/pid changes.

* Renamed `PacketIdentifier` to `Pid`.
  Annoyingly verbose compared to other type names.
* Implement Add<u16> and Sub<u16> for Pid.
  Makes implementing `next_pid()` much easyer.
* Added/Improved docstrings.
* Removed remaining `from_u8()`/`to_u8()` from public API.
  User shouldn't need this low-level stuff.
* `QoSPid::from_u8u16()` is now for unit-testing only and panics.

Breaking change.
---
 src/codec_test.rs   |   4 +-
 src/decoder.rs      |  11 +++--
 src/decoder_test.rs |   9 +---
 src/encoder_test.rs |  26 +++++------
 src/header.rs       |   6 ++-
 src/lib.rs          |   2 +-
 src/packet.rs       |  12 ++---
 src/publish.rs      |   6 +--
 src/subscribe.rs    |  22 ++++-----
 src/utils.rs        | 106 +++++++++++++++++++++++++++++++++-----------
 10 files changed, 126 insertions(+), 78 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index da638d7..5bcffeb 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -15,8 +15,8 @@ prop_compose! {
     }
 }
 prop_compose! {
-    fn stg_pid()(pid in 1..std::u16::MAX) -> PacketIdentifier {
-        PacketIdentifier::new(pid).unwrap()
+    fn stg_pid()(pid in 1..std::u16::MAX) -> Pid {
+        Pid::new(pid).unwrap()
     }
 }
 prop_compose! {
diff --git a/src/decoder.rs b/src/decoder.rs
index 7a58441..cf84036 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -35,10 +35,10 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
             &header,
             &mut buffer.split_to(header.len()),
         )?),
-        PacketType::Puback => Packet::Puback(PacketIdentifier::from_buffer(buffer)?),
-        PacketType::Pubrec => Packet::Pubrec(PacketIdentifier::from_buffer(buffer)?),
-        PacketType::Pubrel => Packet::Pubrel(PacketIdentifier::from_buffer(buffer)?),
-        PacketType::PubComp => Packet::PubComp(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::Puback => Packet::Puback(Pid::from_buffer(buffer)?),
+        PacketType::Pubrec => Packet::Pubrec(Pid::from_buffer(buffer)?),
+        PacketType::Pubrel => Packet::Pubrel(Pid::from_buffer(buffer)?),
+        PacketType::PubComp => Packet::PubComp(Pid::from_buffer(buffer)?),
         PacketType::Subscribe => {
             Packet::Subscribe(Subscribe::from_buffer(&mut buffer.split_to(header.len()))?)
         }
@@ -48,7 +48,7 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
         PacketType::UnSubscribe => Packet::UnSubscribe(Unsubscribe::from_buffer(
             &mut buffer.split_to(header.len()),
         )?),
-        PacketType::UnSubAck => Packet::UnSubAck(PacketIdentifier::from_buffer(buffer)?),
+        PacketType::UnSubAck => Packet::UnSubAck(Pid::from_buffer(buffer)?),
     })
 }
 
@@ -102,7 +102,6 @@ pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
     }
 }
 
-
 #[cfg(test)]
 mod test {
     use crate::decode;
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index 2de80d3..93d112d 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -1,9 +1,4 @@
-#![allow(unused_imports)]
-
-use crate::{
-    decoder, Connack, ConnectReturnCode, Packet, PacketIdentifier, QoS, QosPid,
-    SubscribeReturnCodes, SubscribeTopic,
-};
+use crate::*;
 use bytes::BytesMut;
 
 #[test]
@@ -122,7 +117,7 @@ fn test_publish() {
         Some(Packet::Publish(p)) => {
             assert_eq!(p.dup, true);
             assert_eq!(p.retain, true);
-            assert_eq!(p.qospid, QosPid::from_u8u16(2, 10).unwrap());
+            assert_eq!(p.qospid, QosPid::from_u8u16(2, 10));
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index 2297236..e4c9fe1 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -1,10 +1,4 @@
-#[allow(unused_imports)]
-use crate::{
-    decoder, encoder, Connack, Connect, ConnectReturnCode, Packet, PacketIdentifier, Protocol,
-    Publish, QoS, QosPid, Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe,
-};
-
-#[allow(unused_imports)]
+use crate::*;
 use bytes::BytesMut;
 
 #[test]
@@ -50,7 +44,7 @@ fn test_connack() {
 fn test_publish() {
     let packet = Publish {
         dup: false,
-        qospid: QosPid::from_u8u16(2, 10).unwrap(),
+        qospid: QosPid::from_u8u16(2, 10),
         retain: true,
         topic_name: "asdf".to_string(),
         payload: vec!['h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8],
@@ -69,7 +63,7 @@ fn test_publish() {
 
 #[test]
 fn test_puback() {
-    let packet = Packet::Puback(PacketIdentifier::new(19).unwrap());
+    let packet = Packet::Puback(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -83,7 +77,7 @@ fn test_puback() {
 
 #[test]
 fn test_pubrec() {
-    let packet = Packet::Pubrec(PacketIdentifier::new(19).unwrap());
+    let packet = Packet::Pubrec(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -97,7 +91,7 @@ fn test_pubrec() {
 
 #[test]
 fn test_pubrel() {
-    let packet = Packet::Pubrel(PacketIdentifier::new(19).unwrap());
+    let packet = Packet::Pubrel(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -112,7 +106,7 @@ fn test_pubrel() {
 
 #[test]
 fn test_pubcomp() {
-    let packet = Packet::PubComp(PacketIdentifier::new(19).unwrap());
+    let packet = Packet::PubComp(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
@@ -131,7 +125,7 @@ fn test_subscribe() {
         qos: QoS::ExactlyOnce,
     };
     let packet = Subscribe {
-        pid: PacketIdentifier::new(345).unwrap(),
+        pid: Pid::new(345).unwrap(),
         topics: vec![stopic],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -149,7 +143,7 @@ fn test_subscribe() {
 fn test_suback() {
     let return_code = SubscribeReturnCodes::Success(QoS::ExactlyOnce);
     let packet = Suback {
-        pid: PacketIdentifier::new(12321).unwrap(),
+        pid: Pid::new(12321).unwrap(),
         return_codes: vec![return_code],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -164,7 +158,7 @@ fn test_suback() {
 #[test]
 fn test_unsubscribe() {
     let packet = Unsubscribe {
-        pid: PacketIdentifier::new(12321).unwrap(),
+        pid: Pid::new(12321).unwrap(),
         topics: vec!["a/b".to_string()],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -178,7 +172,7 @@ fn test_unsubscribe() {
 
 #[test]
 fn test_unsuback() {
-    let packet = Packet::UnSubAck(PacketIdentifier::new(19).unwrap());
+    let packet = Packet::UnSubAck(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     let _ = encoder::encode(&packet, &mut buffer);
     let decoded = decoder::decode(&mut buffer).unwrap();
diff --git a/src/header.rs b/src/header.rs
index ea1e297..f2efffd 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -38,7 +38,11 @@ impl Header {
     }
 }
 
-/// Protocol version sent at connection.
+/// Protocol version.
+///
+/// Sent in [`Connect`] packet.
+///
+/// [`Connect`]: struct.Connect.html
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Protocol {
     /// [MQTT 3.1.1] is the most commonly implemented version. [MQTT 5] isn't yet supported my by
diff --git a/src/lib.rs b/src/lib.rs
index 5a1b74d..83b414e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -22,7 +22,7 @@ pub use crate::{
     packet::{Packet, PacketType},
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
-    utils::{ConnectReturnCode, LastWill, PacketIdentifier, QoS, QosPid},
+    utils::{ConnectReturnCode, LastWill, Pid, QoS, QosPid},
 };
 
 const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
diff --git a/src/packet.rs b/src/packet.rs
index 6256d87..dbbe481 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,4 +1,4 @@
-use crate::{Connack, Connect, PacketIdentifier, Publish, Suback, Subscribe, Unsubscribe};
+use crate::{Connack, Connect, Pid, Publish, Suback, Subscribe, Unsubscribe};
 use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
@@ -6,14 +6,14 @@ pub enum Packet {
     Connect(Connect),
     Connack(Connack),
     Publish(Publish),
-    Puback(PacketIdentifier),
-    Pubrec(PacketIdentifier),
-    Pubrel(PacketIdentifier),
-    PubComp(PacketIdentifier),
+    Puback(Pid),
+    Pubrec(Pid),
+    Pubrel(Pid),
+    PubComp(Pid),
     Subscribe(Subscribe),
     SubAck(Suback),
     UnSubscribe(Unsubscribe),
-    UnSubAck(PacketIdentifier),
+    UnSubAck(Pid),
     PingReq,
     PingResp,
     Disconnect,
diff --git a/src/publish.rs b/src/publish.rs
index 62e40f1..1666c64 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,4 +1,4 @@
-use crate::{decoder::*, encoder::*, header::Header, PacketIdentifier, QoS, QosPid};
+use crate::{decoder::*, encoder::*, header::Header, Pid, QoS, QosPid};
 use bytes::{BufMut, BytesMut};
 use std::io::Error;
 
@@ -17,8 +17,8 @@ impl Publish {
 
         let qospid = match header.qos()? {
             QoS::AtMostOnce => QosPid::AtMostOnce,
-            QoS::AtLeastOnce => QosPid::AtLeastOnce(PacketIdentifier::from_buffer(buffer)?),
-            QoS::ExactlyOnce => QosPid::ExactlyOnce(PacketIdentifier::from_buffer(buffer)?),
+            QoS::AtLeastOnce => QosPid::AtLeastOnce(Pid::from_buffer(buffer)?),
+            QoS::ExactlyOnce => QosPid::ExactlyOnce(Pid::from_buffer(buffer)?),
         };
 
         let payload = buffer.to_vec();
diff --git a/src/subscribe.rs b/src/subscribe.rs
index 049afe8..1753fd0 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -1,4 +1,4 @@
-use crate::{decoder::*, encoder::*, PacketIdentifier, QoS};
+use crate::{decoder::*, encoder::*, Pid, QoS};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::io::Error;
 
@@ -24,25 +24,25 @@ impl SubscribeReturnCodes {
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Subscribe {
-    pub pid: PacketIdentifier,
+    pub pid: Pid,
     pub topics: Vec<SubscribeTopic>,
 }
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Suback {
-    pub pid: PacketIdentifier,
+    pub pid: Pid,
     pub return_codes: Vec<SubscribeReturnCodes>,
 }
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Unsubscribe {
-    pub pid: PacketIdentifier,
+    pub pid: Pid,
     pub topics: Vec<String>,
 }
 
 impl Subscribe {
     pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
-        let pid = PacketIdentifier::from_buffer(buffer)?;
+        let pid = Pid::from_buffer(buffer)?;
         let mut topics: Vec<SubscribeTopic> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = read_string(buffer)?;
@@ -80,7 +80,7 @@ impl Subscribe {
 
 impl Unsubscribe {
     pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
-        let pid = PacketIdentifier::from_buffer(buffer)?;
+        let pid = Pid::from_buffer(buffer)?;
         let mut topics: Vec<String> = Vec::new();
         while buffer.len() != 0 {
             let topic_path = read_string(buffer)?;
@@ -89,10 +89,10 @@ impl Unsubscribe {
         Ok(Unsubscribe { pid, topics })
     }
 
-    pub(crate) fn to_buffer(&self, buffer: &mut  BytesMut) -> Result<(), Error>{
-        let header_u8 : u8 = 0b10100010;
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
+        let header_u8: u8 = 0b10100010;
         let mut length = 2;
-        for topic in &self.topics{
+        for topic in &self.topics {
             length += 2 + topic.len();
         }
         check_remaining(buffer, 1)?;
@@ -100,7 +100,7 @@ impl Unsubscribe {
 
         write_length(length, buffer)?;
         self.pid.to_buffer(buffer)?;
-        for topic in&self.topics{
+        for topic in &self.topics {
             write_string(topic.as_ref(), buffer)?;
         }
         Ok(())
@@ -109,7 +109,7 @@ impl Unsubscribe {
 
 impl Suback {
     pub(crate) fn from_buffer(buffer: &mut BytesMut) -> Result<Self, Error> {
-        let pid = PacketIdentifier::from_buffer(buffer)?;
+        let pid = Pid::from_buffer(buffer)?;
         let mut return_codes: Vec<SubscribeReturnCodes> = Vec::new();
         while buffer.len() != 0 {
             let code = buffer.split_to(1).into_buf().get_u8();
diff --git a/src/utils.rs b/src/utils.rs
index 395a8d8..e63a706 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -5,18 +5,28 @@ use std::{
     num::NonZeroU16,
 };
 
-/// Packet Identifier, for ack purposes.
+/// Packet Identifier.
+///
+/// For packets with [`QoS::AtLeastOne` or `QoS::ExactlyOnce`] delivery.
+///
+/// ```rust
+/// # use mqttrs::{Pid, Packet};
+/// let pid = Pid::new(42).expect("illegal pid value");
+/// let next_pid = pid + 1;
+/// let pending_acks = std::collections::HashMap::<Pid, Packet>::new();
+/// ```
 ///
 /// The spec ([MQTT-2.3.1-1], [MQTT-2.2.1-3]) disallows a pid of 0.
 ///
+/// [`QoS::AtLeastOne` or `QoS::ExactlyOnce`]: enum.QoS.html
 /// [MQTT-2.3.1-1]: https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718025
 /// [MQTT-2.2.1-3]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901026
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct PacketIdentifier(NonZeroU16);
-impl PacketIdentifier {
+pub struct Pid(NonZeroU16);
+impl Pid {
     pub fn new(u: u16) -> Result<Self, Error> {
         match NonZeroU16::new(u) {
-            Some(nz) => Ok(PacketIdentifier(nz)),
+            Some(nz) => Ok(Pid(nz)),
             None => Err(Error::new(ErrorKind::InvalidData, "Pid == 0")),
         }
     }
@@ -31,22 +41,42 @@ impl PacketIdentifier {
         Ok(buf.put_u16_be(self.get()))
     }
 }
+impl std::ops::Add<u16> for Pid {
+    type Output = Pid;
+    fn add(self, u: u16) -> Pid {
+        let n = self.get().wrapping_add(u);
+        Pid(NonZeroU16::new(if n == 0 { 1 } else { n }).unwrap())
+    }
+}
+impl std::ops::Sub<u16> for Pid {
+    type Output = Pid;
+    fn sub(self, u: u16) -> Pid {
+        let n = self.get().wrapping_sub(u);
+        Pid(NonZeroU16::new(if n == 0 { std::u16::MAX } else { n }).unwrap())
+    }
+}
 
+/// Packet delivery [Quality of Service] level.
+///
+/// [Quality of Service]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718099
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum QoS {
+    /// `QoS 0`. No ack needed.
     AtMostOnce,
+    /// `QoS 1`. One ack needed.
     AtLeastOnce,
+    /// `QoS 2`. Two acks needed.
     ExactlyOnce,
 }
 impl QoS {
-    pub fn to_u8(&self) -> u8 {
+    pub(crate) fn to_u8(&self) -> u8 {
         match *self {
             QoS::AtMostOnce => 0,
             QoS::AtLeastOnce => 1,
             QoS::ExactlyOnce => 2,
         }
     }
-    pub fn from_u8(byte: u8) -> Result<QoS, Error> {
+    pub(crate) fn from_u8(byte: u8) -> Result<QoS, Error> {
         match byte {
             0 => Ok(QoS::AtMostOnce),
             1 => Ok(QoS::AtLeastOnce),
@@ -60,21 +90,57 @@ impl QoS {
     }
 }
 
+/// Combined [`QoS`]/[`Pid`].
+///
+/// Used only in [`Publish`] packets.
+///
+/// [`Publish`]: struct.Publish.html
+/// [`QoS`]: enum.QoS.html
+/// [`Pid`]: struct.Pid.html
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum QosPid {
     AtMostOnce,
-    AtLeastOnce(PacketIdentifier),
-    ExactlyOnce(PacketIdentifier),
+    AtLeastOnce(Pid),
+    ExactlyOnce(Pid),
 }
 impl QosPid {
-    pub fn from_u8u16(qos: u8, pid: u16) -> Result<Self, Error> {
+    #[cfg(test)]
+    pub(crate) fn from_u8u16(qos: u8, pid: u16) -> Self {
         match qos {
-            0 => Ok(QosPid::AtMostOnce),
-            1 => Ok(QosPid::AtLeastOnce(PacketIdentifier::new(pid)?)),
-            2 => Ok(QosPid::ExactlyOnce(PacketIdentifier::new(pid)?)),
-            _ => Err(Error::new(ErrorKind::InvalidData, "Qos > 2")),
+            0 => QosPid::AtMostOnce,
+            1 => QosPid::AtLeastOnce(Pid::new(pid).expect("pid == 0")),
+            2 => QosPid::ExactlyOnce(Pid::new(pid).expect("pid == 0")),
+            _ => panic!("Qos > 2"),
+        }
+    }
+    /// Extract the [`Pid`] from a `QosPid`, if any.
+    ///
+    /// [`Pid`]: struct.Pid.html
+    pub fn pid(self) -> Option<Pid> {
+        match self {
+            QosPid::AtMostOnce => None,
+            QosPid::AtLeastOnce(p) => Some(p),
+            QosPid::ExactlyOnce(p) => Some(p),
         }
     }
+    /// Extract the [`QoS`] from a `QosPid`.
+    ///
+    /// [`QoS`]: enum.QoS.html
+    pub fn qos(self) -> QoS {
+        match self {
+            QosPid::AtMostOnce => QoS::AtMostOnce,
+            QosPid::AtLeastOnce(_) => QoS::AtLeastOnce,
+            QosPid::ExactlyOnce(_) => QoS::ExactlyOnce,
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct LastWill {
+    pub topic: String,
+    pub message: Vec<u8>,
+    pub qos: QoS,
+    pub retain: bool,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -86,17 +152,8 @@ pub enum ConnectReturnCode {
     BadUsernamePassword,
     NotAuthorized,
 }
-
-#[derive(Debug, Clone, PartialEq)]
-pub struct LastWill {
-    pub topic: String,
-    pub message: Vec<u8>,
-    pub qos: QoS,
-    pub retain: bool,
-}
-
 impl ConnectReturnCode {
-    pub fn to_u8(&self) -> u8 {
+    pub(crate) fn to_u8(&self) -> u8 {
         match *self {
             ConnectReturnCode::Accepted => 0,
             ConnectReturnCode::RefusedProtocolVersion => 1,
@@ -106,8 +163,7 @@ impl ConnectReturnCode {
             ConnectReturnCode::NotAuthorized => 5,
         }
     }
-
-    pub fn from_u8(byte: u8) -> Result<ConnectReturnCode, Error> {
+    pub(crate) fn from_u8(byte: u8) -> Result<ConnectReturnCode, Error> {
         match byte {
             0 => Ok(ConnectReturnCode::Accepted),
             1 => Ok(ConnectReturnCode::RefusedProtocolVersion),

From a4c3becd01824c2efa161fa601589b5546225e4f Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Fri, 18 Oct 2019 16:21:55 +0100
Subject: [PATCH 09/23] Remove cargo feature=edition from Cargo.toml.

We don't support rust versions old enough to need this.
---
 Cargo.toml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 8337346..b04802d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,5 +1,3 @@
-cargo-features = ["edition"]
-
 [package]
 name = "mqttrs"
 version = "0.1.4"
@@ -11,7 +9,6 @@ repository = "https://github.com/00imvj00/mqttrs"
 keywords = ["mqtt", "encoding", "decoding", "async", "async-mqtt"]
 license = "Apache-2.0"
 
-
 [dependencies]
 bytes = "0.4"
 

From 344fb3b6063d5c518dc75e1bcaf7936e827190ff Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Fri, 18 Oct 2019 18:51:55 +0100
Subject: [PATCH 10/23] Use consistent casing for every packet types.

This fixes many snake-case inconsistencies, such as `Packet::SubAck(Suback)`, or
`PacketType::Puback` vs `PacketType::SubAck`. The new casing is just uppercase first letter,
following the principle of least surprise.

Breaking change.
---
 src/codec_test.rs   |  12 ++--
 src/decoder.rs      |  14 ++---
 src/decoder_test.rs |  12 ++--
 src/encoder.rs      |  12 ++--
 src/encoder_test.rs | 148 +++++++++++++++++---------------------------
 src/packet.rs       |  48 +++++++-------
 6 files changed, 107 insertions(+), 139 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 5bcffeb..9e373cf 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -96,7 +96,7 @@ prop_compose! {
 }
 prop_compose! {
     fn stg_pubcomp()(pid in stg_pid()) -> Packet {
-        Packet::PubComp(pid)
+        Packet::Pubcomp(pid)
     }
 }
 prop_compose! {
@@ -106,27 +106,27 @@ prop_compose! {
 }
 prop_compose! {
     fn stg_suback()(pid in stg_pid(), return_codes in vec(stg_subretcode(), 0..300)) -> Packet {
-        Packet::SubAck(Suback{pid: pid, return_codes})
+        Packet::Suback(Suback{pid: pid, return_codes})
     }
 }
 prop_compose! {
     fn stg_unsubscribe()(pid in stg_pid(), topics in vec(stg_topic(), 0..20)) -> Packet {
-        Packet::UnSubscribe(Unsubscribe{pid:pid, topics})
+        Packet::Unsubscribe(Unsubscribe{pid:pid, topics})
     }
 }
 prop_compose! {
     fn stg_unsuback()(pid in stg_pid()) -> Packet {
-        Packet::UnSubAck(pid)
+        Packet::Unsuback(pid)
     }
 }
 prop_compose! {
     fn stg_pingreq()(_ in bool::ANY) -> Packet {
-        Packet::PingReq
+        Packet::Pingreq
     }
 }
 prop_compose! {
     fn stg_pingresp()(_ in bool::ANY) -> Packet {
-        Packet::PingResp
+        Packet::Pingresp
     }
 }
 prop_compose! {
diff --git a/src/decoder.rs b/src/decoder.rs
index cf84036..e2957c7 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -22,8 +22,8 @@ pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, Error> {
 
 fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
     Ok(match header.packet() {
-        PacketType::PingReq => Packet::PingReq,
-        PacketType::PingResp => Packet::PingResp,
+        PacketType::Pingreq => Packet::Pingreq,
+        PacketType::Pingresp => Packet::Pingresp,
         PacketType::Disconnect => Packet::Disconnect,
         PacketType::Connect => {
             Packet::Connect(Connect::from_buffer(&mut buffer.split_to(header.len()))?)
@@ -38,17 +38,17 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
         PacketType::Puback => Packet::Puback(Pid::from_buffer(buffer)?),
         PacketType::Pubrec => Packet::Pubrec(Pid::from_buffer(buffer)?),
         PacketType::Pubrel => Packet::Pubrel(Pid::from_buffer(buffer)?),
-        PacketType::PubComp => Packet::PubComp(Pid::from_buffer(buffer)?),
+        PacketType::Pubcomp => Packet::Pubcomp(Pid::from_buffer(buffer)?),
         PacketType::Subscribe => {
             Packet::Subscribe(Subscribe::from_buffer(&mut buffer.split_to(header.len()))?)
         }
-        PacketType::SubAck => {
-            Packet::SubAck(Suback::from_buffer(&mut buffer.split_to(header.len()))?)
+        PacketType::Suback => {
+            Packet::Suback(Suback::from_buffer(&mut buffer.split_to(header.len()))?)
         }
-        PacketType::UnSubscribe => Packet::UnSubscribe(Unsubscribe::from_buffer(
+        PacketType::Unsubscribe => Packet::Unsubscribe(Unsubscribe::from_buffer(
             &mut buffer.split_to(header.len()),
         )?),
-        PacketType::UnSubAck => Packet::UnSubAck(Pid::from_buffer(buffer)?),
+        PacketType::Unsuback => Packet::Unsuback(Pid::from_buffer(buffer)?),
     })
 }
 
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index 93d112d..be48055 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -61,14 +61,14 @@ fn test_connack() {
 fn test_ping_req() {
     let mut data = BytesMut::from(vec![0b11000000, 0b00000000]);
     let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, Some(Packet::PingReq));
+    assert_eq!(d, Some(Packet::Pingreq));
 }
 
 #[test]
 fn test_ping_resp() {
     let mut data = BytesMut::from(vec![0b11010000, 0b00000000]);
     let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, Some(Packet::PingResp));
+    assert_eq!(d, Some(Packet::Pingresp));
 }
 
 #[test]
@@ -160,7 +160,7 @@ fn test_pub_comp() {
     let mut data = BytesMut::from(vec![0b01110000, 0b00000010, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::PubComp(a)) => assert_eq!(a.get(), 10),
+        Some(Packet::Pubcomp(a)) => assert_eq!(a.get(), 10),
         _ => panic!(),
     };
 }
@@ -191,7 +191,7 @@ fn test_suback() {
     let mut data = BytesMut::from(vec![0b10010000, 3, 0 as u8, 10 as u8, 0b00000010]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::SubAck(s)) => {
+        Some(Packet::Suback(s)) => {
             assert_eq!(s.pid.get(), 10);
             assert_eq!(
                 s.return_codes[0],
@@ -209,7 +209,7 @@ fn test_unsubscribe() {
     ]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::UnSubscribe(a)) => {
+        Some(Packet::Unsubscribe(a)) => {
             assert_eq!(a.pid.get(), 10);
             assert_eq!(a.topics[0], 'a'.to_string());
         }
@@ -222,7 +222,7 @@ fn test_unsub_ack() {
     let mut data = BytesMut::from(vec![0b10110000, 2, 0 as u8, 10 as u8]);
     let d = decoder::decode(&mut data).unwrap();
     match d {
-        Some(Packet::UnSubAck(p)) => {
+        Some(Packet::Unsuback(p)) => {
             assert_eq!(p.get(), 10);
         }
         _ => panic!(),
diff --git a/src/encoder.rs b/src/encoder.rs
index 7429fbc..ff9fbff 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -34,7 +34,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
             buffer.put(length);
             pid.to_buffer(buffer)
         }
-        Packet::PubComp(pid) => {
+        Packet::Pubcomp(pid) => {
             check_remaining(buffer, 3)?;
             let header_u8 = 0b01110000 as u8;
             let length = 0b00000010 as u8;
@@ -43,9 +43,9 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
             pid.to_buffer(buffer)
         }
         Packet::Subscribe(subscribe) => subscribe.to_buffer(buffer),
-        Packet::SubAck(suback) => suback.to_buffer(buffer),
-        Packet::UnSubscribe(unsub) => unsub.to_buffer(buffer),
-        Packet::UnSubAck(pid) => {
+        Packet::Suback(suback) => suback.to_buffer(buffer),
+        Packet::Unsubscribe(unsub) => unsub.to_buffer(buffer),
+        Packet::Unsuback(pid) => {
             check_remaining(buffer, 4)?;
             let header_u8 = 0b10110000 as u8;
             let length = 0b00000010 as u8;
@@ -53,13 +53,13 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
             buffer.put(length);
             pid.to_buffer(buffer)
         }
-        Packet::PingReq => {
+        Packet::Pingreq => {
             check_remaining(buffer, 2)?;
             buffer.put(0b11000000 as u8);
             buffer.put(0b00000000 as u8);
             Ok(())
         }
-        Packet::PingResp => {
+        Packet::Pingresp => {
             check_remaining(buffer, 2)?;
             buffer.put(0b11010000 as u8);
             buffer.put(0b00000000 as u8);
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index e4c9fe1..fb7612e 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -13,13 +13,10 @@ fn test_connect() {
         password: None,
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::Connect(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Connect(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&Packet::Connect(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Connect(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -30,13 +27,10 @@ fn test_connack() {
         code: ConnectReturnCode::Accepted,
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::Connack(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Connack(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&Packet::Connack(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Connack(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -50,14 +44,10 @@ fn test_publish() {
         payload: vec!['h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8],
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::Publish(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    println!("{:?}", decoded);
-    match decoded {
-        Some(Packet::Publish(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&Packet::Publish(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Publish(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -65,13 +55,10 @@ fn test_publish() {
 fn test_puback() {
     let packet = Packet::Puback(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&packet, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Puback(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&packet, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Puback(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -79,13 +66,10 @@ fn test_puback() {
 fn test_pubrec() {
     let packet = Packet::Pubrec(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&packet, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Pubrec(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&packet, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Pubrec(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -93,28 +77,21 @@ fn test_pubrec() {
 fn test_pubrel() {
     let packet = Packet::Pubrel(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&packet, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    println!("{:?}", decoded);
-    match decoded {
-        Some(Packet::Pubrel(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&packet, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Pubrel(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
 #[test]
 fn test_pubcomp() {
-    let packet = Packet::PubComp(Pid::new(19).unwrap());
+    let packet = Packet::Pubcomp(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&packet, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::PubComp(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&packet, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Pubcomp(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -129,13 +106,10 @@ fn test_subscribe() {
         topics: vec![stopic],
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::Subscribe(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Subscribe(_c)) => {
-            assert!(true);
-        }
-        _ => assert!(false),
+    encode(&Packet::Subscribe(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Subscribe(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -147,11 +121,10 @@ fn test_suback() {
         return_codes: vec![return_code],
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::SubAck(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::SubAck(_c)) => assert!(true),
-        _ => assert!(false),
+    encode(&Packet::Suback(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Suback(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
@@ -162,55 +135,50 @@ fn test_unsubscribe() {
         topics: vec!["a/b".to_string()],
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::UnSubscribe(packet), &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::UnSubscribe(_c)) => assert!(true),
-        _ => assert!(false),
+    encode(&Packet::Unsubscribe(packet), &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Unsubscribe(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
 #[test]
 fn test_unsuback() {
-    let packet = Packet::UnSubAck(Pid::new(19).unwrap());
+    let packet = Packet::Unsuback(Pid::new(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&packet, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::UnSubAck(_c)) => assert!(true),
-        _ => assert!(false),
+    encode(&packet, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Unsuback(_))) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
 #[test]
 fn test_ping_req() {
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::PingReq, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::PingReq) => assert!(true),
-        _ => assert!(false),
+    encode(&Packet::Pingreq, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Pingreq)) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
 #[test]
 fn test_ping_resp() {
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::PingResp, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::PingResp) => assert!(true),
-        _ => assert!(false),
+    encode(&Packet::Pingresp, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Pingresp)) => assert!(true),
+        err => assert!(false, err),
     }
 }
 
 #[test]
 fn test_disconnect() {
     let mut buffer = BytesMut::with_capacity(1024);
-    let _ = encoder::encode(&Packet::Disconnect, &mut buffer);
-    let decoded = decoder::decode(&mut buffer).unwrap();
-    match decoded {
-        Some(Packet::Disconnect) => assert!(true),
-        _ => assert!(false),
+    encode(&Packet::Disconnect, &mut buffer).unwrap();
+    match decode(&mut buffer) {
+        Ok(Some(Packet::Disconnect)) => assert!(true),
+        err => assert!(false, err),
     }
 }
diff --git a/src/packet.rs b/src/packet.rs
index dbbe481..b29ebc0 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -9,13 +9,13 @@ pub enum Packet {
     Puback(Pid),
     Pubrec(Pid),
     Pubrel(Pid),
-    PubComp(Pid),
+    Pubcomp(Pid),
     Subscribe(Subscribe),
-    SubAck(Suback),
-    UnSubscribe(Unsubscribe),
-    UnSubAck(Pid),
-    PingReq,
-    PingResp,
+    Suback(Suback),
+    Unsubscribe(Unsubscribe),
+    Unsuback(Pid),
+    Pingreq,
+    Pingresp,
     Disconnect,
 }
 impl Packet {
@@ -27,13 +27,13 @@ impl Packet {
             Packet::Puback(_) => PacketType::Puback,
             Packet::Pubrec(_) => PacketType::Pubrec,
             Packet::Pubrel(_) => PacketType::Pubrel,
-            Packet::PubComp(_) => PacketType::PubComp,
+            Packet::Pubcomp(_) => PacketType::Pubcomp,
             Packet::Subscribe(_) => PacketType::Subscribe,
-            Packet::SubAck(_) => PacketType::SubAck,
-            Packet::UnSubscribe(_) => PacketType::UnSubscribe,
-            Packet::UnSubAck(_) => PacketType::UnSubAck,
-            Packet::PingReq => PacketType::PingReq,
-            Packet::PingResp => PacketType::PingResp,
+            Packet::Suback(_) => PacketType::Suback,
+            Packet::Unsubscribe(_) => PacketType::Unsubscribe,
+            Packet::Unsuback(_) => PacketType::Unsuback,
+            Packet::Pingreq => PacketType::Pingreq,
+            Packet::Pingresp => PacketType::Pingresp,
             Packet::Disconnect => PacketType::Disconnect,
         }
     }
@@ -47,13 +47,13 @@ pub enum PacketType {
     Puback,
     Pubrec,
     Pubrel,
-    PubComp,
+    Pubcomp,
     Subscribe,
-    SubAck,
-    UnSubscribe,
-    UnSubAck,
-    PingReq,
-    PingResp,
+    Suback,
+    Unsubscribe,
+    Unsuback,
+    Pingreq,
+    Pingresp,
     Disconnect,
 }
 impl PacketType {
@@ -66,13 +66,13 @@ impl PacketType {
             4 => Ok(PacketType::Puback),
             5 => Ok(PacketType::Pubrec),
             6 => Ok(PacketType::Pubrel),
-            7 => Ok(PacketType::PubComp),
+            7 => Ok(PacketType::Pubcomp),
             8 => Ok(PacketType::Subscribe),
-            9 => Ok(PacketType::SubAck),
-            10 => Ok(PacketType::UnSubscribe),
-            11 => Ok(PacketType::UnSubAck),
-            12 => Ok(PacketType::PingReq),
-            13 => Ok(PacketType::PingResp),
+            9 => Ok(PacketType::Suback),
+            10 => Ok(PacketType::Unsubscribe),
+            11 => Ok(PacketType::Unsuback),
+            12 => Ok(PacketType::Pingreq),
+            13 => Ok(PacketType::Pingresp),
             14 => Ok(PacketType::Disconnect),
             _ => Err(Error::new(
                 ErrorKind::InvalidInput,

From f93af2e6b606fd321dbd86a674cba2cb990e581e Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Mon, 21 Oct 2019 14:28:01 +0100
Subject: [PATCH 11/23] Fix false-negative in remaining_length unittest, remove
 unnecessary encode-time check.

* We weren't sizing the buffer properly in the test.
* The pubrel test was accidentally a puback test (so pubrel encoding bugs wouldn't show up).
* The pubcomp size guard was off by one (but we got back on our feet later thanks to pid's size
  guard).
* As a result of the improved tests, we can now assert that pid's size guard was superfluous, so
  it's gone.
---
 src/codec_test.rs   | 29 +++++++++++++++--------------
 src/decoder_test.rs | 11 ++++-------
 src/encoder.rs      |  2 +-
 src/utils.rs        |  2 --
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 9e373cf..1b84784 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -1,4 +1,5 @@
 use crate::*;
+use bytes::BufMut;
 use bytes::BytesMut;
 use proptest::{bool, collection::vec, num::*, prelude::*};
 use std::io::ErrorKind;
@@ -91,7 +92,7 @@ prop_compose! {
 }
 prop_compose! {
     fn stg_pubrel()(pid in stg_pid()) -> Packet {
-        Packet::Puback(pid)
+        Packet::Pubrel(pid)
     }
 }
 prop_compose! {
@@ -145,14 +146,14 @@ macro_rules! impl_proptests {
             fn $name(pkt in $stg()) {
                 // Encode the packet
                 let mut buf = BytesMut::with_capacity(10240);
-                let res = encoder::encode(&pkt.clone(), &mut buf);
+                let res = encode(&pkt, &mut buf);
                 prop_assert!(res.is_ok(), "encode({:?}) -> {:?}", pkt, res);
                 prop_assert!(buf.len() >= 2, "buffer too small: {:?}", buf); //PING is 2 bytes
                 prop_assert!(buf[0] >> 4 > 0 && buf[0] >> 4 < 16, "bad packet type {:?}", buf);
 
                 // Check that decoding returns the original
-                let mut encoded = buf.clone();
-                let decoded = decoder::decode(&mut buf);
+                let encoded = buf.clone();
+                let decoded = decode(&mut buf);
                 let ok = match &decoded {
                     Ok(Some(p)) if *p == pkt => true,
                     _other => false,
@@ -161,23 +162,23 @@ macro_rules! impl_proptests {
                 prop_assert!(buf.is_empty(), "Buffer not empty: {:?}", buf);
 
                 // Check that decoding a partial packet returns Ok(None)
-                encoded.split_off(encoded.len() - 1);
-                let decoded = decoder::decode(&mut encoded).unwrap();
+                let decoded = decode(&mut encoded.clone().split_off(encoded.len() - 1)).unwrap();
                 prop_assert!(decoded.is_none(), "partial decode {:?} -> {:?}", encoded, decoded);
 
                 // Check that encoding into a small buffer fails cleanly
                 buf.clear();
-                buf.split_off(encoded.len()+1);
-                prop_assert!(encoder::encode(&pkt.clone(), &mut buf).is_ok(), "exact buffer capacity {}", buf.capacity());
-                loop {
+                buf.split_off(encoded.len());
+                prop_assert!(encoded.len() == buf.remaining_mut() && buf.is_empty(),
+                             "Wrong buffer init1 {}/{}/{}", encoded.len(), buf.remaining_mut(), buf.is_empty());
+                prop_assert!(encode(&pkt, &mut buf).is_ok(), "exact buffer capacity {}", buf.capacity());
+                for l in (0..encoded.len()).rev() {
                     buf.clear();
-                    buf.split_off(buf.capacity()-1);
+                    buf.split_to(1);
+                    prop_assert!(l == buf.remaining_mut() && buf.is_empty(),
+                                 "Wrong buffer init2 {}/{}/{}", l, buf.remaining_mut(), buf.is_empty());
                     prop_assert_eq!(ErrorKind::WriteZero,
-                                    encoder::encode(&pkt.clone(), &mut buf).unwrap_err().kind(),
+                                    encode(&pkt, &mut buf).unwrap_err().kind(),
                                     "small buffer capacity {}/{}", buf.capacity(), encoded.len());
-                    if buf.capacity() == 0 {
-                        break;
-                    }
                 }
             }
         }
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index be48055..33cb0f3 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -1,3 +1,4 @@
+//TODO: Should be able to directly `assert_eq!(..., decode(&mut data));` when we switch to our own error type that implements `Eq`.
 use crate::*;
 use bytes::BytesMut;
 
@@ -15,10 +16,8 @@ fn test_half_connect() {
               // 0x00, 0x04, 'r' as u8, 'u' as u8, 's' as u8, 't' as u8, // username = 'rust'
               // 0x00, 0x02, 'm' as u8, 'q' as u8, // password = 'mq'
     ]);
-    let length = data.len();
-    let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, None);
-    assert_eq!(length, 12);
+    assert_eq!(None, decode(&mut data).unwrap());
+    assert_eq!(12, data.len());
 }
 
 #[test]
@@ -34,8 +33,7 @@ fn test_connect() {
         0x00, 0x04, 'r' as u8, 'u' as u8, 's' as u8, 't' as u8, // username = 'rust'
         0x00, 0x02, 'm' as u8, 'q' as u8, // password = 'mq'
     ]);
-    let d = decoder::decode(&mut data).unwrap();
-    assert_ne!(d, None);
+    assert!(decode(&mut data).unwrap().is_some());
     assert_eq!(data.len(), 0);
 }
 
@@ -92,7 +90,6 @@ fn test_publish() {
     let d2 = decoder::decode(&mut data).unwrap();
     let d3 = decoder::decode(&mut data).unwrap();
 
-    println!("{:?}", d1);
     match d1 {
         Some(Packet::Publish(p)) => {
             assert_eq!(p.dup, false);
diff --git a/src/encoder.rs b/src/encoder.rs
index ff9fbff..49d95ed 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -35,7 +35,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
             pid.to_buffer(buffer)
         }
         Packet::Pubcomp(pid) => {
-            check_remaining(buffer, 3)?;
+            check_remaining(buffer, 4)?;
             let header_u8 = 0b01110000 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
diff --git a/src/utils.rs b/src/utils.rs
index e63a706..9f24141 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,4 +1,3 @@
-use crate::encoder::check_remaining;
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::{
     io::{Error, ErrorKind},
@@ -37,7 +36,6 @@ impl Pid {
         Self::new(buf.split_to(2).into_buf().get_u16_be())
     }
     pub(crate) fn to_buffer(self, buf: &mut BytesMut) -> Result<(), Error> {
-        check_remaining(buf, 2)?;
         Ok(buf.put_u16_be(self.get()))
     }
 }

From 9177d04ac192545ee90aec40a84a6ba0a0e14e4e Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Mon, 21 Oct 2019 17:54:12 +0100
Subject: [PATCH 12/23] Add `impl From<Foo> for Packet` for all packet
 subtypes.

It makes instanciating a `Packet` nicer/shorter to write, but the effect is limited because
`encode()` still takes an `&Packet`, so the caller needs to call `.into()` itself. Changing
`encode()` to consume the packet would solve that and would enable us to later have a zerocopy
implementation, but that seems a step too far for this iteration, and it may be handy to be able to
hold on to the original `Packet` after encoding it, for QoS implementaion purposes.
---
 src/connect.rs      |  3 +--
 src/decoder.rs      | 25 ++++++++-----------------
 src/encoder_test.rs |  6 +++---
 src/packet.rs       | 14 +++++++++++++-
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/connect.rs b/src/connect.rs
index 421f56a..0e65662 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -69,7 +69,6 @@ impl Connect {
         })
     }
     pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
-
         let header_u8: u8 = 0b00010000;
         let mut length: usize = 6 + 1 + 1; //NOTE: protocol_name(6) + protocol_level(1) + flags(1);
         let mut connect_flags: u8 = 0b00000000;
@@ -98,7 +97,7 @@ impl Connect {
             length += last_will.topic.len();
             length += 4;
         };
-        check_remaining(buffer, length+1)?;
+        check_remaining(buffer, length + 1)?;
 
         //NOTE: putting data into buffer.
         buffer.put(header_u8);
diff --git a/src/decoder.rs b/src/decoder.rs
index e2957c7..64d1f99 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -25,29 +25,20 @@ fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
         PacketType::Pingreq => Packet::Pingreq,
         PacketType::Pingresp => Packet::Pingresp,
         PacketType::Disconnect => Packet::Disconnect,
-        PacketType::Connect => {
-            Packet::Connect(Connect::from_buffer(&mut buffer.split_to(header.len()))?)
+        PacketType::Connect => Connect::from_buffer(&mut buffer.split_to(header.len()))?.into(),
+        PacketType::Connack => Connack::from_buffer(&mut buffer.split_to(header.len()))?.into(),
+        PacketType::Publish => {
+            Publish::from_buffer(&header, &mut buffer.split_to(header.len()))?.into()
         }
-        PacketType::Connack => {
-            Packet::Connack(Connack::from_buffer(&mut buffer.split_to(header.len()))?)
-        }
-        PacketType::Publish => Packet::Publish(Publish::from_buffer(
-            &header,
-            &mut buffer.split_to(header.len()),
-        )?),
         PacketType::Puback => Packet::Puback(Pid::from_buffer(buffer)?),
         PacketType::Pubrec => Packet::Pubrec(Pid::from_buffer(buffer)?),
         PacketType::Pubrel => Packet::Pubrel(Pid::from_buffer(buffer)?),
         PacketType::Pubcomp => Packet::Pubcomp(Pid::from_buffer(buffer)?),
-        PacketType::Subscribe => {
-            Packet::Subscribe(Subscribe::from_buffer(&mut buffer.split_to(header.len()))?)
-        }
-        PacketType::Suback => {
-            Packet::Suback(Suback::from_buffer(&mut buffer.split_to(header.len()))?)
+        PacketType::Subscribe => Subscribe::from_buffer(&mut buffer.split_to(header.len()))?.into(),
+        PacketType::Suback => Suback::from_buffer(&mut buffer.split_to(header.len()))?.into(),
+        PacketType::Unsubscribe => {
+            Unsubscribe::from_buffer(&mut buffer.split_to(header.len()))?.into()
         }
-        PacketType::Unsubscribe => Packet::Unsubscribe(Unsubscribe::from_buffer(
-            &mut buffer.split_to(header.len()),
-        )?),
         PacketType::Unsuback => Packet::Unsuback(Pid::from_buffer(buffer)?),
     })
 }
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index fb7612e..1fe43c6 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -13,7 +13,7 @@ fn test_connect() {
         password: None,
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    encode(&Packet::Connect(packet), &mut buffer).unwrap();
+    encode(&packet.into(), &mut buffer).unwrap();
     match decode(&mut buffer) {
         Ok(Some(Packet::Connect(_))) => assert!(true),
         err => assert!(false, err),
@@ -27,7 +27,7 @@ fn test_connack() {
         code: ConnectReturnCode::Accepted,
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    encode(&Packet::Connack(packet), &mut buffer).unwrap();
+    encode(&packet.into(), &mut buffer).unwrap();
     match decode(&mut buffer) {
         Ok(Some(Packet::Connack(_))) => assert!(true),
         err => assert!(false, err),
@@ -44,7 +44,7 @@ fn test_publish() {
         payload: vec!['h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8],
     };
     let mut buffer = BytesMut::with_capacity(1024);
-    encode(&Packet::Publish(packet), &mut buffer).unwrap();
+    encode(&packet.into(), &mut buffer).unwrap();
     match decode(&mut buffer) {
         Ok(Some(Packet::Publish(_))) => assert!(true),
         err => assert!(false, err),
diff --git a/src/packet.rs b/src/packet.rs
index b29ebc0..2d3cf53 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,4 +1,4 @@
-use crate::{Connack, Connect, Pid, Publish, Suback, Subscribe, Unsubscribe};
+use crate::*;
 use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
@@ -38,6 +38,18 @@ impl Packet {
         }
     }
 }
+macro_rules! packet_from {
+    ($($t:ident),+) => {
+        $(
+            impl From<$t> for Packet {
+                fn from(p: $t) -> Self {
+                    Packet::$t(p)
+                }
+            }
+        )+
+    }
+}
+packet_from!(Connect, Connack, Publish, Subscribe, Suback, Unsubscribe);
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum PacketType {

From 830836a64f219e8bb7e897ce73f6f032f759112a Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Mon, 21 Oct 2019 18:48:29 +0100
Subject: [PATCH 13/23] Refactor code to return our own error type instead of
 `std::io::Error`.

This is generally considered good practice, it documents the possible erros exhaustively, and makes
matching on errors cleaner. Some unittest have been converted to make use of the new matching
possibilities.
---
 src/codec_test.rs   |   4 +-
 src/connect.rs      |   3 +-
 src/decoder.rs      |  28 +++++-------
 src/decoder_test.rs | 105 ++++++++++++++++++++++----------------------
 src/encoder.rs      |   7 ++-
 src/header.rs       |   8 +---
 src/lib.rs          |   2 +-
 src/packet.rs       |   6 +--
 src/publish.rs      |   3 +-
 src/subscribe.rs    |   3 +-
 src/utils.rs        |  58 ++++++++++++++++++++++--
 11 files changed, 128 insertions(+), 99 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 1b84784..046efb7 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -2,7 +2,6 @@ use crate::*;
 use bytes::BufMut;
 use bytes::BytesMut;
 use proptest::{bool, collection::vec, num::*, prelude::*};
-use std::io::ErrorKind;
 
 // Proptest strategies to generate packet elements
 prop_compose! {
@@ -176,8 +175,7 @@ macro_rules! impl_proptests {
                     buf.split_to(1);
                     prop_assert!(l == buf.remaining_mut() && buf.is_empty(),
                                  "Wrong buffer init2 {}/{}/{}", l, buf.remaining_mut(), buf.is_empty());
-                    prop_assert_eq!(ErrorKind::WriteZero,
-                                    encode(&pkt, &mut buf).unwrap_err().kind(),
+                    prop_assert_eq!(Err(Error::BufferFull), encode(&pkt, &mut buf),
                                     "small buffer capacity {}/{}", buf.capacity(), encoded.len());
                 }
             }
diff --git a/src/connect.rs b/src/connect.rs
index 0e65662..e4e2346 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -1,6 +1,5 @@
-use crate::{decoder::*, encoder::*, ConnectReturnCode, LastWill, Protocol, QoS};
+use crate::{decoder::*, encoder::*, *};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
-use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Connect {
diff --git a/src/decoder.rs b/src/decoder.rs
index 64d1f99..e50a261 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,6 +1,5 @@
 use crate::{header::Header, *};
 use bytes::{Buf, BytesMut, IntoBuf};
-use std::io::{Error, ErrorKind};
 
 /// Decode network bytes into a [Packet] enum.
 ///
@@ -80,24 +79,22 @@ fn read_length(buffer: &BytesMut, mut pos: usize) -> Option<(usize, usize)> {
 }
 
 pub(crate) fn read_string(buffer: &mut BytesMut) -> Result<String, Error> {
-    String::from_utf8(read_bytes(buffer)?)
-        .map_err(|_| Error::new(ErrorKind::InvalidData, "Non-utf8 string"))
+    String::from_utf8(read_bytes(buffer)?).map_err(|e| Error::InvalidString(e.utf8_error()))
 }
 
 pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
-    let length = buffer.split_to(2).into_buf().get_u16_be();
-    if length as usize > buffer.len() {
-        Err(Error::new(ErrorKind::InvalidData, "length > buffer.len()"))
+    let len = buffer.split_to(2).into_buf().get_u16_be() as usize;
+    if len > buffer.len() {
+        Err(Error::InvalidLength(len))
     } else {
-        Ok(buffer.split_to(length as usize).to_vec())
+        Ok(buffer.split_to(len).to_vec())
     }
 }
 
 #[cfg(test)]
 mod test {
-    use crate::decode;
+    use crate::*;
     use bytes::BytesMut;
-    use std::io::ErrorKind;
 
     #[test]
     fn non_utf8_string() {
@@ -106,10 +103,10 @@ mod test {
             0x00, 0x03, 'a' as u8, '/' as u8, 0xc0 as u8, // Topic with Invalid utf8
             'h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8, // payload
         ]);
-        assert_eq!(
-            ErrorKind::InvalidData,
-            decode(&mut data).unwrap_err().kind()
-        );
+        assert!(match decode(&mut data) {
+            Err(Error::InvalidString(_)) => true,
+            _ => false,
+        });
     }
 
     /// Validity of remaining_len is tested exhaustively elsewhere, this is for inner lengths, which
@@ -124,9 +121,6 @@ mod test {
             0x00, 0x04, 't' as u8, 'e' as u8, 's' as u8, 't' as u8, // client_id
             0x00, 0x03, 'm' as u8, 'q' as u8, // password with invalid length
         ]);
-        assert_eq!(
-            ErrorKind::InvalidData,
-            decode(&mut data).unwrap_err().kind()
-        );
+        assert_eq!(Err(Error::InvalidLength(3)), decode(&mut data));
     }
 }
diff --git a/src/decoder_test.rs b/src/decoder_test.rs
index 33cb0f3..45c39e7 100644
--- a/src/decoder_test.rs
+++ b/src/decoder_test.rs
@@ -1,4 +1,3 @@
-//TODO: Should be able to directly `assert_eq!(..., decode(&mut data));` when we switch to our own error type that implements `Eq`.
 use crate::*;
 use bytes::BytesMut;
 
@@ -16,7 +15,7 @@ fn test_half_connect() {
               // 0x00, 0x04, 'r' as u8, 'u' as u8, 's' as u8, 't' as u8, // username = 'rust'
               // 0x00, 0x02, 'm' as u8, 'q' as u8, // password = 'mq'
     ]);
-    assert_eq!(None, decode(&mut data).unwrap());
+    assert_eq!(Ok(None), decode(&mut data));
     assert_eq!(12, data.len());
 }
 
@@ -33,7 +32,21 @@ fn test_connect() {
         0x00, 0x04, 'r' as u8, 'u' as u8, 's' as u8, 't' as u8, // username = 'rust'
         0x00, 0x02, 'm' as u8, 'q' as u8, // password = 'mq'
     ]);
-    assert!(decode(&mut data).unwrap().is_some());
+    let pkt = Connect {
+        protocol: Protocol::MQTT311,
+        keep_alive: 10,
+        client_id: "test".into(),
+        clean_session: true,
+        last_will: Some(LastWill {
+            topic: "/a".into(),
+            message: "offline".into(),
+            qos: QoS::AtLeastOnce,
+            retain: false,
+        }),
+        username: Some("rust".into()),
+        password: Some("mq".into()),
+    };
+    assert_eq!(Ok(Some(pkt.into())), decode(&mut data));
     assert_eq!(data.len(), 0);
 }
 
@@ -58,22 +71,19 @@ fn test_connack() {
 #[test]
 fn test_ping_req() {
     let mut data = BytesMut::from(vec![0b11000000, 0b00000000]);
-    let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, Some(Packet::Pingreq));
+    assert_eq!(Ok(Some(Packet::Pingreq)), decode(&mut data));
 }
 
 #[test]
 fn test_ping_resp() {
     let mut data = BytesMut::from(vec![0b11010000, 0b00000000]);
-    let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, Some(Packet::Pingresp));
+    assert_eq!(Ok(Some(Packet::Pingresp)), decode(&mut data));
 }
 
 #[test]
 fn test_disconnect() {
     let mut data = BytesMut::from(vec![0b11100000, 0b00000000]);
-    let d = decoder::decode(&mut data).unwrap();
-    assert_eq!(d, Some(Packet::Disconnect));
+    assert_eq!(Ok(Some(Packet::Disconnect)), decode(&mut data));
 }
 
 #[test]
@@ -86,79 +96,72 @@ fn test_publish() {
         0b00111101, 12, 0x00, 0x03, 'a' as u8, '/' as u8, 'b' as u8, 0 as u8, 10 as u8, 'h' as u8,
         'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8,
     ]);
-    let d1 = decoder::decode(&mut data).unwrap();
-    let d2 = decoder::decode(&mut data).unwrap();
-    let d3 = decoder::decode(&mut data).unwrap();
 
-    match d1 {
-        Some(Packet::Publish(p)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Publish(p))) => {
             assert_eq!(p.dup, false);
             assert_eq!(p.retain, false);
             assert_eq!(p.qospid, QosPid::AtMostOnce);
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
-        _ => panic!("Should not be None"),
+        other => panic!("Failed decode: {:?}", other),
     }
-    match d2 {
-        Some(Packet::Publish(p)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Publish(p))) => {
             assert_eq!(p.dup, true);
             assert_eq!(p.retain, false);
             assert_eq!(p.qospid, QosPid::AtMostOnce);
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
-        _ => panic!("Should not be None"),
+        other => panic!("Failed decode: {:?}", other),
     }
-    match d3 {
-        Some(Packet::Publish(p)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Publish(p))) => {
             assert_eq!(p.dup, true);
             assert_eq!(p.retain, true);
             assert_eq!(p.qospid, QosPid::from_u8u16(2, 10));
             assert_eq!(p.topic_name, "a/b");
             assert_eq!(String::from_utf8(p.payload).unwrap(), "hello");
         }
-        _ => panic!("Should not be None"),
+        other => panic!("Failed decode: {:?}", other),
     }
 }
 
 #[test]
 fn test_pub_ack() {
     let mut data = BytesMut::from(vec![0b01000000, 0b00000010, 0 as u8, 10 as u8]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Puback(a)) => assert_eq!(a.get(), 10),
-        _ => panic!(),
+    match decode(&mut data) {
+        Ok(Some(Packet::Puback(a))) => assert_eq!(a.get(), 10),
+        other => panic!("Failed decode: {:?}", other),
     };
 }
 
 #[test]
 fn test_pub_rec() {
     let mut data = BytesMut::from(vec![0b01010000, 0b00000010, 0 as u8, 10 as u8]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Pubrec(a)) => assert_eq!(a.get(), 10),
-        _ => panic!(),
+    match decode(&mut data) {
+        Ok(Some(Packet::Pubrec(a))) => assert_eq!(a.get(), 10),
+        other => panic!("Failed decode: {:?}", other),
     };
 }
 
 #[test]
 fn test_pub_rel() {
     let mut data = BytesMut::from(vec![0b01100010, 0b00000010, 0 as u8, 10 as u8]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Pubrel(a)) => assert_eq!(a.get(), 10),
-        _ => panic!(),
+    match decode(&mut data) {
+        Ok(Some(Packet::Pubrel(a))) => assert_eq!(a.get(), 10),
+        other => panic!("Failed decode: {:?}", other),
     };
 }
 
 #[test]
 fn test_pub_comp() {
     let mut data = BytesMut::from(vec![0b01110000, 0b00000010, 0 as u8, 10 as u8]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Pubcomp(a)) => assert_eq!(a.get(), 10),
-        _ => panic!(),
+    match decode(&mut data) {
+        Ok(Some(Packet::Pubcomp(a))) => assert_eq!(a.get(), 10),
+        other => panic!("Failed decode: {:?}", other),
     };
 }
 
@@ -168,9 +171,8 @@ fn test_subscribe() {
         0b10000010, 8, 0 as u8, 10 as u8, 0 as u8, 3 as u8, 'a' as u8, '/' as u8, 'b' as u8,
         0 as u8,
     ]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Subscribe(s)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Subscribe(s))) => {
             assert_eq!(s.pid.get(), 10);
             let t = SubscribeTopic {
                 topic_path: "a/b".to_string(),
@@ -178,7 +180,7 @@ fn test_subscribe() {
             };
             assert_eq!(s.topics[0], t);
         }
-        _ => panic!(),
+        other => panic!("Failed decode: {:?}", other),
     }
 }
 
@@ -186,16 +188,15 @@ fn test_subscribe() {
 
 fn test_suback() {
     let mut data = BytesMut::from(vec![0b10010000, 3, 0 as u8, 10 as u8, 0b00000010]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Suback(s)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Suback(s))) => {
             assert_eq!(s.pid.get(), 10);
             assert_eq!(
                 s.return_codes[0],
                 SubscribeReturnCodes::Success(QoS::ExactlyOnce)
             );
         }
-        _ => panic!(),
+        other => panic!("Failed decode: {:?}", other),
     }
 }
 
@@ -204,24 +205,22 @@ fn test_unsubscribe() {
     let mut data = BytesMut::from(vec![
         0b10100010, 5, 0 as u8, 10 as u8, 0 as u8, 1 as u8, 'a' as u8,
     ]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Unsubscribe(a)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Unsubscribe(a))) => {
             assert_eq!(a.pid.get(), 10);
             assert_eq!(a.topics[0], 'a'.to_string());
         }
-        _ => panic!(),
+        other => panic!("Failed decode: {:?}", other),
     }
 }
 
 #[test]
 fn test_unsub_ack() {
     let mut data = BytesMut::from(vec![0b10110000, 2, 0 as u8, 10 as u8]);
-    let d = decoder::decode(&mut data).unwrap();
-    match d {
-        Some(Packet::Unsuback(p)) => {
+    match decode(&mut data) {
+        Ok(Some(Packet::Unsuback(p))) => {
             assert_eq!(p.get(), 10);
         }
-        _ => panic!(),
+        other => panic!("Failed decode: {:?}", other),
     }
 }
diff --git a/src/encoder.rs b/src/encoder.rs
index 49d95ed..0390f72 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -1,6 +1,5 @@
-use crate::Packet;
+use crate::{Error, Packet};
 use bytes::{BufMut, BytesMut};
-use std::io::{Error, ErrorKind};
 
 /// Encode a [Packet] enum into a buffer.
 ///
@@ -78,7 +77,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
 /// Result::Err instead of panicking.
 pub(crate) fn check_remaining(buffer: &BytesMut, len: usize) -> Result<(), Error> {
     if buffer.remaining_mut() < len {
-        Err(Error::new(ErrorKind::WriteZero, "buffer full"))
+        Err(Error::BufferFull)
     } else {
         Ok(())
     }
@@ -91,7 +90,7 @@ pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), Erro
         128..=16383 => check_remaining(buffer, len + 2)?,
         16384..=2097151 => check_remaining(buffer, len + 3)?,
         2097152..=268435455 => check_remaining(buffer, len + 4)?,
-        _ => return Err(Error::new(ErrorKind::PermissionDenied, "data too big")),
+        _ => return Err(Error::InvalidLength(len)),
     }
     let mut done = false;
     let mut x = len;
diff --git a/src/header.rs b/src/header.rs
index f2efffd..0f78543 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -1,6 +1,5 @@
-use crate::{PacketType, QoS};
+use crate::{Error, PacketType, QoS};
 use bytes::{BufMut, BytesMut};
-use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) struct Header {
@@ -61,10 +60,7 @@ impl Protocol {
         match (name, level) {
             ("MQIsdp", 3) => Ok(Protocol::MQIsdp),
             ("MQTT", 4) => Ok(Protocol::MQTT311),
-            _ => Err(Error::new(
-                ErrorKind::InvalidData,
-                format!("Unsupported protocol {:?} {}", name, level),
-            )),
+            _ => Err(Error::InvalidProtocol(name.into(), level)),
         }
     }
     pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
diff --git a/src/lib.rs b/src/lib.rs
index 83b414e..4f38e14 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -22,7 +22,7 @@ pub use crate::{
     packet::{Packet, PacketType},
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
-    utils::{ConnectReturnCode, LastWill, Pid, QoS, QosPid},
+    utils::{Error, ConnectReturnCode, LastWill, Pid, QoS, QosPid},
 };
 
 const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
diff --git a/src/packet.rs b/src/packet.rs
index 2d3cf53..522db09 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,5 +1,4 @@
 use crate::*;
-use std::io::{Error, ErrorKind};
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum Packet {
@@ -86,10 +85,7 @@ impl PacketType {
             12 => Ok(PacketType::Pingreq),
             13 => Ok(PacketType::Pingresp),
             14 => Ok(PacketType::Disconnect),
-            _ => Err(Error::new(
-                ErrorKind::InvalidInput,
-                "Unsupported packet type",
-            )),
+            n => Err(Error::InvalidPacket(n)),
         }
     }
 }
diff --git a/src/publish.rs b/src/publish.rs
index 1666c64..bd3d9c7 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,6 +1,5 @@
-use crate::{decoder::*, encoder::*, header::Header, Pid, QoS, QosPid};
+use crate::{decoder::*, encoder::*, header::Header, *};
 use bytes::{BufMut, BytesMut};
-use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Publish {
diff --git a/src/subscribe.rs b/src/subscribe.rs
index 1753fd0..f094f93 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -1,6 +1,5 @@
-use crate::{decoder::*, encoder::*, Pid, QoS};
+use crate::{decoder::*, encoder::*, *};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
-use std::io::Error;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct SubscribeTopic {
diff --git a/src/utils.rs b/src/utils.rs
index 9f24141..79de422 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,9 +1,59 @@
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 use std::{
-    io::{Error, ErrorKind},
+    error::Error as ErrorTrait,
+    fmt,
+    io::{Error as IoError, ErrorKind},
     num::NonZeroU16,
 };
 
+/// Errors returned by [`encode()`] and [`decode()`].
+///
+/// [`encode()`]: fn.encode.html
+/// [`decode()`]: fn.decode.html
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum Error {
+    /// Not enough data in the read buffer.
+    ///
+    /// Do not treat this as a fatal error. Read more data into the buffer and try `decode()` again.
+    BufferIncomplete,
+    /// Not enough space in the write buffer.
+    ///
+    /// It is the caller's responsiblity to pass a big enough buffer to `encode()`.
+    BufferFull,
+    /// Tried to encode or decode a ProcessIdentifier==0.
+    InvalidPid,
+    /// Tried to decode a QoS > 2.
+    InvalidQos(u8),
+    /// Tried to decode a ConnectReturnCode > 5.
+    InvalidConnectReturnCode(u8),
+    /// Tried to decode an unknown protocol.
+    InvalidProtocol(String, u8),
+    /// Tried to decode an invalid packet type for this protocol.
+    InvalidPacket(u8),
+    /// Trying to encode/decode an invalid length.
+    ///
+    /// The difference with `BufferFull`/`BufferIncomplete` is that it refers to an invalid/corrupt
+    /// length rather than a buffer size issue.
+    InvalidLength(usize),
+    /// Trying to decode a non-utf8 string.
+    InvalidString(std::str::Utf8Error),
+}
+impl ErrorTrait for Error {}
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{:?}", self)
+    }
+}
+impl From<Error> for IoError {
+    fn from(err: Error) -> IoError {
+        match err {
+            Error::BufferFull => IoError::new(ErrorKind::WriteZero, err),
+            Error::BufferIncomplete => IoError::new(ErrorKind::UnexpectedEof, err),
+            _ => IoError::new(ErrorKind::InvalidData, err),
+        }
+    }
+}
+
 /// Packet Identifier.
 ///
 /// For packets with [`QoS::AtLeastOne` or `QoS::ExactlyOnce`] delivery.
@@ -26,7 +76,7 @@ impl Pid {
     pub fn new(u: u16) -> Result<Self, Error> {
         match NonZeroU16::new(u) {
             Some(nz) => Ok(Pid(nz)),
-            None => Err(Error::new(ErrorKind::InvalidData, "Pid == 0")),
+            None => Err(Error::InvalidPid),
         }
     }
     pub fn get(self) -> u16 {
@@ -79,7 +129,7 @@ impl QoS {
             0 => Ok(QoS::AtMostOnce),
             1 => Ok(QoS::AtLeastOnce),
             2 => Ok(QoS::ExactlyOnce),
-            _ => Err(Error::new(ErrorKind::InvalidData, "Qos > 2")),
+            n => Err(Error::InvalidQos(n)),
         }
     }
     #[inline]
@@ -169,7 +219,7 @@ impl ConnectReturnCode {
             3 => Ok(ConnectReturnCode::ServerUnavailable),
             4 => Ok(ConnectReturnCode::BadUsernamePassword),
             5 => Ok(ConnectReturnCode::NotAuthorized),
-            _ => Err(Error::new(ErrorKind::InvalidInput, "ConnectReturnCode > 5")),
+            n => Err(Error::InvalidConnectReturnCode(n)),
         }
     }
 }

From 2fd878f35a3f9be22031f32f617a0cdbf9198470 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Tue, 22 Oct 2019 11:48:12 +0100
Subject: [PATCH 14/23] Rename `{BufferFull,BufferIncomplete}` to
 `{WriteZero,UnexpectedEof}`.

Matches the `std::io::ErrorKind` names, hopefully improving programmer familiarity.
---
 src/codec_test.rs | 2 +-
 src/encoder.rs    | 2 +-
 src/utils.rs      | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 046efb7..617745b 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -175,7 +175,7 @@ macro_rules! impl_proptests {
                     buf.split_to(1);
                     prop_assert!(l == buf.remaining_mut() && buf.is_empty(),
                                  "Wrong buffer init2 {}/{}/{}", l, buf.remaining_mut(), buf.is_empty());
-                    prop_assert_eq!(Err(Error::BufferFull), encode(&pkt, &mut buf),
+                    prop_assert_eq!(Err(Error::WriteZero), encode(&pkt, &mut buf),
                                     "small buffer capacity {}/{}", buf.capacity(), encoded.len());
                 }
             }
diff --git a/src/encoder.rs b/src/encoder.rs
index 0390f72..3908476 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -77,7 +77,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
 /// Result::Err instead of panicking.
 pub(crate) fn check_remaining(buffer: &BytesMut, len: usize) -> Result<(), Error> {
     if buffer.remaining_mut() < len {
-        Err(Error::BufferFull)
+        Err(Error::WriteZero)
     } else {
         Ok(())
     }
diff --git a/src/utils.rs b/src/utils.rs
index 79de422..042d40a 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -15,11 +15,11 @@ pub enum Error {
     /// Not enough data in the read buffer.
     ///
     /// Do not treat this as a fatal error. Read more data into the buffer and try `decode()` again.
-    BufferIncomplete,
+    UnexpectedEof,
     /// Not enough space in the write buffer.
     ///
     /// It is the caller's responsiblity to pass a big enough buffer to `encode()`.
-    BufferFull,
+    WriteZero,
     /// Tried to encode or decode a ProcessIdentifier==0.
     InvalidPid,
     /// Tried to decode a QoS > 2.
@@ -47,8 +47,8 @@ impl fmt::Display for Error {
 impl From<Error> for IoError {
     fn from(err: Error) -> IoError {
         match err {
-            Error::BufferFull => IoError::new(ErrorKind::WriteZero, err),
-            Error::BufferIncomplete => IoError::new(ErrorKind::UnexpectedEof, err),
+            Error::WriteZero => IoError::new(ErrorKind::WriteZero, err),
+            Error::UnexpectedEof => IoError::new(ErrorKind::UnexpectedEof, err),
             _ => IoError::new(ErrorKind::InvalidData, err),
         }
     }

From 89f5c73705ee156ffd13316dc1eb20996b6eaf4c Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Tue, 22 Oct 2019 11:50:26 +0100
Subject: [PATCH 15/23] `impl From<std::io::Error> for mqttrs::Error`.

This is required by some libraries, for example `tokio::codec::Decoder`. The conversion is lossy,
but there's not much we can do about that. We're storing `(ErrorKind, String)` instead of simply
`IoError` so that `mqttrs::Error` can remain `Clone` and `Eq`.
---
 src/utils.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/utils.rs b/src/utils.rs
index 042d40a..ba00b46 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -37,6 +37,10 @@ pub enum Error {
     InvalidLength(usize),
     /// Trying to decode a non-utf8 string.
     InvalidString(std::str::Utf8Error),
+    /// Catch-all error when converting from `std::io::Error`.
+    ///
+    /// You'll hopefully never see this.
+    IoError(ErrorKind, String),
 }
 impl ErrorTrait for Error {}
 impl fmt::Display for Error {
@@ -53,6 +57,15 @@ impl From<Error> for IoError {
         }
     }
 }
+impl From<IoError> for Error {
+    fn from(err: IoError) -> Error {
+        match err.kind() {
+            ErrorKind::WriteZero => Error::WriteZero,
+            ErrorKind::UnexpectedEof => Error::UnexpectedEof,
+            k => Error::IoError(k, format!("{}",err)),
+        }
+    }
+}
 
 /// Packet Identifier.
 ///

From c3e90f54cfd801d3d7eaa75d559211f9e1576c16 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 11:29:04 +0100
Subject: [PATCH 16/23] Fix various header-{en,de}coding issues.

* Correctly set qos flag when encoding a Pubrel.
* Do not panic when buffer contains a partial remaining_len.
* More coherent InvalidHeader/InvalidLength errors.
* Strictly check the flags when decoding the header byte.
* Only decode the header byte when we know we have a full packet.

This started as a quick fix for a bug found while writing docs, and ended up as a near-rewrite of
the header parsing. The resulting code is smaller (although two new large unttests result in a net
gain) and hopefully more readable and efficient.
---
 src/decoder.rs | 163 +++++++++++++++++++++++++++++++++----------------
 src/encoder.rs |   4 +-
 src/header.rs  |  68 +++++++++------------
 src/lib.rs     |   2 -
 src/packet.rs  |  22 -------
 src/publish.rs |   6 +-
 src/utils.rs   |  11 ++--
 7 files changed, 149 insertions(+), 127 deletions(-)

diff --git a/src/decoder.rs b/src/decoder.rs
index e50a261..9c9c0d7 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -5,77 +5,60 @@ use bytes::{Buf, BytesMut, IntoBuf};
 ///
 /// [Packet]: ../enum.Packet.html
 pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, Error> {
-    if let Some((header, header_size)) = read_header(buffer) {
-        if buffer.len() >= header.len() + header_size {
-            //NOTE: Check if buffer has, header bytes + remaining length bytes in buffer.
-            buffer.split_to(header_size); //NOTE: Remove header bytes from buffer.
-            let p = read_packet(header, buffer)?; //NOTE: Read remaining packet.
-            Ok(Some(p))
-        } else {
-            Ok(None)
-        }
+    if let Some((header, remaining_len)) = read_header(buffer)? {
+        // Advance the buffer position to the next packet, and parse the current packet
+        let p = &mut buffer.split_to(remaining_len);
+        Ok(Some(read_packet(header, p)?))
     } else {
+        // Don't have a full packet
         Ok(None)
     }
 }
 
 fn read_packet(header: Header, buffer: &mut BytesMut) -> Result<Packet, Error> {
-    Ok(match header.packet() {
+    Ok(match header.typ {
         PacketType::Pingreq => Packet::Pingreq,
         PacketType::Pingresp => Packet::Pingresp,
         PacketType::Disconnect => Packet::Disconnect,
-        PacketType::Connect => Connect::from_buffer(&mut buffer.split_to(header.len()))?.into(),
-        PacketType::Connack => Connack::from_buffer(&mut buffer.split_to(header.len()))?.into(),
-        PacketType::Publish => {
-            Publish::from_buffer(&header, &mut buffer.split_to(header.len()))?.into()
-        }
+        PacketType::Connect => Connect::from_buffer(buffer)?.into(),
+        PacketType::Connack => Connack::from_buffer(buffer)?.into(),
+        PacketType::Publish => Publish::from_buffer(&header, buffer)?.into(),
         PacketType::Puback => Packet::Puback(Pid::from_buffer(buffer)?),
         PacketType::Pubrec => Packet::Pubrec(Pid::from_buffer(buffer)?),
         PacketType::Pubrel => Packet::Pubrel(Pid::from_buffer(buffer)?),
         PacketType::Pubcomp => Packet::Pubcomp(Pid::from_buffer(buffer)?),
-        PacketType::Subscribe => Subscribe::from_buffer(&mut buffer.split_to(header.len()))?.into(),
-        PacketType::Suback => Suback::from_buffer(&mut buffer.split_to(header.len()))?.into(),
-        PacketType::Unsubscribe => {
-            Unsubscribe::from_buffer(&mut buffer.split_to(header.len()))?.into()
-        }
+        PacketType::Subscribe => Subscribe::from_buffer(buffer)?.into(),
+        PacketType::Suback => Suback::from_buffer(buffer)?.into(),
+        PacketType::Unsubscribe => Unsubscribe::from_buffer(buffer)?.into(),
         PacketType::Unsuback => Packet::Unsuback(Pid::from_buffer(buffer)?),
     })
 }
 
-/// Read the header of the stream
-fn read_header(buffer: &mut BytesMut) -> Option<(Header, usize)> {
-    if buffer.len() > 1 {
-        let header_u8 = buffer.get(0).unwrap();
-        if let Some((length, size)) = read_length(buffer, 1) {
-            let header = Header::new(*header_u8, length).unwrap();
-            Some((header, size + 1))
-        } else {
-            None
-        }
-    } else {
-        None
-    }
-}
-
-fn read_length(buffer: &BytesMut, mut pos: usize) -> Option<(usize, usize)> {
-    let mut mult: usize = 1;
+/// Read the parsed header and remaining_len from the buffer. Only return Some() and advance the
+/// buffer position if there is enough data in th ebuffer to read the full packet.
+fn read_header(buffer: &mut BytesMut) -> Result<Option<(Header, usize)>, Error> {
     let mut len: usize = 0;
-    let mut done = false;
-
-    while !done {
-        let byte = (*buffer.get(pos).unwrap()) as usize;
-        len += (byte & 0x7F) * mult;
-        mult *= 0x80;
-        if mult > MULTIPLIER {
-            return None;
-        }
-        if (byte & 0x80) == 0 {
-            done = true;
+    for pos in 0..=3 {
+        if let Some(&byte) = buffer.get(pos + 1) {
+            len += (byte as usize & 0x7F) << (pos * 7);
+            if (byte & 0x80) == 0 {
+                // Continuation bit == 0, length is parsed
+                if buffer.len() < 2 + pos + len {
+                    // Won't be able to read full packet
+                    return Ok(None);
+                }
+                // Parse header byte, skip past the header, and return
+                let header = Header::new(*buffer.get(0).unwrap())?;
+                buffer.advance(pos + 2);
+                return Ok(Some((header, len)));
+            }
         } else {
-            pos += 1;
+            // Couldn't read full length
+            return Ok(None);
         }
     }
-    Some((len as usize, pos))
+    // Continuation byte == 1 four times, that's illegal.
+    Err(Error::InvalidHeader)
 }
 
 pub(crate) fn read_string(buffer: &mut BytesMut) -> Result<String, Error> {
@@ -85,7 +68,7 @@ pub(crate) fn read_string(buffer: &mut BytesMut) -> Result<String, Error> {
 pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
     let len = buffer.split_to(2).into_buf().get_u16_be() as usize;
     if len > buffer.len() {
-        Err(Error::InvalidLength(len))
+        Err(Error::InvalidLength)
     } else {
         Ok(buffer.split_to(len).to_vec())
     }
@@ -93,9 +76,83 @@ pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
 
 #[cfg(test)]
 mod test {
-    use crate::*;
+    use crate::{decoder::read_header, header::Header, *};
     use bytes::BytesMut;
 
+    macro_rules! header {
+        ($t:ident, $d:expr, $q:ident, $r:expr) => {
+            Header {
+                typ: PacketType::$t,
+                dup: $d,
+                qos: QoS::$q,
+                retain: $r,
+            }
+        };
+    }
+
+    /// Test all possible header first byte, using remaining_len=0.
+    #[test]
+    fn header_firstbyte() {
+        let valid = vec![
+            (0b0001_0000, header!(Connect, false, AtMostOnce, false)),
+            (0b0010_0000, header!(Connack, false, AtMostOnce, false)),
+            (0b0011_0000, header!(Publish, false, AtMostOnce, false)),
+            (0b0011_0001, header!(Publish, false, AtMostOnce, true)),
+            (0b0011_0010, header!(Publish, false, AtLeastOnce, false)),
+            (0b0011_0011, header!(Publish, false, AtLeastOnce, true)),
+            (0b0011_0100, header!(Publish, false, ExactlyOnce, false)),
+            (0b0011_0101, header!(Publish, false, ExactlyOnce, true)),
+            (0b0011_1000, header!(Publish, true, AtMostOnce, false)),
+            (0b0011_1001, header!(Publish, true, AtMostOnce, true)),
+            (0b0011_1010, header!(Publish, true, AtLeastOnce, false)),
+            (0b0011_1011, header!(Publish, true, AtLeastOnce, true)),
+            (0b0011_1100, header!(Publish, true, ExactlyOnce, false)),
+            (0b0011_1101, header!(Publish, true, ExactlyOnce, true)),
+            (0b0100_0000, header!(Puback, false, AtMostOnce, false)),
+            (0b0101_0000, header!(Pubrec, false, AtMostOnce, false)),
+            (0b0110_0010, header!(Pubrel, false, AtLeastOnce, false)),
+            (0b0111_0000, header!(Pubcomp, false, AtMostOnce, false)),
+            (0b1000_0010, header!(Subscribe, false, AtLeastOnce, false)),
+            (0b1001_0000, header!(Suback, false, AtMostOnce, false)),
+            (0b1010_0010, header!(Unsubscribe, false, AtLeastOnce, false)),
+            (0b1011_0000, header!(Unsuback, false, AtMostOnce, false)),
+            (0b1100_0000, header!(Pingreq, false, AtMostOnce, false)),
+            (0b1101_0000, header!(Pingresp, false, AtMostOnce, false)),
+            (0b1110_0000, header!(Disconnect, false, AtMostOnce, false)),
+        ];
+        for n in 0..=255 {
+            let res = match valid.iter().find(|(byte, _)| *byte == n) {
+                Some((_, header)) => Ok(Some((*header, 0))),
+                None if ((n & 0b110) == 0b110) && (n >> 4 == 3) => Err(Error::InvalidQos(3)),
+                None => Err(Error::InvalidHeader),
+            };
+            let buf = &mut BytesMut::from(vec![n, 0]);
+            assert_eq!(res, read_header(buf), "{:08b}", n);
+        }
+    }
+
+    /// Test decoding of length and actual buffer len.
+    #[rustfmt::skip]
+    #[test]
+    fn header_len() {
+        let h = header!(Connect, false, AtMostOnce, false);
+        for (res, bytes, buflen) in vec![
+            (Ok(Some((h, 0))),          vec![1 << 4, 0],   2),
+            (Ok(None),                  vec![1 << 4, 127], 128),
+            (Ok(Some((h, 127))),        vec![1 << 4, 127], 129),
+            (Ok(None),                  vec![1 << 4, 0x80], 2),
+            (Ok(Some((h, 0))),          vec![1 << 4, 0x80, 0], 3), //Weird encoding for "0" buf matches spec
+            (Ok(Some((h, 128))),        vec![1 << 4, 0x80, 1], 131),
+            (Ok(None),                  vec![1 << 4, 0x80+16, 78], 10002),
+            (Ok(Some((h, 10000))),      vec![1 << 4, 0x80+16, 78], 10003),
+            (Err(Error::InvalidHeader), vec![1 << 4, 0x80, 0x80, 0x80, 0x80], 10),
+        ] {
+            let mut buf = BytesMut::from(bytes);
+            buf.resize(buflen, 0);
+            assert_eq!(res, read_header(&mut buf));
+        }
+    }
+
     #[test]
     fn non_utf8_string() {
         let mut data = BytesMut::from(vec![
@@ -121,6 +178,6 @@ mod test {
             0x00, 0x04, 't' as u8, 'e' as u8, 's' as u8, 't' as u8, // client_id
             0x00, 0x03, 'm' as u8, 'q' as u8, // password with invalid length
         ]);
-        assert_eq!(Err(Error::InvalidLength(3)), decode(&mut data));
+        assert_eq!(Err(Error::InvalidLength), decode(&mut data));
     }
 }
diff --git a/src/encoder.rs b/src/encoder.rs
index 3908476..b88c9e4 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -27,7 +27,7 @@ pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
         }
         Packet::Pubrel(pid) => {
             check_remaining(buffer, 4)?;
-            let header_u8 = 0b01100000 as u8;
+            let header_u8 = 0b01100010 as u8;
             let length = 0b00000010 as u8;
             buffer.put(header_u8);
             buffer.put(length);
@@ -90,7 +90,7 @@ pub(crate) fn write_length(len: usize, buffer: &mut BytesMut) -> Result<(), Erro
         128..=16383 => check_remaining(buffer, len + 2)?,
         16384..=2097151 => check_remaining(buffer, len + 3)?,
         2097152..=268435455 => check_remaining(buffer, len + 4)?,
-        _ => return Err(Error::InvalidLength(len)),
+        _ => return Err(Error::InvalidLength),
     }
     let mut done = false;
     let mut x = len;
diff --git a/src/header.rs b/src/header.rs
index 0f78543..87ac7c0 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -1,40 +1,43 @@
 use crate::{Error, PacketType, QoS};
 use bytes::{BufMut, BytesMut};
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) struct Header {
-    hd: u8,
-    packet_type: PacketType,
-    len: usize,
+    pub typ: PacketType,
+    pub dup: bool,
+    pub qos: QoS,
+    pub retain: bool,
 }
 
 impl Header {
-    pub fn new(hd: u8, len: usize) -> Result<Header, Error> {
+    pub fn new(hd: u8) -> Result<Header, Error> {
+        let (typ, flags_ok) = match hd >> 4 {
+            1 => (PacketType::Connect, hd & 0b1111 == 0),
+            2 => (PacketType::Connack, hd & 0b1111 == 0),
+            3 => (PacketType::Publish, true),
+            4 => (PacketType::Puback, hd & 0b1111 == 0),
+            5 => (PacketType::Pubrec, hd & 0b1111 == 0),
+            6 => (PacketType::Pubrel, hd & 0b1111 == 0b0010),
+            7 => (PacketType::Pubcomp, hd & 0b1111 == 0),
+            8 => (PacketType::Subscribe, hd & 0b1111 == 0b0010),
+            9 => (PacketType::Suback, hd & 0b1111 == 0),
+            10 => (PacketType::Unsubscribe, hd & 0b1111 == 0b0010),
+            11 => (PacketType::Unsuback, hd & 0b1111 == 0),
+            12 => (PacketType::Pingreq, hd & 0b1111 == 0),
+            13 => (PacketType::Pingresp, hd & 0b1111 == 0),
+            14 => (PacketType::Disconnect, hd & 0b1111 == 0),
+            _ => (PacketType::Connect, false),
+        };
+        if !flags_ok {
+            return Err(Error::InvalidHeader);
+        }
         Ok(Header {
-            hd,
-            len,
-            packet_type: PacketType::from_hd(hd)?,
+            typ,
+            dup: hd & 0b1000 != 0,
+            qos: QoS::from_u8((hd & 0b110) >> 1)?,
+            retain: hd & 1 == 1,
         })
     }
-    pub fn packet(&self) -> PacketType {
-        self.packet_type
-    }
-    #[inline]
-    pub fn len(&self) -> usize {
-        self.len
-    }
-    #[inline]
-    pub fn dup(&self) -> bool {
-        (self.hd & 0b1000) != 0
-    }
-    #[inline]
-    pub fn qos(&self) -> Result<QoS, Error> {
-        QoS::from_hd(self.hd)
-    }
-    #[inline]
-    pub fn retain(&self) -> bool {
-        (self.hd & 1) != 0
-    }
 }
 
 /// Protocol version.
@@ -74,14 +77,3 @@ impl Protocol {
         }
     }
 }
-
-/* TESTS */
-#[cfg(test)]
-mod tests {
-    use super::*;
-    #[test]
-    fn header() {
-        let h = Header::new(0b00010000, 0).unwrap();
-        assert_eq!(h.packet(), PacketType::Connect)
-    }
-}
diff --git a/src/lib.rs b/src/lib.rs
index 4f38e14..82820c4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -24,5 +24,3 @@ pub use crate::{
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
     utils::{Error, ConnectReturnCode, LastWill, Pid, QoS, QosPid},
 };
-
-const MULTIPLIER: usize = 0x80 * 0x80 * 0x80 * 0x80;
diff --git a/src/packet.rs b/src/packet.rs
index 522db09..8a3bbc5 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -67,25 +67,3 @@ pub enum PacketType {
     Pingresp,
     Disconnect,
 }
-impl PacketType {
-    #[inline]
-    pub(crate) fn from_hd(hd: u8) -> Result<Self, Error> {
-        match hd >> 4 {
-            1 => Ok(PacketType::Connect),
-            2 => Ok(PacketType::Connack),
-            3 => Ok(PacketType::Publish),
-            4 => Ok(PacketType::Puback),
-            5 => Ok(PacketType::Pubrec),
-            6 => Ok(PacketType::Pubrel),
-            7 => Ok(PacketType::Pubcomp),
-            8 => Ok(PacketType::Subscribe),
-            9 => Ok(PacketType::Suback),
-            10 => Ok(PacketType::Unsubscribe),
-            11 => Ok(PacketType::Unsuback),
-            12 => Ok(PacketType::Pingreq),
-            13 => Ok(PacketType::Pingresp),
-            14 => Ok(PacketType::Disconnect),
-            n => Err(Error::InvalidPacket(n)),
-        }
-    }
-}
diff --git a/src/publish.rs b/src/publish.rs
index bd3d9c7..01289bf 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -14,7 +14,7 @@ impl Publish {
     pub(crate) fn from_buffer(header: &Header, buffer: &mut BytesMut) -> Result<Self, Error> {
         let topic_name = read_string(buffer)?;
 
-        let qospid = match header.qos()? {
+        let qospid = match header.qos {
             QoS::AtMostOnce => QosPid::AtMostOnce,
             QoS::AtLeastOnce => QosPid::AtLeastOnce(Pid::from_buffer(buffer)?),
             QoS::ExactlyOnce => QosPid::ExactlyOnce(Pid::from_buffer(buffer)?),
@@ -22,9 +22,9 @@ impl Publish {
 
         let payload = buffer.to_vec();
         Ok(Publish {
-            dup: header.dup(),
+            dup: header.dup,
             qospid,
-            retain: header.retain(),
+            retain: header.retain,
             topic_name,
             payload,
         })
diff --git a/src/utils.rs b/src/utils.rs
index ba00b46..f47793f 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -15,6 +15,7 @@ pub enum Error {
     /// Not enough data in the read buffer.
     ///
     /// Do not treat this as a fatal error. Read more data into the buffer and try `decode()` again.
+    //FIXME: Should not be needed: decode returns `Ok(None)`.
     UnexpectedEof,
     /// Not enough space in the write buffer.
     ///
@@ -28,13 +29,13 @@ pub enum Error {
     InvalidConnectReturnCode(u8),
     /// Tried to decode an unknown protocol.
     InvalidProtocol(String, u8),
-    /// Tried to decode an invalid packet type for this protocol.
-    InvalidPacket(u8),
+    /// Tried to decode an invalid fixed header (packet type, flags, or remaining_length).
+    InvalidHeader,
     /// Trying to encode/decode an invalid length.
     ///
     /// The difference with `BufferFull`/`BufferIncomplete` is that it refers to an invalid/corrupt
     /// length rather than a buffer size issue.
-    InvalidLength(usize),
+    InvalidLength,
     /// Trying to decode a non-utf8 string.
     InvalidString(std::str::Utf8Error),
     /// Catch-all error when converting from `std::io::Error`.
@@ -145,10 +146,6 @@ impl QoS {
             n => Err(Error::InvalidQos(n)),
         }
     }
-    #[inline]
-    pub(crate) fn from_hd(hd: u8) -> Result<QoS, Error> {
-        Self::from_u8((hd & 0b110) >> 1)
-    }
 }
 
 /// Combined [`QoS`]/[`Pid`].

From 4af0a326cc0a208b085ce0bec3d5421e4482f2f3 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 12:28:11 +0100
Subject: [PATCH 17/23] Add docstring to all public types/modules/functions.

---
 src/connect.rs   |  6 ++++++
 src/decoder.rs   | 22 +++++++++++++++++++++-
 src/encoder.rs   | 25 ++++++++++++++++++++++++-
 src/lib.rs       | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 src/packet.rs    | 42 ++++++++++++++++++++++++++++++++++++++++++
 src/publish.rs   |  3 +++
 src/subscribe.rs | 19 +++++++++++++++++++
 src/utils.rs     | 12 ++++++++++++
 8 files changed, 173 insertions(+), 2 deletions(-)

diff --git a/src/connect.rs b/src/connect.rs
index e4e2346..b5d58d1 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -1,6 +1,9 @@
 use crate::{decoder::*, encoder::*, *};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 
+/// Connect packet ([MQTT 3.1]).
+///
+/// [MQTT 3.1]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028
 #[derive(Debug, Clone, PartialEq)]
 pub struct Connect {
     pub protocol: Protocol,
@@ -12,6 +15,9 @@ pub struct Connect {
     pub password: Option<Vec<u8>>,
 }
 
+/// Connack packet ([MQTT 3.2]).
+///
+/// [MQTT 3.2]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718033
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub struct Connack {
     pub session_present: bool,
diff --git a/src/decoder.rs b/src/decoder.rs
index 9c9c0d7..338ca51 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,9 +1,29 @@
 use crate::{header::Header, *};
 use bytes::{Buf, BytesMut, IntoBuf};
 
-/// Decode network bytes into a [Packet] enum.
+/// Decode bytes from a [BytesMut] buffer as a [Packet] enum.
+///
+/// ```
+/// # use mqttrs::*;
+/// # use bytes::*;
+/// // Fill a buffer with encoded data (probably from a `TcpStream`).
+/// let mut buf = BytesMut::from(vec![0b00110000, 11,
+///                                   0, 4, 't' as u8, 'e' as u8, 's' as u8, 't' as u8,
+///                                  'h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8]);
+///
+/// // Parse the bytes and check the result.
+/// match decode(&mut buf) {
+///     Ok(Some(Packet::Publish(p))) => {
+///         assert_eq!(p.payload, "hello".as_bytes().to_vec());
+///     },
+///     // In real code you probably don't want to panic like that ;)
+///     Ok(None) => panic!("not enough data"),
+///     other => panic!("unexpected {:?}", other),
+/// }
+/// ```
 ///
 /// [Packet]: ../enum.Packet.html
+/// [BytesMut]: https://docs.rs/bytes/0.4.12/bytes/struct.BytesMut.html
 pub fn decode(buffer: &mut BytesMut) -> Result<Option<Packet>, Error> {
     if let Some((header, remaining_len)) = read_header(buffer)? {
         // Advance the buffer position to the next packet, and parse the current packet
diff --git a/src/encoder.rs b/src/encoder.rs
index b88c9e4..ea5a74a 100644
--- a/src/encoder.rs
+++ b/src/encoder.rs
@@ -1,9 +1,32 @@
 use crate::{Error, Packet};
 use bytes::{BufMut, BytesMut};
 
-/// Encode a [Packet] enum into a buffer.
+/// Encode a [Packet] enum into a [BytesMut] buffer.
+///
+/// ```
+/// # use mqttrs::*;
+/// # use bytes::*;
+/// // Instantiate a `Packet` to encode.
+/// let packet = Publish {
+///    dup: false,
+///    qospid: QosPid::AtMostOnce,
+///    retain: false,
+///    topic_name: "test".into(),
+///    payload: "hello".into(),
+/// }.into();
+///
+/// // Allocate a appropriately-sized buffer.
+/// let mut buf = BytesMut::with_capacity(1024);
+///
+/// // Write bytes corresponding to `&Packet` into the `BytesMut`.
+/// encode(&packet, &mut buf).expect("failed encoding");
+/// assert_eq!(&*buf, &[0b00110000, 11,
+///                     0, 4, 't' as u8, 'e' as u8, 's' as u8, 't' as u8,
+///                    'h' as u8, 'e' as u8, 'l' as u8, 'l' as u8, 'o' as u8]);
+/// ```
 ///
 /// [Packet]: ../enum.Packet.html
+/// [BytesMut]: https://docs.rs/bytes/0.4.12/bytes/struct.BytesMut.html
 pub fn encode(packet: &Packet, buffer: &mut BytesMut) -> Result<(), Error> {
     match packet {
         Packet::Connect(connect) => connect.to_buffer(buffer),
diff --git a/src/lib.rs b/src/lib.rs
index 82820c4..af8f284 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,3 +1,49 @@
+//! `mqttrs` is a codec for the MQTT protocol.
+//!
+//! The API aims to be straightforward and composable, usable with plain `std` or with a framework
+//! like [tokio]. The decoded packet is help in a [Packet] struct, and the encoded bytes in a
+//! [bytes::BytesMut] struct. Convert between the two using [encode()] and [decode()]. Almost all
+//! struct fields can be accessed directly, to create or read packets.
+//!
+//! It currently targets [MQTT 3.1], with [MQTT 5] support planned.
+//!
+//! ```
+//! use mqttrs::*;
+//! use bytes::BytesMut;
+//!
+//! // Allocate buffer.
+//! let mut buf = BytesMut::with_capacity(1024);
+//!
+//! // Encode an MQTT Connect packet.
+//! let pkt = Packet::Connect(Connect { protocol: Protocol::MQTT311,
+//!                                     keep_alive: 30,
+//!                                     client_id: "doc_client".into(),
+//!                                     clean_session: true,
+//!                                     last_will: None,
+//!                                     username: None,
+//!                                     password: None });
+//! assert!(encode(&pkt, &mut buf).is_ok());
+//! assert_eq!(&buf[14..], "doc_client".as_bytes());
+//! let mut encoded = buf.clone();
+//!
+//! // Decode one packet. The buffer will advance to the next packet.
+//! assert_eq!(Ok(Some(pkt)), decode(&mut buf));
+//!
+//! // Example decode failures.
+//! let mut incomplete = encoded.split_to(10);
+//! assert_eq!(Ok(None), decode(&mut incomplete));
+//! let mut garbage = BytesMut::from(vec![0u8,0,0,0]);
+//! assert_eq!(Err(Error::InvalidHeader), decode(&mut garbage));
+//! ```
+//!
+//! [MQTT 3.1]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html
+//! [MQTT 5]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html
+//! [tokio]: https://tokio.rs/
+//! [Packet]: enum.Packet.html
+//! [encode()]: fn.encode.html
+//! [decode()]: fn.decode.html
+//! [bytes::BytesMut]: https://docs.rs/bytes/0.4.12/bytes/struct.BytesMut.html
+
 mod connect;
 mod decoder;
 mod encoder;
diff --git a/src/packet.rs b/src/packet.rs
index 8a3bbc5..2fd2c86 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -1,23 +1,64 @@
 use crate::*;
 
+/// Base enum for all MQTT packet types.
+///
+/// This is the main type you'll be interacting with, as an output of [`decode()`] and an input of
+/// [`encode()`]. Most variants can be constructed directly without using methods.
+///
+/// ```
+/// # use mqttrs::*;
+/// // Simplest form
+/// let pkt = Packet::Connack(Connack { session_present: false,
+///                                     code: ConnectReturnCode::Accepted });
+/// // Using `Into` trait
+/// let publish = Publish { dup: false,
+///                         qospid: QosPid::AtMostOnce,
+///                         retain: false,
+///                         topic_name: "to/pic".into(),
+///                         payload: "payload".into() };
+/// let pkt: Packet = publish.into();
+/// // Identifyer-only packets
+/// let pkt = Packet::Puback(Pid::new(42).unwrap());
+/// ```
+///
+/// [`encode()`]: fn.encode.html
+/// [`decode()`]: fn.decode.html
 #[derive(Debug, Clone, PartialEq)]
 pub enum Packet {
+    /// [MQTT 3.1](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028)
     Connect(Connect),
+    /// [MQTT 3.2](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718033)
     Connack(Connack),
+    /// [MQTT 3.3](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718037)
     Publish(Publish),
+    /// [MQTT 3.4](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718043)
     Puback(Pid),
+    /// [MQTT 3.5](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718048)
     Pubrec(Pid),
+    /// [MQTT 3.6](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718053)
     Pubrel(Pid),
+    /// [MQTT 3.7](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718058)
     Pubcomp(Pid),
+    /// [MQTT 3.8](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718063)
     Subscribe(Subscribe),
+    /// [MQTT 3.9](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718068)
     Suback(Suback),
+    /// [MQTT 3.10](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718072)
     Unsubscribe(Unsubscribe),
+    /// [MQTT 3.11](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718077)
     Unsuback(Pid),
+    /// [MQTT 3.12](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718081)
     Pingreq,
+    /// [MQTT 3.13](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718086)
     Pingresp,
+    /// [MQTT 3.14](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718090)
     Disconnect,
 }
 impl Packet {
+    /// Return the packet type variant.
+    ///
+    /// This can be used for matching, categorising, debuging, etc. Most users will match directly
+    /// on `Packet` instead.
     pub fn get_type(&self) -> PacketType {
         match self {
             Packet::Connect(_) => PacketType::Connect,
@@ -50,6 +91,7 @@ macro_rules! packet_from {
 }
 packet_from!(Connect, Connack, Publish, Subscribe, Suback, Unsubscribe);
 
+/// Packet type variant, without the associated data.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum PacketType {
     Connect,
diff --git a/src/publish.rs b/src/publish.rs
index 01289bf..22f3cbd 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,6 +1,9 @@
 use crate::{decoder::*, encoder::*, header::Header, *};
 use bytes::{BufMut, BytesMut};
 
+/// Publish packet ([MQTT 3.3]).
+///
+/// [MQTT 3.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718037
 #[derive(Debug, Clone, PartialEq)]
 pub struct Publish {
     pub dup: bool,
diff --git a/src/subscribe.rs b/src/subscribe.rs
index f094f93..de8c58e 100644
--- a/src/subscribe.rs
+++ b/src/subscribe.rs
@@ -1,12 +1,22 @@
 use crate::{decoder::*, encoder::*, *};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 
+/// Subscribe topic.
+///
+/// [Subscribe] packets contain a `Vec` of those.
+///
+/// [Subscribe]: struct.Subscribe.html
 #[derive(Debug, Clone, PartialEq)]
 pub struct SubscribeTopic {
     pub topic_path: String,
     pub qos: QoS,
 }
 
+/// Subscribe return value.
+///
+/// [Suback] packets contain a `Vec` of those.
+///
+/// [Suback]: struct.Subscribe.html
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum SubscribeReturnCodes {
     Success(QoS),
@@ -21,18 +31,27 @@ impl SubscribeReturnCodes {
     }
 }
 
+/// Subscribe packet ([MQTT 3.8]).
+///
+/// [MQTT 3.8]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718063
 #[derive(Debug, Clone, PartialEq)]
 pub struct Subscribe {
     pub pid: Pid,
     pub topics: Vec<SubscribeTopic>,
 }
 
+/// Subsack packet ([MQTT 3.9]).
+///
+/// [MQTT 3.9]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718068
 #[derive(Debug, Clone, PartialEq)]
 pub struct Suback {
     pub pid: Pid,
     pub return_codes: Vec<SubscribeReturnCodes>,
 }
 
+/// Unsubscribe packet ([MQTT 3.10]).
+///
+/// [MQTT 3.10]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718072
 #[derive(Debug, Clone, PartialEq)]
 pub struct Unsubscribe {
     pub pid: Pid,
diff --git a/src/utils.rs b/src/utils.rs
index f47793f..3c97f33 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -193,6 +193,12 @@ impl QosPid {
     }
 }
 
+/// Message that the server should publish when the client disconnects.
+///
+/// Sent by the client in the [Connect] packet. [MQTT 3.1.3.3].
+///
+/// [Connect]: struct.Connect.html
+/// [MQTT 3.1.3.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718031
 #[derive(Debug, Clone, PartialEq)]
 pub struct LastWill {
     pub topic: String,
@@ -201,6 +207,12 @@ pub struct LastWill {
     pub retain: bool,
 }
 
+/// Sucess value of a [Connack] packet.
+///
+/// See [MQTT 3.2.2.3] for interpretations.
+///
+/// [Connack]: struct.Connack.html
+/// [MQTT 3.2.2.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718035
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum ConnectReturnCode {
     Accepted,

From 38eaa9168634d145915ff1dcb32a4fc65c50989f Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 13:03:50 +0100
Subject: [PATCH 18/23] Move code around a bit.

Move structs away from header.rs and utils.rs, keeping them closer to where they are used.
---
 src/connect.rs | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/decoder.rs | 42 +++++++++++++++++++++--
 src/header.rs  | 79 -------------------------------------------
 src/lib.rs     |  6 ++--
 src/publish.rs |  2 +-
 src/utils.rs   | 53 -----------------------------
 6 files changed, 134 insertions(+), 139 deletions(-)
 delete mode 100644 src/header.rs

diff --git a/src/connect.rs b/src/connect.rs
index b5d58d1..75cb7f7 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -1,6 +1,97 @@
 use crate::{decoder::*, encoder::*, *};
 use bytes::{Buf, BufMut, BytesMut, IntoBuf};
 
+/// Protocol version.
+///
+/// Sent in [`Connect`] packet.
+///
+/// [`Connect`]: struct.Connect.html
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Protocol {
+    /// [MQTT 3.1.1] is the most commonly implemented version. [MQTT 5] isn't yet supported my by
+    /// `mqttrs`.
+    ///
+    /// [MQTT 3.1.1]: https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html
+    /// [MQTT 5]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html
+    MQTT311,
+    /// MQIsdp, aka SCADA are pre-standardisation names of MQTT. It should mostly conform to MQTT
+    /// 3.1.1, but you should watch out for implementation discrepancies. `Mqttrs` handles it like
+    /// standard MQTT 3.1.1.
+    MQIsdp,
+}
+impl Protocol {
+    pub(crate) fn new(name: &str, level: u8) -> Result<Protocol, Error> {
+        match (name, level) {
+            ("MQIsdp", 3) => Ok(Protocol::MQIsdp),
+            ("MQTT", 4) => Ok(Protocol::MQTT311),
+            _ => Err(Error::InvalidProtocol(name.into(), level)),
+        }
+    }
+    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
+        match self {
+            Protocol::MQTT311 => {
+                Ok(buffer.put_slice(&[0u8, 4, 'M' as u8, 'Q' as u8, 'T' as u8, 'T' as u8, 4]))
+            }
+            Protocol::MQIsdp => Ok(buffer.put_slice(&[
+                0u8, 4, 'M' as u8, 'Q' as u8, 'i' as u8, 's' as u8, 'd' as u8, 'p' as u8, 4,
+            ])),
+        }
+    }
+}
+
+/// Message that the server should publish when the client disconnects.
+///
+/// Sent by the client in the [Connect] packet. [MQTT 3.1.3.3].
+///
+/// [Connect]: struct.Connect.html
+/// [MQTT 3.1.3.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718031
+#[derive(Debug, Clone, PartialEq)]
+pub struct LastWill {
+    pub topic: String,
+    pub message: Vec<u8>,
+    pub qos: QoS,
+    pub retain: bool,
+}
+
+/// Sucess value of a [Connack] packet.
+///
+/// See [MQTT 3.2.2.3] for interpretations.
+///
+/// [Connack]: struct.Connack.html
+/// [MQTT 3.2.2.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718035
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub enum ConnectReturnCode {
+    Accepted,
+    RefusedProtocolVersion,
+    RefusedIdentifierRejected,
+    ServerUnavailable,
+    BadUsernamePassword,
+    NotAuthorized,
+}
+impl ConnectReturnCode {
+    fn to_u8(&self) -> u8 {
+        match *self {
+            ConnectReturnCode::Accepted => 0,
+            ConnectReturnCode::RefusedProtocolVersion => 1,
+            ConnectReturnCode::RefusedIdentifierRejected => 2,
+            ConnectReturnCode::ServerUnavailable => 3,
+            ConnectReturnCode::BadUsernamePassword => 4,
+            ConnectReturnCode::NotAuthorized => 5,
+        }
+    }
+    pub(crate) fn from_u8(byte: u8) -> Result<ConnectReturnCode, Error> {
+        match byte {
+            0 => Ok(ConnectReturnCode::Accepted),
+            1 => Ok(ConnectReturnCode::RefusedProtocolVersion),
+            2 => Ok(ConnectReturnCode::RefusedIdentifierRejected),
+            3 => Ok(ConnectReturnCode::ServerUnavailable),
+            4 => Ok(ConnectReturnCode::BadUsernamePassword),
+            5 => Ok(ConnectReturnCode::NotAuthorized),
+            n => Err(Error::InvalidConnectReturnCode(n)),
+        }
+    }
+}
+
 /// Connect packet ([MQTT 3.1]).
 ///
 /// [MQTT 3.1]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028
diff --git a/src/decoder.rs b/src/decoder.rs
index 338ca51..d8acbc5 100644
--- a/src/decoder.rs
+++ b/src/decoder.rs
@@ -1,4 +1,4 @@
-use crate::{header::Header, *};
+use crate::*;
 use bytes::{Buf, BytesMut, IntoBuf};
 
 /// Decode bytes from a [BytesMut] buffer as a [Packet] enum.
@@ -81,6 +81,44 @@ fn read_header(buffer: &mut BytesMut) -> Result<Option<(Header, usize)>, Error>
     Err(Error::InvalidHeader)
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct Header {
+    pub typ: PacketType,
+    pub dup: bool,
+    pub qos: QoS,
+    pub retain: bool,
+}
+impl Header {
+    pub fn new(hd: u8) -> Result<Header, Error> {
+        let (typ, flags_ok) = match hd >> 4 {
+            1 => (PacketType::Connect, hd & 0b1111 == 0),
+            2 => (PacketType::Connack, hd & 0b1111 == 0),
+            3 => (PacketType::Publish, true),
+            4 => (PacketType::Puback, hd & 0b1111 == 0),
+            5 => (PacketType::Pubrec, hd & 0b1111 == 0),
+            6 => (PacketType::Pubrel, hd & 0b1111 == 0b0010),
+            7 => (PacketType::Pubcomp, hd & 0b1111 == 0),
+            8 => (PacketType::Subscribe, hd & 0b1111 == 0b0010),
+            9 => (PacketType::Suback, hd & 0b1111 == 0),
+            10 => (PacketType::Unsubscribe, hd & 0b1111 == 0b0010),
+            11 => (PacketType::Unsuback, hd & 0b1111 == 0),
+            12 => (PacketType::Pingreq, hd & 0b1111 == 0),
+            13 => (PacketType::Pingresp, hd & 0b1111 == 0),
+            14 => (PacketType::Disconnect, hd & 0b1111 == 0),
+            _ => (PacketType::Connect, false),
+        };
+        if !flags_ok {
+            return Err(Error::InvalidHeader);
+        }
+        Ok(Header {
+            typ,
+            dup: hd & 0b1000 != 0,
+            qos: QoS::from_u8((hd & 0b110) >> 1)?,
+            retain: hd & 1 == 1,
+        })
+    }
+}
+
 pub(crate) fn read_string(buffer: &mut BytesMut) -> Result<String, Error> {
     String::from_utf8(read_bytes(buffer)?).map_err(|e| Error::InvalidString(e.utf8_error()))
 }
@@ -96,7 +134,7 @@ pub(crate) fn read_bytes(buffer: &mut BytesMut) -> Result<Vec<u8>, Error> {
 
 #[cfg(test)]
 mod test {
-    use crate::{decoder::read_header, header::Header, *};
+    use crate::decoder::*;
     use bytes::BytesMut;
 
     macro_rules! header {
diff --git a/src/header.rs b/src/header.rs
deleted file mode 100644
index 87ac7c0..0000000
--- a/src/header.rs
+++ /dev/null
@@ -1,79 +0,0 @@
-use crate::{Error, PacketType, QoS};
-use bytes::{BufMut, BytesMut};
-
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub(crate) struct Header {
-    pub typ: PacketType,
-    pub dup: bool,
-    pub qos: QoS,
-    pub retain: bool,
-}
-
-impl Header {
-    pub fn new(hd: u8) -> Result<Header, Error> {
-        let (typ, flags_ok) = match hd >> 4 {
-            1 => (PacketType::Connect, hd & 0b1111 == 0),
-            2 => (PacketType::Connack, hd & 0b1111 == 0),
-            3 => (PacketType::Publish, true),
-            4 => (PacketType::Puback, hd & 0b1111 == 0),
-            5 => (PacketType::Pubrec, hd & 0b1111 == 0),
-            6 => (PacketType::Pubrel, hd & 0b1111 == 0b0010),
-            7 => (PacketType::Pubcomp, hd & 0b1111 == 0),
-            8 => (PacketType::Subscribe, hd & 0b1111 == 0b0010),
-            9 => (PacketType::Suback, hd & 0b1111 == 0),
-            10 => (PacketType::Unsubscribe, hd & 0b1111 == 0b0010),
-            11 => (PacketType::Unsuback, hd & 0b1111 == 0),
-            12 => (PacketType::Pingreq, hd & 0b1111 == 0),
-            13 => (PacketType::Pingresp, hd & 0b1111 == 0),
-            14 => (PacketType::Disconnect, hd & 0b1111 == 0),
-            _ => (PacketType::Connect, false),
-        };
-        if !flags_ok {
-            return Err(Error::InvalidHeader);
-        }
-        Ok(Header {
-            typ,
-            dup: hd & 0b1000 != 0,
-            qos: QoS::from_u8((hd & 0b110) >> 1)?,
-            retain: hd & 1 == 1,
-        })
-    }
-}
-
-/// Protocol version.
-///
-/// Sent in [`Connect`] packet.
-///
-/// [`Connect`]: struct.Connect.html
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum Protocol {
-    /// [MQTT 3.1.1] is the most commonly implemented version. [MQTT 5] isn't yet supported my by
-    /// `mqttrs`.
-    ///
-    /// [MQTT 3.1.1]: https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html
-    /// [MQTT 5]: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html
-    MQTT311,
-    /// MQIsdp, aka SCADA are pre-standardisation names of MQTT. It should mostly conform to MQTT
-    /// 3.1.1, but you should watch out for implementation discrepancies. `Mqttrs` handles it like
-    /// standard MQTT 3.1.1.
-    MQIsdp,
-}
-impl Protocol {
-    pub(crate) fn new(name: &str, level: u8) -> Result<Protocol, Error> {
-        match (name, level) {
-            ("MQIsdp", 3) => Ok(Protocol::MQIsdp),
-            ("MQTT", 4) => Ok(Protocol::MQTT311),
-            _ => Err(Error::InvalidProtocol(name.into(), level)),
-        }
-    }
-    pub(crate) fn to_buffer(&self, buffer: &mut BytesMut) -> Result<(), Error> {
-        match self {
-            Protocol::MQTT311 => {
-                Ok(buffer.put_slice(&[0u8, 4, 'M' as u8, 'Q' as u8, 'T' as u8, 'T' as u8, 4]))
-            }
-            Protocol::MQIsdp => Ok(buffer.put_slice(&[
-                0u8, 4, 'M' as u8, 'Q' as u8, 'i' as u8, 's' as u8, 'd' as u8, 'p' as u8, 4,
-            ])),
-        }
-    }
-}
diff --git a/src/lib.rs b/src/lib.rs
index af8f284..3dc15cc 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -47,7 +47,6 @@
 mod connect;
 mod decoder;
 mod encoder;
-mod header;
 mod packet;
 mod publish;
 mod subscribe;
@@ -61,12 +60,11 @@ mod decoder_test;
 mod encoder_test;
 
 pub use crate::{
-    connect::{Connack, Connect},
+    connect::{Connack, Connect, ConnectReturnCode, LastWill, Protocol},
     decoder::decode,
     encoder::encode,
-    header::Protocol,
     packet::{Packet, PacketType},
     publish::Publish,
     subscribe::{Suback, Subscribe, SubscribeReturnCodes, SubscribeTopic, Unsubscribe},
-    utils::{Error, ConnectReturnCode, LastWill, Pid, QoS, QosPid},
+    utils::{Error, Pid, QoS, QosPid},
 };
diff --git a/src/publish.rs b/src/publish.rs
index 22f3cbd..d204af3 100644
--- a/src/publish.rs
+++ b/src/publish.rs
@@ -1,4 +1,4 @@
-use crate::{decoder::*, encoder::*, header::Header, *};
+use crate::{decoder::*, encoder::*, *};
 use bytes::{BufMut, BytesMut};
 
 /// Publish packet ([MQTT 3.3]).
diff --git a/src/utils.rs b/src/utils.rs
index 3c97f33..28ea31a 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -192,56 +192,3 @@ impl QosPid {
         }
     }
 }
-
-/// Message that the server should publish when the client disconnects.
-///
-/// Sent by the client in the [Connect] packet. [MQTT 3.1.3.3].
-///
-/// [Connect]: struct.Connect.html
-/// [MQTT 3.1.3.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718031
-#[derive(Debug, Clone, PartialEq)]
-pub struct LastWill {
-    pub topic: String,
-    pub message: Vec<u8>,
-    pub qos: QoS,
-    pub retain: bool,
-}
-
-/// Sucess value of a [Connack] packet.
-///
-/// See [MQTT 3.2.2.3] for interpretations.
-///
-/// [Connack]: struct.Connack.html
-/// [MQTT 3.2.2.3]: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718035
-#[derive(Debug, Clone, Copy, PartialEq)]
-pub enum ConnectReturnCode {
-    Accepted,
-    RefusedProtocolVersion,
-    RefusedIdentifierRejected,
-    ServerUnavailable,
-    BadUsernamePassword,
-    NotAuthorized,
-}
-impl ConnectReturnCode {
-    pub(crate) fn to_u8(&self) -> u8 {
-        match *self {
-            ConnectReturnCode::Accepted => 0,
-            ConnectReturnCode::RefusedProtocolVersion => 1,
-            ConnectReturnCode::RefusedIdentifierRejected => 2,
-            ConnectReturnCode::ServerUnavailable => 3,
-            ConnectReturnCode::BadUsernamePassword => 4,
-            ConnectReturnCode::NotAuthorized => 5,
-        }
-    }
-    pub(crate) fn from_u8(byte: u8) -> Result<ConnectReturnCode, Error> {
-        match byte {
-            0 => Ok(ConnectReturnCode::Accepted),
-            1 => Ok(ConnectReturnCode::RefusedProtocolVersion),
-            2 => Ok(ConnectReturnCode::RefusedIdentifierRejected),
-            3 => Ok(ConnectReturnCode::ServerUnavailable),
-            4 => Ok(ConnectReturnCode::BadUsernamePassword),
-            5 => Ok(ConnectReturnCode::NotAuthorized),
-            n => Err(Error::InvalidConnectReturnCode(n)),
-        }
-    }
-}

From 6f5373e6fbbc4ef193f171e025e342cac3c1371c Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 13:22:52 +0100
Subject: [PATCH 19/23] Remove Error::UnexpectedEof variant.

We never contruct it: `decode()` actually returns `Ok(None)` in that case.
---
 src/utils.rs | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/src/utils.rs b/src/utils.rs
index 28ea31a..ca3452e 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -12,11 +12,6 @@ use std::{
 /// [`decode()`]: fn.decode.html
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Error {
-    /// Not enough data in the read buffer.
-    ///
-    /// Do not treat this as a fatal error. Read more data into the buffer and try `decode()` again.
-    //FIXME: Should not be needed: decode returns `Ok(None)`.
-    UnexpectedEof,
     /// Not enough space in the write buffer.
     ///
     /// It is the caller's responsiblity to pass a big enough buffer to `encode()`.
@@ -53,7 +48,6 @@ impl From<Error> for IoError {
     fn from(err: Error) -> IoError {
         match err {
             Error::WriteZero => IoError::new(ErrorKind::WriteZero, err),
-            Error::UnexpectedEof => IoError::new(ErrorKind::UnexpectedEof, err),
             _ => IoError::new(ErrorKind::InvalidData, err),
         }
     }
@@ -62,7 +56,6 @@ impl From<IoError> for Error {
     fn from(err: IoError) -> Error {
         match err.kind() {
             ErrorKind::WriteZero => Error::WriteZero,
-            ErrorKind::UnexpectedEof => Error::UnexpectedEof,
             k => Error::IoError(k, format!("{}",err)),
         }
     }

From fc9a6cb64caf38cfe49c83baa9440e16989c9725 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 14:21:47 +0100
Subject: [PATCH 20/23] Rename `Pid::new(u16)` to `Pid::try_from(u16)` and add
 an infaillible `Pid::new()`.

This is just for convenience. Creating a new `Pid` with a specific value is expected to be rare :
either we want value 1, or we get a `Pid` from a packet/session and eventually add 1 to it.
---
 src/codec_test.rs   |  2 +-
 src/encoder_test.rs | 16 ++++++++--------
 src/packet.rs       |  2 +-
 src/utils.rs        | 17 ++++++++++++-----
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/src/codec_test.rs b/src/codec_test.rs
index 617745b..526a26a 100644
--- a/src/codec_test.rs
+++ b/src/codec_test.rs
@@ -16,7 +16,7 @@ prop_compose! {
 }
 prop_compose! {
     fn stg_pid()(pid in 1..std::u16::MAX) -> Pid {
-        Pid::new(pid).unwrap()
+        Pid::try_from(pid).unwrap()
     }
 }
 prop_compose! {
diff --git a/src/encoder_test.rs b/src/encoder_test.rs
index 1fe43c6..4bab58e 100644
--- a/src/encoder_test.rs
+++ b/src/encoder_test.rs
@@ -53,7 +53,7 @@ fn test_publish() {
 
 #[test]
 fn test_puback() {
-    let packet = Packet::Puback(Pid::new(19).unwrap());
+    let packet = Packet::Puback(Pid::try_from(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     encode(&packet, &mut buffer).unwrap();
     match decode(&mut buffer) {
@@ -64,7 +64,7 @@ fn test_puback() {
 
 #[test]
 fn test_pubrec() {
-    let packet = Packet::Pubrec(Pid::new(19).unwrap());
+    let packet = Packet::Pubrec(Pid::try_from(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     encode(&packet, &mut buffer).unwrap();
     match decode(&mut buffer) {
@@ -75,7 +75,7 @@ fn test_pubrec() {
 
 #[test]
 fn test_pubrel() {
-    let packet = Packet::Pubrel(Pid::new(19).unwrap());
+    let packet = Packet::Pubrel(Pid::try_from(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     encode(&packet, &mut buffer).unwrap();
     match decode(&mut buffer) {
@@ -86,7 +86,7 @@ fn test_pubrel() {
 
 #[test]
 fn test_pubcomp() {
-    let packet = Packet::Pubcomp(Pid::new(19).unwrap());
+    let packet = Packet::Pubcomp(Pid::try_from(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     encode(&packet, &mut buffer).unwrap();
     match decode(&mut buffer) {
@@ -102,7 +102,7 @@ fn test_subscribe() {
         qos: QoS::ExactlyOnce,
     };
     let packet = Subscribe {
-        pid: Pid::new(345).unwrap(),
+        pid: Pid::try_from(345).unwrap(),
         topics: vec![stopic],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -117,7 +117,7 @@ fn test_subscribe() {
 fn test_suback() {
     let return_code = SubscribeReturnCodes::Success(QoS::ExactlyOnce);
     let packet = Suback {
-        pid: Pid::new(12321).unwrap(),
+        pid: Pid::try_from(12321).unwrap(),
         return_codes: vec![return_code],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -131,7 +131,7 @@ fn test_suback() {
 #[test]
 fn test_unsubscribe() {
     let packet = Unsubscribe {
-        pid: Pid::new(12321).unwrap(),
+        pid: Pid::try_from(12321).unwrap(),
         topics: vec!["a/b".to_string()],
     };
     let mut buffer = BytesMut::with_capacity(1024);
@@ -144,7 +144,7 @@ fn test_unsubscribe() {
 
 #[test]
 fn test_unsuback() {
-    let packet = Packet::Unsuback(Pid::new(19).unwrap());
+    let packet = Packet::Unsuback(Pid::try_from(19).unwrap());
     let mut buffer = BytesMut::with_capacity(1024);
     encode(&packet, &mut buffer).unwrap();
     match decode(&mut buffer) {
diff --git a/src/packet.rs b/src/packet.rs
index 2fd2c86..afdd6a5 100644
--- a/src/packet.rs
+++ b/src/packet.rs
@@ -18,7 +18,7 @@ use crate::*;
 ///                         payload: "payload".into() };
 /// let pkt: Packet = publish.into();
 /// // Identifyer-only packets
-/// let pkt = Packet::Puback(Pid::new(42).unwrap());
+/// let pkt = Packet::Puback(Pid::try_from(42).unwrap());
 /// ```
 ///
 /// [`encode()`]: fn.encode.html
diff --git a/src/utils.rs b/src/utils.rs
index ca3452e..4438c2a 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -67,7 +67,7 @@ impl From<IoError> for Error {
 ///
 /// ```rust
 /// # use mqttrs::{Pid, Packet};
-/// let pid = Pid::new(42).expect("illegal pid value");
+/// let pid = Pid::try_from(42).expect("illegal pid value");
 /// let next_pid = pid + 1;
 /// let pending_acks = std::collections::HashMap::<Pid, Packet>::new();
 /// ```
@@ -80,17 +80,24 @@ impl From<IoError> for Error {
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Pid(NonZeroU16);
 impl Pid {
-    pub fn new(u: u16) -> Result<Self, Error> {
+    /// Returns a new `Pid` with value `1`.
+    pub fn new() -> Self {
+        Pid(NonZeroU16::new(1).unwrap())
+    }
+    /// Returns a new `Pid` with specified value.
+    // Not using std::convert::TryFrom so that don't have to depend on rust 1.34.
+    pub fn try_from(u: u16) -> Result<Self, Error> {
         match NonZeroU16::new(u) {
             Some(nz) => Ok(Pid(nz)),
             None => Err(Error::InvalidPid),
         }
     }
+    /// Get the `Pid` as a raw `u16`.
     pub fn get(self) -> u16 {
         self.0.get()
     }
     pub(crate) fn from_buffer(buf: &mut BytesMut) -> Result<Self, Error> {
-        Self::new(buf.split_to(2).into_buf().get_u16_be())
+        Self::try_from(buf.split_to(2).into_buf().get_u16_be())
     }
     pub(crate) fn to_buffer(self, buf: &mut BytesMut) -> Result<(), Error> {
         Ok(buf.put_u16_be(self.get()))
@@ -159,8 +166,8 @@ impl QosPid {
     pub(crate) fn from_u8u16(qos: u8, pid: u16) -> Self {
         match qos {
             0 => QosPid::AtMostOnce,
-            1 => QosPid::AtLeastOnce(Pid::new(pid).expect("pid == 0")),
-            2 => QosPid::ExactlyOnce(Pid::new(pid).expect("pid == 0")),
+            1 => QosPid::AtLeastOnce(Pid::try_from(pid).expect("pid == 0")),
+            2 => QosPid::ExactlyOnce(Pid::try_from(pid).expect("pid == 0")),
             _ => panic!("Qos > 2"),
         }
     }

From e2d0b5cc43c479df86ad2822e240deb4cb492351 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 16:07:44 +0100
Subject: [PATCH 21/23] Revamp README.md.

---
 README.md | 65 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/README.md b/README.md
index b677a9d..9dee2e6 100644
--- a/README.md
+++ b/README.md
@@ -1,20 +1,45 @@
-# Rust Mqtt Encoding & Decoding
-
-### What is Mqtt?
-MQTT is an ISO standard publish-subscribe-based messaging protocol. It works on top of the TCP/IP protocol.
-
-### What is Rust?
-Rust is a multi-paradigm systems programming language focused on safety, especially safe concurrency. Rust is syntactically similar to C++, but is designed to provide better memory safety while maintaining high performance.
-
-### What is mqttrs?
-
-It is library which can be used in any rust projects where you need to transform valid mqtt bytes buffer to Mqtt types and vice versa. 
-
-In short it is encoding/decoding library which you can use it in sync as well as async environment.
-
-The way it works is, It will take byte buffer as input and then will try to read the header of the mqtt packet, if the packet is not completely received as it happens in async networking, the library function will return `None` and will not remove any bytes from buffer.
-
-Once, the whole mqtt packet is received, mqttrs will convert the bytes into appropriate mqtt packet type and return as well as remove all bytes from the beginning which belongs to already received packet.  
-
-So, in this way, this library can be used for sync tcp streams as well as async streams like tokio tcp streams. 
-
+# Rust Mqtt Encoding & Decoding  [![Crates.io](https://img.shields.io/crates/l/mqttrs)](LICENSE) [![Docs.rs](https://docs.rs/mqttrs/badge.svg)](https://docs.rs/mqttrs/*/mqttrs/)
+
+`Mqttrs` is a [Rust](https://www.rust-lang.org/) crate (library) to write [MQTT
+protocol](https://mqtt.org/) clients and servers.
+
+It is a codec-only library with [very few dependencies](Cargo.toml) and a [straightworward and
+composable API](https://docs.rs/mqttrs/*/mqttrs/), usable with rust's standard library or with async
+frameworks like [tokio](https://tokio.rs/).
+
+`Mqttrs` currently requires [Rust >= 1.32](https://www.rust-lang.org/learn/get-started) and supports
+[MQTT 3.1.1](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html). Support for [MQTT
+5](https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html) is planned for a future version.
+
+## Usage
+
+Add `mqttrs = "0.2"` to your `Cargo.toml`.
+
+```rust
+use mqttrs::*;
+use bytes::BytesMut;
+
+// Allocate write buffer.
+let mut buf = BytesMut::with_capacity(1024);
+
+// Encode an MQTT Connect packet.
+let pkt = Packet::Connect(Connect { protocol: Protocol::MQTT311,
+                                    keep_alive: 30,
+                                    client_id: "doc_client".into(),
+                                    clean_session: true,
+                                    last_will: None,
+                                    username: None,
+                                    password: None });
+assert!(encode(&pkt, &mut buf).is_ok());
+assert_eq!(&buf[14..], "doc_client".as_bytes());
+let mut encoded = buf.clone();
+
+// Decode one packet. The buffer will advance to the next packet.
+assert_eq!(Ok(Some(pkt)), decode(&mut buf));
+
+// Example decode failures.
+let mut incomplete = encoded.split_to(10);
+assert_eq!(Ok(None), decode(&mut incomplete));
+let mut garbage = BytesMut::from(vec![0u8,0,0,0]);
+assert_eq!(Err(Error::InvalidHeader), decode(&mut garbage));
+```
\ No newline at end of file

From 2448bd21283e402aa9db89993a8bab6b6f745def Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 16:08:11 +0100
Subject: [PATCH 22/23] Add CHANGELOG.md.

---
 CHANGELOG.md | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 CHANGELOG.md

diff --git a/CHANGELOG.md b/CHANGELOG.md
new file mode 100644
index 0000000..c77aac4
--- /dev/null
+++ b/CHANGELOG.md
@@ -0,0 +1,27 @@
+# 0.2 (unreleased)
+
+This is a fairly large release with API fixes and improvements, bug fixes, and much better test
+coverage and documentation.
+
+## API changes
+
+* We now return a dedicated error type instead of abusing `std::io::Error`.
+* `PacketIdentifier` was renamed to `Pid`. It now avoids the illegal value 0, wraps around automatically, and can be hashed.
+* `Publish.qos` and `Publish.pid` have been merged together, avoiding accidental illegal combinations.
+* `Connect.password` and `Connect.will.payload` can now contain binary data.
+* The `Protocol` enum doesn't carry extra data anymore.
+* All public structs/enum/functions are now (re)exported from the crate root, and the rest has been made private.
+* The letter-casing of packet types is more consistent.
+* Packet subtypes can be converted to `Packet` using `.into()`.
+
+## Other changes
+
+* Much improved documentation. See it with `cargo doc --open`.
+* More thorough unittesting, including exhaustive and random value ranges testing.
+* Lots of corner-case bugfixes, particularly when decoding partial or corrupted data.
+* The minimum rust version is now 1.32.
+* Raised `mqttrs`'s bus factor to 2 ;)
+
+# 0.1.4 (2019-09-16)
+
+* Fix issue #8: Decoding an incomplete packet still consumes bytes from the buffer.

From 7c77d351d0da473935308e7e143ea7a544831440 Mon Sep 17 00:00:00 2001
From: Vincent de Phily <vincent.dephily@mobile-devices.fr>
Date: Wed, 23 Oct 2019 16:10:23 +0100
Subject: [PATCH 23/23] Add myself to authors.

I think it's deserved at this stage ;)
---
 Cargo.toml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Cargo.toml b/Cargo.toml
index b04802d..fedacc8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,8 @@
 [package]
 name = "mqttrs"
 version = "0.1.4"
-authors = ["00imvj00 <vijaybambhaniya007@gmail.com>"]
+authors = ["00imvj00 <vijaybambhaniya007@gmail.com>",
+        "Vincent de Phily <moltonel@gmail.com>"]
 edition = "2018"
 description = "mqttrs is encoding & decoding library for mqtt protocol, it can work with both sync as well as async apps"
 homepage = "https://github.com/00imvj00/mqttrs"