move code to make functions smaller

crates/servicepoint/examples/brightness_tester.rs

@@ -3,7 +3,6 @@
 use clap::Parser;
 
 use servicepoint::*;
-use servicepoint::Command::BitmapLinearWin;
 
 #[derive(Parser, Debug)]
 struct Cli {
@@ -19,7 +18,7 @@ fn main() {
     let mut pixels = PixelGrid::max_sized();
     pixels.fill(true);
 
-    let command = BitmapLinearWin(
+    let command = Command::BitmapLinearWin(
         Origin::new(0, 0),
         pixels,
         CompressionCode::Uncompressed,

crates/servicepoint/src/brightness.rs

@@ -94,11 +94,7 @@ impl TryFrom<PrimitiveGrid<u8>> for BrightnessGrid {
         let brightnesses = value
             .iter()
             .map(|b| Brightness::try_from(*b))
-            .collect::<Result<Vec<Brightness>, _>>();
+            .collect::<Result<Vec<Brightness>, _>>()?;
-        let brightnesses = match brightnesses {
-            Ok(vec) => vec,
-            Err(u8) => return Err(u8),
-        };
         Ok(BrightnessGrid::load(
             value.width(),
             value.height(),
@@ -110,6 +106,6 @@ impl TryFrom<PrimitiveGrid<u8>> for BrightnessGrid {
 #[cfg(feature = "rand")]
 impl Distribution<Brightness> for Standard {
     fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Brightness {
-        Brightness(rng.gen_range(Brightness::MIN.0..(Brightness::MAX.0 + 1)))
+        Brightness(rng.gen_range(Brightness::MIN.0..=Brightness::MAX.0))
     }
 }

crates/servicepoint/src/command.rs

@@ -1,10 +1,9 @@
 use bitvec::prelude::BitVec;
 
 use crate::{
-    command_code::CommandCode,
+    command_code::CommandCode, compression::into_decompressed, Brightness,
-    compression::{into_compressed, into_decompressed},
+    BrightnessGrid, CompressionCode, Header, Origin, Packet, PixelGrid, Pixels,
-    Brightness, BrightnessGrid, CompressionCode, Grid, Header, Origin, Packet,
+    PrimitiveGrid, SpBitVec, Tiles, TILE_SIZE,
-    PixelGrid, Pixels, PrimitiveGrid, SpBitVec, Tiles, TILE_SIZE,
 };
 
 /// Type alias for documenting the meaning of the u16 in enum values
@@ -166,124 +165,6 @@ pub enum Command {
     BitmapLegacy,
 }
 
-impl From<Command> for Packet {
-    /// Move the `Command` into a `Packet` instance for sending.
-    #[allow(clippy::cast_possible_truncation)]
-    fn from(value: Command) -> Self {
-        match value {
-            Command::Clear => Command::command_code_only(CommandCode::Clear),
-            Command::FadeOut => {
-                Command::command_code_only(CommandCode::FadeOut)
-            }
-            Command::HardReset => {
-                Command::command_code_only(CommandCode::HardReset)
-            }
-            #[allow(deprecated)]
-            Command::BitmapLegacy => {
-                Command::command_code_only(CommandCode::BitmapLegacy)
-            }
-            Command::CharBrightness(origin, grid) => Packet(
-                Header(
-                    CommandCode::CharBrightness.into(),
-                    origin.x as u16,
-                    origin.y as u16,
-                    grid.width() as u16,
-                    grid.height() as u16,
-                ),
-                grid.into(),
-            ),
-            Command::Brightness(brightness) => Packet(
-                Header(
-                    CommandCode::Brightness.into(),
-                    0x00000,
-                    0x0000,
-                    0x0000,
-                    0x0000,
-                ),
-                vec![brightness.into()],
-            ),
-            Command::BitmapLinearWin(origin, pixels, compression) => {
-                bitmap_win_into_packet(origin, pixels, compression)
-            }
-            Command::BitmapLinear(offset, bits, compression) => {
-                Command::bitmap_linear_into_packet(
-                    CommandCode::BitmapLinear,
-                    offset,
-                    compression,
-                    bits.into(),
-                )
-            }
-            Command::BitmapLinearAnd(offset, bits, compression) => {
-                Command::bitmap_linear_into_packet(
-                    CommandCode::BitmapLinearAnd,
-                    offset,
-                    compression,
-                    bits.into(),
-                )
-            }
-            Command::BitmapLinearOr(offset, bits, compression) => {
-                Command::bitmap_linear_into_packet(
-                    CommandCode::BitmapLinearOr,
-                    offset,
-                    compression,
-                    bits.into(),
-                )
-            }
-            Command::BitmapLinearXor(offset, bits, compression) => {
-                Command::bitmap_linear_into_packet(
-                    CommandCode::BitmapLinearXor,
-                    offset,
-                    compression,
-                    bits.into(),
-                )
-            }
-            Command::Cp437Data(origin, grid) => Packet(
-                Header(
-                    CommandCode::Cp437Data.into(),
-                    origin.x as u16,
-                    origin.y as u16,
-                    grid.width() as u16,
-                    grid.height() as u16,
-                ),
-                grid.into(),
-            ),
-        }
-    }
-}
-
-#[allow(clippy::cast_possible_truncation)]
-fn bitmap_win_into_packet(
-    origin: Origin<Pixels>,
-    pixels: PixelGrid,
-    compression: CompressionCode,
-) -> Packet {
-    debug_assert_eq!(origin.x % 8, 0);
-    debug_assert_eq!(pixels.width() % 8, 0);
-
-    let tile_x = (origin.x / TILE_SIZE) as u16;
-    let tile_w = (pixels.width() / TILE_SIZE) as u16;
-    let pixel_h = pixels.height() as u16;
-    let payload = into_compressed(compression, pixels.into());
-    let command = match compression {
-        CompressionCode::Uncompressed => {
-            CommandCode::BitmapLinearWinUncompressed
-        }
-        #[cfg(feature = "compression_zlib")]
-        CompressionCode::Zlib => CommandCode::BitmapLinearWinZlib,
-        #[cfg(feature = "compression_bzip2")]
-        CompressionCode::Bzip2 => CommandCode::BitmapLinearWinBzip2,
-        #[cfg(feature = "compression_lzma")]
-        CompressionCode::Lzma => CommandCode::BitmapLinearWinLzma,
-        #[cfg(feature = "compression_zstd")]
-        CompressionCode::Zstd => CommandCode::BitmapLinearWinZstd,
-    };
-
-    Packet(
-        Header(command.into(), tile_x, origin.y as u16, tile_w, pixel_h),
-        payload,
-    )
-}
-
 #[derive(Debug)]
 /// Err values for `Command::try_from`.
 #[derive(PartialEq)]
@@ -309,7 +190,7 @@ impl TryFrom<Packet> for Command {
 
     /// Try to interpret the `Packet` as one containing a `Command`
     fn try_from(packet: Packet) -> Result<Self, Self::Error> {
-        let Packet(Header(command_u16, a, b, c, d), _) = packet;
+        let Packet(Header(command_u16, a, _, _, _), _) = packet;
         let command_code = match CommandCode::try_from(command_u16) {
             Err(()) => {
                 return Err(TryFromPacketError::InvalidCommand(command_u16));
@@ -318,62 +199,19 @@ impl TryFrom<Packet> for Command {
         };
 
         match command_code {
-            CommandCode::Clear => match Self::check_command_only(packet) {
+            CommandCode::Clear => {
-                Some(err) => Err(err),
+                Self::packet_into_command_only(packet, Command::Clear)
-                None => Ok(Command::Clear),
-            },
-            CommandCode::Brightness => {
-                let Packet(header, payload) = packet;
-                if payload.len() != 1 {
-                    return Err(TryFromPacketError::UnexpectedPayloadSize(
-                        1,
-                        payload.len(),
-                    ));
             }
-
+            CommandCode::Brightness => Self::packet_into_brightness(&packet),
-                let Header(_, a, b, c, d) = header;
+            CommandCode::HardReset => {
-                if a != 0 || b != 0 || c != 0 || d != 0 {
+                Self::packet_into_command_only(packet, Command::HardReset)
-                    return Err(TryFromPacketError::ExtraneousHeaderValues);
             }
-
+            CommandCode::FadeOut => {
-                match Brightness::try_from(payload[0]) {
+                Self::packet_into_command_only(packet, Command::FadeOut)
-                    Ok(b) => Ok(Command::Brightness(b)),
-                    Err(_) => {
-                        Err(TryFromPacketError::InvalidBrightness(payload[0]))
-                    }
-                }
-            }
-            CommandCode::HardReset => match Self::check_command_only(packet) {
-                Some(err) => Err(err),
-                None => Ok(Command::HardReset),
-            },
-            CommandCode::FadeOut => match Self::check_command_only(packet) {
-                Some(err) => Err(err),
-                None => Ok(Command::FadeOut),
-            },
-            CommandCode::Cp437Data => {
-                let Packet(_, payload) = packet;
-                Ok(Command::Cp437Data(
-                    Origin::new(a as usize, b as usize),
-                    Cp437Grid::load(c as usize, d as usize, &payload),
-                ))
             }
+            CommandCode::Cp437Data => Self::packet_into_cp437(&packet),
             CommandCode::CharBrightness => {
-                let Packet(_, payload) = packet;
+                Self::packet_into_char_brightness(&packet)
-
-                let grid =
-                    PrimitiveGrid::load(c as usize, d as usize, &payload);
-                let grid = match BrightnessGrid::try_from(grid) {
-                    Ok(grid) => grid,
-                    Err(val) => {
-                        return Err(TryFromPacketError::InvalidBrightness(val))
-                    }
-                };
-
-                Ok(Command::CharBrightness(
-                    Origin::new(a as usize, b as usize),
-                    grid,
-                ))
             }
             #[allow(deprecated)]
             CommandCode::BitmapLegacy => Ok(Command::BitmapLegacy),
@@ -447,42 +285,18 @@ impl Command {
         ))
     }
 
-    /// Helper method for `BitMapLinear*`-Commands into `Packet`
-    #[allow(clippy::cast_possible_truncation)]
-    fn bitmap_linear_into_packet(
-        command: CommandCode,
-        offset: Offset,
-        compression: CompressionCode,
-        payload: Vec<u8>,
-    ) -> Packet {
-        let length = payload.len() as u16;
-        let payload = into_compressed(compression, payload);
-        Packet(
-            Header(
-                command.into(),
-                offset as u16,
-                length,
-                compression.into(),
-                0,
-            ),
-            payload,
-        )
-    }
-
-    /// Helper method for creating empty packets only containing the command code
-    fn command_code_only(code: CommandCode) -> Packet {
-        Packet(Header(code.into(), 0x0000, 0x0000, 0x0000, 0x0000), vec![])
-    }
-
     /// Helper method for checking that a packet is empty and only contains a command code
-    fn check_command_only(packet: Packet) -> Option<TryFromPacketError> {
+    fn packet_into_command_only(
+        packet: Packet,
+        command: Command,
+    ) -> Result<Command, TryFromPacketError> {
         let Packet(Header(_, a, b, c, d), payload) = packet;
         if !payload.is_empty() {
-            Some(TryFromPacketError::UnexpectedPayloadSize(0, payload.len()))
+            Err(TryFromPacketError::UnexpectedPayloadSize(0, payload.len()))
         } else if a != 0 || b != 0 || c != 0 || d != 0 {
-            Some(TryFromPacketError::ExtraneousHeaderValues)
+            Err(TryFromPacketError::ExtraneousHeaderValues)
         } else {
-            None
+            Ok(command)
         }
     }
 
@@ -512,6 +326,55 @@ impl Command {
         }
         Ok((BitVec::from_vec(payload), sub))
     }
+
+    fn packet_into_char_brightness(
+        packet: &Packet,
+    ) -> Result<Command, TryFromPacketError> {
+        let Packet(Header(_, x, y, width, height), payload) = packet;
+
+        let grid =
+            PrimitiveGrid::load(*width as usize, *height as usize, payload);
+        let grid = match BrightnessGrid::try_from(grid) {
+            Ok(grid) => grid,
+            Err(val) => return Err(TryFromPacketError::InvalidBrightness(val)),
+        };
+
+        Ok(Command::CharBrightness(
+            Origin::new(*x as usize, *y as usize),
+            grid,
+        ))
+    }
+
+    fn packet_into_brightness(
+        packet: &Packet,
+    ) -> Result<Command, TryFromPacketError> {
+        let Packet(Header(_, a, b, c, d), payload) = packet;
+        if payload.len() != 1 {
+            return Err(TryFromPacketError::UnexpectedPayloadSize(
+                1,
+                payload.len(),
+            ));
+        }
+
+        if *a != 0 || *b != 0 || *c != 0 || *d != 0 {
+            return Err(TryFromPacketError::ExtraneousHeaderValues);
+        }
+
+        match Brightness::try_from(payload[0]) {
+            Ok(b) => Ok(Command::Brightness(b)),
+            Err(_) => Err(TryFromPacketError::InvalidBrightness(payload[0])),
+        }
+    }
+
+    fn packet_into_cp437(
+        packet: &Packet,
+    ) -> Result<Command, TryFromPacketError> {
+        let Packet(Header(_, a, b, c, d), payload) = packet;
+        Ok(Command::Cp437Data(
+            Origin::new(*a as usize, *b as usize),
+            Cp437Grid::load(*c as usize, *d as usize, payload),
+        ))
+    }
 }
 
 #[cfg(test)]

crates/servicepoint/src/origin.rs

@@ -16,7 +16,7 @@ impl<Unit: DisplayUnit> Origin<Unit> {
         Self {
             x,
             y,
-            phantom_data: PhantomData::default(),
+            phantom_data: PhantomData,
         }
     }
 }
@@ -28,7 +28,7 @@ impl<T: DisplayUnit> std::ops::Add<Origin<T>> for Origin<T> {
         Origin {
             x: self.x + rhs.x,
             y: self.y + rhs.y,
-            phantom_data: PhantomData::default(),
+            phantom_data: PhantomData,
         }
     }
 }

crates/servicepoint/src/packet.rs

@@ -1,5 +1,12 @@
 use std::mem::size_of;
 
+use crate::command_code::CommandCode;
+use crate::compression::into_compressed;
+use crate::{
+    Command, CompressionCode, Grid, Offset, Origin, PixelGrid, Pixels,
+    TILE_SIZE,
+};
+
 /// A raw header. Should probably not be used directly.
 #[derive(Debug, PartialEq)]
 pub struct Header(pub u16, pub u16, pub u16, pub u16, pub u16);
@@ -58,6 +65,151 @@ impl TryFrom<&[u8]> for Packet {
     }
 }
 
+impl From<Command> for Packet {
+    /// Move the `Command` into a `Packet` instance for sending.
+    #[allow(clippy::cast_possible_truncation)]
+    fn from(value: Command) -> Self {
+        match value {
+            Command::Clear => Self::command_code_only(CommandCode::Clear),
+            Command::FadeOut => Self::command_code_only(CommandCode::FadeOut),
+            Command::HardReset => {
+                Self::command_code_only(CommandCode::HardReset)
+            }
+            #[allow(deprecated)]
+            Command::BitmapLegacy => {
+                Self::command_code_only(CommandCode::BitmapLegacy)
+            }
+            Command::CharBrightness(origin, grid) => Packet(
+                Header(
+                    CommandCode::CharBrightness.into(),
+                    origin.x as u16,
+                    origin.y as u16,
+                    grid.width() as u16,
+                    grid.height() as u16,
+                ),
+                grid.into(),
+            ),
+            Command::Brightness(brightness) => Packet(
+                Header(
+                    CommandCode::Brightness.into(),
+                    0x00000,
+                    0x0000,
+                    0x0000,
+                    0x0000,
+                ),
+                vec![brightness.into()],
+            ),
+            Command::BitmapLinearWin(origin, pixels, compression) => {
+                Self::bitmap_win_into_packet(origin, pixels, compression)
+            }
+            Command::BitmapLinear(offset, bits, compression) => {
+                Self::bitmap_linear_into_packet(
+                    CommandCode::BitmapLinear,
+                    offset,
+                    compression,
+                    bits.into(),
+                )
+            }
+            Command::BitmapLinearAnd(offset, bits, compression) => {
+                Self::bitmap_linear_into_packet(
+                    CommandCode::BitmapLinearAnd,
+                    offset,
+                    compression,
+                    bits.into(),
+                )
+            }
+            Command::BitmapLinearOr(offset, bits, compression) => {
+                Self::bitmap_linear_into_packet(
+                    CommandCode::BitmapLinearOr,
+                    offset,
+                    compression,
+                    bits.into(),
+                )
+            }
+            Command::BitmapLinearXor(offset, bits, compression) => {
+                Self::bitmap_linear_into_packet(
+                    CommandCode::BitmapLinearXor,
+                    offset,
+                    compression,
+                    bits.into(),
+                )
+            }
+            Command::Cp437Data(origin, grid) => Packet(
+                Header(
+                    CommandCode::Cp437Data.into(),
+                    origin.x as u16,
+                    origin.y as u16,
+                    grid.width() as u16,
+                    grid.height() as u16,
+                ),
+                grid.into(),
+            ),
+        }
+    }
+}
+
+impl Packet {
+    /// Helper method for `BitMapLinear*`-Commands into `Packet`
+    #[allow(clippy::cast_possible_truncation)]
+    fn bitmap_linear_into_packet(
+        command: CommandCode,
+        offset: Offset,
+        compression: CompressionCode,
+        payload: Vec<u8>,
+    ) -> Packet {
+        let length = payload.len() as u16;
+        let payload = into_compressed(compression, payload);
+        Packet(
+            Header(
+                command.into(),
+                offset as u16,
+                length,
+                compression.into(),
+                0,
+            ),
+            payload,
+        )
+    }
+
+    #[allow(clippy::cast_possible_truncation)]
+    fn bitmap_win_into_packet(
+        origin: Origin<Pixels>,
+        pixels: PixelGrid,
+        compression: CompressionCode,
+    ) -> Packet {
+        debug_assert_eq!(origin.x % 8, 0);
+        debug_assert_eq!(pixels.width() % 8, 0);
+
+        let tile_x = (origin.x / TILE_SIZE) as u16;
+        let tile_w = (pixels.width() / TILE_SIZE) as u16;
+        let pixel_h = pixels.height() as u16;
+        let payload = into_compressed(compression, pixels.into());
+        let command = match compression {
+            CompressionCode::Uncompressed => {
+                CommandCode::BitmapLinearWinUncompressed
+            }
+            #[cfg(feature = "compression_zlib")]
+            CompressionCode::Zlib => CommandCode::BitmapLinearWinZlib,
+            #[cfg(feature = "compression_bzip2")]
+            CompressionCode::Bzip2 => CommandCode::BitmapLinearWinBzip2,
+            #[cfg(feature = "compression_lzma")]
+            CompressionCode::Lzma => CommandCode::BitmapLinearWinLzma,
+            #[cfg(feature = "compression_zstd")]
+            CompressionCode::Zstd => CommandCode::BitmapLinearWinZstd,
+        };
+
+        Packet(
+            Header(command.into(), tile_x, origin.y as u16, tile_w, pixel_h),
+            payload,
+        )
+    }
+
+    /// Helper method for creating empty packets only containing the command code
+    fn command_code_only(code: CommandCode) -> Packet {
+        Packet(Header(code.into(), 0x0000, 0x0000, 0x0000, 0x0000), vec![])
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::{Header, Packet};

crates/servicepoint_binding_c/src/command.rs

@@ -5,8 +5,7 @@
 use std::ptr::null_mut;
 
 use servicepoint::{
-    Brightness, Command, CompressionCode, Offset,
+    Brightness, Command, CompressionCode, Offset, Origin, Packet, PixelGrid,
-    Origin, Packet, PixelGrid,
 };
 
 use crate::bit_vec::CBitVec;