Use num_enum instead of hand-rolling a parse function.

Also fixes the unit tests for neotron-bmc-protocol.

Author: thejpster

neotron-bmc-commands/Cargo.toml

@@ -6,3 +6,4 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+num_enum = { version = "0.5", default-features=false }

neotron-bmc-commands/src/lib.rs

@@ -4,7 +4,7 @@
 
 #![no_std]
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Copy, Clone, num_enum::IntoPrimitive, num_enum::TryFromPrimitive)]
 #[repr(u8)]
 pub enum Command {
 	/// # Protocol Version
@@ -138,37 +138,3 @@ pub enum Command {
 	/// * Mode: R/W
 	I2cBaudRate = 0x64,
 }
-
-impl Command {
-	pub fn parse(byte: u8) -> Option<Command> {
-		match byte {
-			0x00 => Some(Command::ProtocolVersion),
-			0x01 => Some(Command::FirmwareVersion),
-			0x10 => Some(Command::InterruptStatus),
-			0x11 => Some(Command::InterruptControl),
-			0x20 => Some(Command::ButtonStatus),
-			0x21 => Some(Command::SystemTemperature),
-			0x22 => Some(Command::SystemVoltage33S),
-			0x23 => Some(Command::SystemVoltage33),
-			0x24 => Some(Command::SystemVoltage55),
-			0x25 => Some(Command::PowerControl),
-			0x30 => Some(Command::UartBuffer),
-			0x31 => Some(Command::UartFifoControl),
-			0x32 => Some(Command::UartControl),
-			0x33 => Some(Command::UartStatus),
-			0x34 => Some(Command::UartBaudRate),
-			0x40 => Some(Command::Ps2KbBuffer),
-			0x41 => Some(Command::Ps2KbControl),
-			0x42 => Some(Command::Ps2KbStatus),
-			0x50 => Some(Command::Ps2MouseBuffer),
-			0x51 => Some(Command::Ps2MouseControl),
-			0x52 => Some(Command::Ps2MouseStatus),
-			0x60 => Some(Command::I2cBuffer),
-			0x61 => Some(Command::I2cFifoControl),
-			0x62 => Some(Command::I2cControl),
-			0x63 => Some(Command::I2cStatus),
-			0x64 => Some(Command::I2cBaudRate),
-			_ => None,
-		}
-	}
-}

neotron-bmc-pico/Cargo.toml

@@ -13,7 +13,7 @@ defmt-rtt = "0.4"
 heapless= "0.7"
 panic-probe = { version = "0.3", features = ["print-defmt"] }
 stm32f0xx-hal = { version = "0.18", features = ["stm32f030x6", "rt"] }
-neotron-bmc-protocol = { version = "0.1", path = "../neotron-bmc-protocol" }
+neotron-bmc-protocol = { version = "0.1", path = "../neotron-bmc-protocol", features = ["defmt"] }
 neotron-bmc-commands = { version = "0.1", path = "../neotron-bmc-commands" }
 systick-monotonic = "1.0"
 embedded-hal = "*"

neotron-bmc-pico/src/main.rs

@@ -11,6 +11,8 @@
 #![no_main]
 #![no_std]
 
+use core::convert::TryFrom;
+
 use heapless::spsc::{Consumer, Producer, Queue};
 use rtic::app;
 use stm32f0xx_hal::{
@@ -347,6 +349,7 @@ mod app {
 	/// This task is called when there is nothing else to do.
 	#[idle(shared = [msg_q_out, msg_q_in, spi])]
 	fn idle(mut ctx: idle::Context) -> ! {
+		// TODO: Get this from the VERSION static variable or from PKG_VERSION
 		let mut register_state = RegisterState {
 			firmware_version: *b"Neotron BMC v0.4.1-alpha\x00\x00\x00\x00\x00\x00\x00\x00",
 			..Default::default()
@@ -645,9 +648,9 @@ where
 	register_state.last_req = None;
 
 	// What do they want?
-	let rsp = match (req.request_type, Command::parse(req.register)) {
-		(proto::RequestType::Read, Some(Command::ProtocolVersion))
-		| (proto::RequestType::ReadAlt, Some(Command::ProtocolVersion)) => {
+	let rsp = match (req.request_type, Command::try_from(req.register)) {
+		(proto::RequestType::Read, Ok(Command::ProtocolVersion))
+		| (proto::RequestType::ReadAlt, Ok(Command::ProtocolVersion)) => {
 			defmt::debug!("Reading ProtocolVersion");
 			// They want the Protocol Version we support. Give them v0.1.1.
 			let length = req.length_or_data as usize;
@@ -658,8 +661,8 @@ where
 				proto::Response::new_without_data(proto::ResponseResult::BadLength)
 			}
 		}
-		(proto::RequestType::Read, Some(Command::FirmwareVersion))
-		| (proto::RequestType::ReadAlt, Some(Command::FirmwareVersion)) => {
+		(proto::RequestType::Read, Ok(Command::FirmwareVersion))
+		| (proto::RequestType::ReadAlt, Ok(Command::FirmwareVersion)) => {
 			defmt::debug!("Reading FirmwareVersion");
 			// They want the Firmware Version string.
 			let length = req.length_or_data as usize;
@@ -671,8 +674,8 @@ where
 				proto::Response::new_without_data(proto::ResponseResult::BadLength)
 			}
 		}
-		(proto::RequestType::Read, Some(Command::Ps2KbBuffer))
-		| (proto::RequestType::ReadAlt, Some(Command::Ps2KbBuffer)) => {
+		(proto::RequestType::Read, Ok(Command::Ps2KbBuffer))
+		| (proto::RequestType::ReadAlt, Ok(Command::Ps2KbBuffer)) => {
 			defmt::debug!("Reading Ps2KbBuffer");
 			let length = req.length_or_data as usize;
 			if length > 0 && length <= register_state.scratch.len() {

neotron-bmc-protocol/Cargo.toml

@@ -9,4 +9,8 @@ readme = "README.md"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-defmt = "0.3"
+defmt = { version = "0.3", optional = true }
+num_enum = { version = "0.5", default-features = false }
+
+[features]
+defmt = ["dep:defmt"]

neotron-bmc-protocol/src/lib.rs

@@ -5,6 +5,7 @@
 // Modules and Imports
 // ============================================================================
 
+#[cfg(feature = "defmt")]
 use defmt::Format;
 
 mod crc;
@@ -34,7 +35,8 @@ pub trait Receivable<'a>: Sized {
 // ============================================================================
 
 /// The ways this API can fail
-#[derive(Debug, Copy, Clone, Format, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(Format))]
 pub enum Error {
 	BadCrc,
 	BadLength,
@@ -44,8 +46,11 @@ pub enum Error {
 }
 
 /// The kinds of [`Request`] the *Host* can make to the NBMC
+#[derive(
+	Debug, Copy, Clone, PartialEq, Eq, num_enum::IntoPrimitive, num_enum::TryFromPrimitive,
+)]
+#[cfg_attr(feature = "defmt", derive(Format))]
 #[repr(u8)]
-#[derive(Debug, Copy, Clone, Format, PartialEq, Eq)]
 pub enum RequestType {
 	Read = 0xC0,
 	ReadAlt = 0xC1,
@@ -57,7 +62,11 @@ pub enum RequestType {
 
 /// The NBMC returns this code to indicate whether the previous [`Request`] was
 /// succesful or not.
-#[derive(Debug, Copy, Clone, Format, PartialEq, Eq)]
+#[derive(
+	Debug, Copy, Clone, PartialEq, Eq, num_enum::IntoPrimitive, num_enum::TryFromPrimitive,
+)]
+#[cfg_attr(feature = "defmt", derive(Format))]
+#[repr(u8)]
 pub enum ResponseResult {
 	/// The [`Request`] was correctly understood and actioned.
 	Ok = 0xA0,
@@ -85,7 +94,8 @@ pub enum ResponseResult {
 // ============================================================================
 
 /// A *Request* made by the *Host* to the *NBMC*
-#[derive(Debug, Clone, Format, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(Format))]
 pub struct Request {
 	pub request_type: RequestType,
 	pub register: u8,
@@ -94,7 +104,8 @@ pub struct Request {
 }
 
 /// A *Response* sent by the *NBMC* in reply to a [`Request`] from a *Host*
-#[derive(Debug, Clone, Format, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(Format))]
 pub struct Response<'a> {
 	pub result: ResponseResult,
 	pub data: &'a [u8],
@@ -103,7 +114,8 @@ pub struct Response<'a> {
 
 /// Describes the [semantic version](https://semver.org) of this implementation
 /// of the NBMC interface.
-#[derive(Debug, Copy, Clone, Format, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(Format))]
 pub struct ProtocolVersion {
 	major: u8,
 	minor: u8,
@@ -114,22 +126,6 @@ pub struct ProtocolVersion {
 // Impls
 // ============================================================================
 
-impl TryFrom<u8> for RequestType {
-	type Error = Error;
-
-	fn try_from(byte: u8) -> Result<Self, Error> {
-		match byte {
-			0xC0 => Ok(RequestType::Read),
-			0xC1 => Ok(RequestType::ReadAlt),
-			0xC2 => Ok(RequestType::ShortWrite),
-			0xC3 => Ok(RequestType::ShortWriteAlt),
-			0xC4 => Ok(RequestType::LongWrite),
-			0xC5 => Ok(RequestType::LongWriteAlt),
-			_ => Err(Error::BadRequestType),
-		}
-	}
-}
-
 impl Request {
 	/// Make a new Read Request, requesting the given register and number of
 	/// bytes.
@@ -208,6 +204,7 @@ impl Request {
 		]
 	}
 }
+
 impl Sendable for Request {
 	/// Convert to bytes for transmission.
 	///
@@ -223,6 +220,7 @@ impl Sendable for Request {
 		Ok(bytes.len())
 	}
 }
+
 impl<'a> Receivable<'a> for Request {
 	/// Convert from received bytes.
 	///
@@ -244,29 +242,14 @@ impl<'a> Receivable<'a> for Request {
 			return Err(Error::BadCrc);
 		}
 		Ok(Request {
-			request_type: data[0].try_into()?,
+			request_type: data[0].try_into().map_err(|_| Error::BadRequestType)?,
 			register: data[1],
 			length_or_data: data[2],
 			crc: data[3],
 		})
 	}
 }
 
-impl TryFrom<u8> for ResponseResult {
-	type Error = Error;
-
-	fn try_from(byte: u8) -> Result<Self, Error> {
-		match byte {
-			0xA0 => Ok(ResponseResult::Ok),
-			0xA1 => Ok(ResponseResult::CrcFailure),
-			0xA2 => Ok(ResponseResult::BadRequestType),
-			0xA3 => Ok(ResponseResult::BadRegister),
-			0xA4 => Ok(ResponseResult::BadLength),
-			_ => Err(Error::BadResponseResult),
-		}
-	}
-}
-
 impl<'a> Response<'a> {
 	/// Make a new OK response, with some optional data
 	pub fn new_ok_with_data(data: &'a [u8]) -> Response<'a> {
@@ -346,7 +329,7 @@ impl<'a> Receivable<'a> for Response<'a> {
 			return Err(Error::BadCrc);
 		}
 		Ok(Response {
-			result: data[0].try_into()?,
+			result: data[0].try_into().map_err(|_| Error::BadResponseResult)?,
 			data: &data[1..=(data.len() - 2)],
 			crc: data[data.len() - 1],
 		})