From b262a4afa5616a96a192355ffb5fc7567db80837 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Sat, 2 Nov 2024 09:44:26 +0100 Subject: [PATCH 1/5] Clean up and re-organize `lib.rs` a bit (sorry) --- esp-config/src/lib.rs | 61 +++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/esp-config/src/lib.rs b/esp-config/src/lib.rs index 941c420692e..2ef0c18f9a9 100644 --- a/esp-config/src/lib.rs +++ b/esp-config/src/lib.rs @@ -3,17 +3,30 @@ #![doc = document_features::document_features!(feature_label = r#"{feature}"#)] #![doc(html_logo_url = "https://avatars.githubusercontent.com/u/46717278")] #![cfg_attr(not(feature = "build"), no_std)] +#![deny(missing_docs, rust_2018_idioms)] #[cfg(feature = "build")] mod generate; #[cfg(feature = "build")] -pub use generate::*; +pub use generate::{generate_config, Error, Validator, Value}; +/// Parse the value of an environment variable as a [bool] at compile time. +#[macro_export] +macro_rules! esp_config_bool { + ( $var:expr ) => { + match env!($var).as_bytes() { + b"true" => true, + b"false" => false, + _ => ::core::panic!("boolean value must be either 'true' or 'false'"), + } + }; +} + +// TODO: From 1.82 on, we can use `<$ty>::from_str_radix(env!($var), 10)` +/// Parse the value of an environment variable as an integer at compile time. #[macro_export] -// TODO from 1.82 we can use <$ty>::from_str_radix(env!($var), 10) instead -/// Parse the value of a `env` variable as an integer at compile time macro_rules! esp_config_int { - ($ty:ty, $var:expr) => { + ( $ty:ty, $var:expr ) => { const { const BYTES: &[u8] = env!($var).as_bytes(); esp_config_int_parse!($ty, BYTES) @@ -21,63 +34,53 @@ macro_rules! esp_config_int { }; } +/// Get the string value of an environment variable at compile time. #[macro_export] -/// Get the string value of an `env` variable at compile time macro_rules! esp_config_str { - ($var:expr) => { + ( $var:expr ) => { env!($var) }; } -#[macro_export] -/// Parse the value of a `env` variable as an bool at compile time -macro_rules! esp_config_bool { - ($var:expr) => { - match env!($var).as_bytes() { - b"false" => false, - _ => true, - } - }; -} - -#[macro_export] -#[doc(hidden)] // to avoid confusion with esp_config_int, let's hide this /// Parse a string like "777" into an integer, which _can_ be used in a `const` /// context +#[doc(hidden)] // To avoid confusion with `esp_config_int`, hide this in the docs +#[macro_export] macro_rules! esp_config_int_parse { - ($ty:ty, $bytes:expr) => {{ + ( $ty:ty, $bytes:expr ) => {{ let mut bytes = $bytes; let mut val: $ty = 0; let mut sign_seen = false; let mut is_negative = false; + while let [byte, rest @ ..] = bytes { match *byte { b'0'..=b'9' => { val = val * 10 + (*byte - b'0') as $ty; } b'-' | b'+' if !sign_seen => { - if *byte == b'-' { - is_negative = true; - } + is_negative = *byte == b'-'; sign_seen = true; } - _ => ::core::panic!("invalid digit"), + _ => ::core::panic!("invalid character encountered while parsing integer"), } + bytes = rest; } + if is_negative { let original = val; - // subtract twice to get the negative + // Subtract the value twice to get a negative: val -= original; val -= original; } + val }}; } #[cfg(test)] mod test { - // We can only test success in the const context const _: () = { core::assert!(esp_config_int_parse!(i64, "-77777".as_bytes()) == -77777); @@ -98,4 +101,10 @@ mod test { fn test_expect_positive() { esp_config_int_parse!(u8, "-5".as_bytes()); } + + #[test] + #[should_panic] + fn test_invalid_digit() { + esp_config_int_parse!(u32, "a".as_bytes()); + } } From a70c3e041c0ce05bc026784c16fd0cf1b35797f6 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Sat, 2 Nov 2024 12:15:28 +0100 Subject: [PATCH 2/5] Add configuration validation, some other refactoring/improvements --- esp-config/src/generate.rs | 520 +++++++++++++++++++++++++++---------- 1 file changed, 384 insertions(+), 136 deletions(-) diff --git a/esp-config/src/generate.rs b/esp-config/src/generate.rs index 1afa1c92591..9e33752576d 100644 --- a/esp-config/src/generate.rs +++ b/esp-config/src/generate.rs @@ -1,102 +1,262 @@ -use core::fmt::Write; -use std::{collections::HashMap, env, fs, path::PathBuf}; +use std::{ + collections::HashMap, + env, + fmt::{self, Write as _}, + fs, + path::PathBuf, +}; const DOC_TABLE_HEADER: &str = r#" | Name | Description | Default value | |------|-------------|---------------| "#; -const CHOSEN_TABLE_HEADER: &str = r#" + +const SELECTED_TABLE_HEADER: &str = r#" | Name | Selected value | |------|----------------| "#; -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct ParseError(String); +/// Configuration errors. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Error { + /// Parse errors. + Parse(String), + /// Validation errors. + Validation(String), +} + +impl Error { + /// Convenience function for creating parse errors. + pub fn parse(message: S) -> Self + where + S: Into, + { + Self::Parse(message.into()) + } + + /// Convenience function for creating validation errors. + pub fn validation(message: S) -> Self + where + S: Into, + { + Self::Validation(message.into()) + } +} -#[derive(Clone, Debug, PartialEq, Eq)] +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Error::Parse(message) => write!(f, "{message}"), + Error::Validation(message) => write!(f, "{message}"), + } + } +} + +/// Supported configuration value types. +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Value { - SignedInteger(isize), - UnsignedInteger(usize), + /// Booleans. Bool(bool), + /// Integers. + Integer(i128), + /// Strings. String(String), } +// TODO: Do we want to handle negative values for non-decimal values? impl Value { - fn parse_in_place(&mut self, s: &str) -> Result<(), ParseError> { + fn parse_in_place(&mut self, s: &str) -> Result<(), Error> { *self = match self { - Value::Bool(_) => match s.to_lowercase().as_ref() { - "false" | "no" | "n" | "f" => Value::Bool(false), - "true" | "yes" | "y" | "t" => Value::Bool(true), - _ => return Err(ParseError(format!("Invalid boolean value: {}", s))), - }, - Value::SignedInteger(_) => Value::SignedInteger( - match s.as_bytes() { - [b'0', b'x', ..] => isize::from_str_radix(&s[2..], 16), - [b'0', b'o', ..] => isize::from_str_radix(&s[2..], 8), - [b'0', b'b', ..] => isize::from_str_radix(&s[2..], 2), - _ => isize::from_str_radix(&s, 10), + Value::Bool(_) => match s { + "true" => Value::Bool(true), + "false" => Value::Bool(false), + _ => { + return Err(Error::parse(format!( + "Expected 'true' or 'false', found: '{s}'" + ))) } - .map_err(|_| ParseError(format!("Invalid signed numerical value: {}", s)))?, - ), - Value::UnsignedInteger(_) => Value::UnsignedInteger( - match s.as_bytes() { - [b'0', b'x', ..] => usize::from_str_radix(&s[2..], 16), - [b'0', b'o', ..] => usize::from_str_radix(&s[2..], 8), - [b'0', b'b', ..] => usize::from_str_radix(&s[2..], 2), - _ => usize::from_str_radix(&s, 10), + }, + Value::Integer(_) => { + let inner = match s.as_bytes() { + [b'0', b'x', ..] => i128::from_str_radix(&s[2..], 16), + [b'0', b'o', ..] => i128::from_str_radix(&s[2..], 8), + [b'0', b'b', ..] => i128::from_str_radix(&s[2..], 2), + _ => i128::from_str_radix(&s, 10), } - .map_err(|_| ParseError(format!("Invalid unsigned numerical value: {}", s)))?, - ), - Value::String(_) => Value::String(String::from(s)), + .map_err(|_| Error::parse(format!("Expected valid intger value, found: '{s}'")))?; + + Value::Integer(inner) + } + Value::String(_) => Value::String(s.into()), }; + Ok(()) } - fn as_string(&self) -> String { + /// Convert the value to a [bool]. + pub fn as_bool(&self) -> bool { match self { - Value::Bool(value) => String::from(if *value { "true" } else { "false" }), - Value::SignedInteger(value) => format!("{}", value), - Value::UnsignedInteger(value) => format!("{}", value), - Value::String(value) => value.clone(), + Value::Bool(value) => *value, + _ => panic!("attempted to convert non-bool value to a bool"), } } + + /// Convert the value to an [i128]. + pub fn as_integer(&self) -> i128 { + match self { + Value::Integer(value) => *value, + _ => panic!("attempted to convert non-integer value to an integer"), + } + } + + /// Convert the value to a [String]. + pub fn as_string(&self) -> String { + match self { + Value::String(value) => value.to_owned(), + _ => panic!("attempted to convert non-string value to a string"), + } + } + + /// Is the value a bool? + pub fn is_bool(&self) -> bool { + matches!(self, Value::Bool(_)) + } + + /// Is the value an integer? + pub fn is_integer(&self) -> bool { + matches!(self, Value::Integer(_)) + } + + /// Is the value a string? + pub fn is_string(&self) -> bool { + matches!(self, Value::String(_)) + } } -/// Generate and parse config from a prefix, and array of key, default, -/// description tuples. -/// -/// This function will parse any `SCREAMING_SNAKE_CASE` environment variables -/// that match the given prefix. It will then attempt to parse the [`Value`]. -/// Once the config has been parsed, this function will emit `snake_case` cfg's -/// _without_ the prefix which can be used in the dependant crate. After that, -/// it will create a markdown table in the `OUT_DIR` under the name -/// `{prefix}_config_table.md` where prefix has also been converted -/// to `snake_case`. This can be included in crate documentation to outline the -/// available configuration options for the crate. -/// -/// Passing a value of true for the `emit_md_tables` argument will create and -/// write markdown files of the available configuration and selected -/// configuration which can be included in documentation. -/// -/// Unknown keys with the supplied prefix will cause this function to panic. +impl fmt::Display for Value { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Value::Bool(b) => write!(f, "{b}"), + Value::Integer(i) => write!(f, "{i}"), + Value::String(s) => write!(f, "{s}"), + } + } +} + +/// Configuration value validation functions. +pub enum Validator { + /// Only allow negative integers, i.e. any values less than 0. + NegativeInteger, + /// Only allow non-negative integers, i.e. any values greater than or equal + /// to 0. + NonNegativeInteger, + /// Only allow positive integers, i.e. any values greater than to 0. + PositiveInteger, + /// A custom validation function to run against any supported value type. + Custom(Box Result<(), Error>>), +} + +impl Validator { + fn validate(&self, value: &Value) -> Result<(), Error> { + match self { + Validator::NegativeInteger => negative_integer(value)?, + Validator::NonNegativeInteger => non_negative_integer(value)?, + Validator::PositiveInteger => positive_integer(value)?, + Validator::Custom(validator_fn) => validator_fn(value)?, + } + + Ok(()) + } +} + +fn negative_integer(value: &Value) -> Result<(), Error> { + if !value.is_integer() { + return Err(Error::validation( + "Validator::NegativeInteger can only be used with integer values", + )); + } else if value.as_integer() >= 0 { + return Err(Error::validation(format!( + "Expected negative integer, found '{}'", + value.as_integer() + ))); + } + + Ok(()) +} + +fn non_negative_integer(value: &Value) -> Result<(), Error> { + if !value.is_integer() { + return Err(Error::validation( + "Validator::NonNegativeInteger can only be used with integer values", + )); + } else if value.as_integer() < 0 { + return Err(Error::validation(format!( + "Expected non-negative integer, found '{}'", + value.as_integer() + ))); + } + + Ok(()) +} + +fn positive_integer(value: &Value) -> Result<(), Error> { + if !value.is_integer() { + return Err(Error::validation( + "Validator::PositiveInteger can only be used with integer values", + )); + } else if value.as_integer() <= 0 { + return Err(Error::validation(format!( + "Expected positive integer, found '{}'", + value.as_integer() + ))); + } + + Ok(()) +} + +/// TODO: Document me! pub fn generate_config( prefix: &str, - config: &[(&str, Value, &str)], + config: &[(&str, &str, Value, Option)], emit_md_tables: bool, ) -> HashMap { - // only rebuild if build.rs changed. Otherwise Cargo will rebuild if any + // Only rebuild if `build.rs` changed. Otherwise, Cargo will rebuild if any // other file changed. println!("cargo:rerun-if-changed=build.rs"); + #[cfg(not(test))] env_change_work_around(); - // ensure that the prefix is `SCREAMING_SNAKE_CASE` - let prefix = format!("{}_", screaming_snake_case(prefix)); let mut doc_table = String::from(DOC_TABLE_HEADER); - let mut selected_config = String::from(CHOSEN_TABLE_HEADER); + let mut selected_config = String::from(SELECTED_TABLE_HEADER); + + // Ensure that the prefix is `SCREAMING_SNAKE_CASE`: + let prefix = screaming_snake_case(prefix); + + // Build a lookup table for any provided validators; we must prefix the + // name of the config and transform it to SCREAMING_SNAKE_CASE so that + // it matches the keys in the hash table produced by `create_config`. + let config_validators = config + .iter() + .flat_map(|(name, _description, _default, validator)| { + if let Some(validator) = validator { + let name = format!("{prefix}_{}", screaming_snake_case(name)); + Some((name, validator)) + } else { + None + } + }) + .collect::>(); let mut configs = create_config(&prefix, config, &mut doc_table); capture_from_env(&prefix, &mut configs); + + for (name, value) in configs.iter() { + if let Some(validator) = config_validators.get(name) { + validator.validate(value).unwrap(); + } + } + emit_configuration(&prefix, &configs, &mut selected_config); if emit_md_tables { @@ -118,8 +278,7 @@ fn env_change_work_around() { // target while !out_dir.ends_with("target") { if !out_dir.pop() { - // We ran out of directories... - return; + return; // We ran out of directories... } } out_dir.pop(); @@ -141,44 +300,42 @@ fn env_change_work_around() { } } -fn emit_configuration( +fn create_config( prefix: &str, - configs: &HashMap, - selected_config: &mut String, -) { - // emit cfgs and set envs - for (name, value) in configs.into_iter() { - let cfg_name = snake_case(name.trim_start_matches(prefix)); - println!("cargo:rustc-check-cfg=cfg({cfg_name})"); - match value { - Value::Bool(true) => { - println!("cargo:rustc-cfg={cfg_name}") - } - _ => {} - } + config: &[(&str, &str, Value, Option)], + doc_table: &mut String, +) -> HashMap { + let mut configs = HashMap::new(); - let value = value.as_string(); - // values that haven't been seen will be output here with the default value - println!("cargo:rustc-env={}={}", name, value); + for (name, description, default, _validator) in config { + let name = format!("{prefix}_{}", screaming_snake_case(name)); + configs.insert(name.clone(), default.clone()); - writeln!(selected_config, "|**{name}**|{value}|").unwrap(); + // Write documentation table line: + let default = default.to_string(); + writeln!(doc_table, "|**{name}**|{description}|{default}|").unwrap(); + + // Rebuild if config environment variable changed: + println!("cargo:rerun-if-env-changed={name}"); } + + configs } fn capture_from_env(prefix: &str, configs: &mut HashMap) { let mut unknown = Vec::new(); let mut failed = Vec::new(); - // Try and capture input from the environment + // Try and capture input from the environment: for (var, value) in env::vars() { - if let Some(_) = var.strip_prefix(prefix) { + if var.strip_prefix(prefix).is_some() { let Some(cfg) = configs.get_mut(&var) else { unknown.push(var); continue; }; if let Err(e) = cfg.parse_in_place(&value) { - failed.push(format!("{}: {e:?}", var)); + failed.push(format!("{var}: {e}")); } } } @@ -192,61 +349,54 @@ fn capture_from_env(prefix: &str, configs: &mut HashMap) { } } -fn create_config( +fn emit_configuration( prefix: &str, - config: &[(&str, Value, &str)], - doc_table: &mut String, -) -> HashMap { - // Generate the template for the config - let mut configs = HashMap::new(); - for (name, default, desc) in config { - let name = format!("{prefix}{}", screaming_snake_case(&name)); - configs.insert(name.clone(), default.clone()); + configs: &HashMap, + selected_config: &mut String, +) { + for (name, value) in configs.iter() { + let cfg_name = snake_case(name.trim_start_matches(prefix)); + println!("cargo:rustc-check-cfg=cfg({cfg_name})"); - // write doc table line - let default = default.as_string(); - writeln!(doc_table, "|**{name}**|{desc}|{default}|").unwrap(); + if let Value::Bool(true) = value { + println!("cargo:rustc-cfg={cfg_name}"); + } - // Rebuild if config envvar changed. - println!("cargo:rerun-if-env-changed={name}"); - } + let value = value.to_string(); - configs + // Values that haven't been seen will be output here with the default value: + println!("cargo:rustc-env={}={}", name, value); + writeln!(selected_config, "|**{name}**|{value}|").unwrap(); + } } fn write_config_tables(prefix: &str, doc_table: String, selected_config: String) { let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + let out_file = out_dir - .join(format!("{prefix}config_table.md")) - .to_string_lossy() + .join(format!("{prefix}_config_table.md")) + .display() .to_string(); fs::write(out_file, doc_table).unwrap(); - let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); let out_file = out_dir - .join(format!("{prefix}selected_config.md")) - .to_string_lossy() + .join(format!("{prefix}_selected_config.md")) + .display() .to_string(); fs::write(out_file, selected_config).unwrap(); } -// Converts a symbol name like -// "PLACE-spi_DRIVER-IN_ram" -// to -// "place_spi_driver_in_ram" fn snake_case(name: &str) -> String { let mut name = name.replace("-", "_"); name.make_ascii_lowercase(); + name } -// Converts a symbol name like -// "PLACE-spi_DRIVER-IN_ram" -// to -// "PLACE_SPI_DRIVER_IN_RAM" fn screaming_snake_case(name: &str) -> String { let mut name = name.replace("-", "_"); name.make_ascii_uppercase(); + name } @@ -257,32 +407,24 @@ mod test { #[test] fn value_number_formats() { const INPUTS: &[&str] = &["0xAA", "0o252", "0b0000000010101010", "170"]; - let mut v = Value::SignedInteger(0); + let mut v = Value::Integer(0); for input in INPUTS { v.parse_in_place(input).unwrap(); // no matter the input format, the output format should be decimal - assert_eq!(v.as_string(), "170"); + assert_eq!(format!("{v}"), "170"); } } #[test] fn value_bool_inputs() { - const TRUE_INPUTS: &[&str] = &["true", "y", "yes"]; - const FALSE_INPUTS: &[&str] = &["false", "n", "no"]; let mut v = Value::Bool(false); - for input in TRUE_INPUTS { - v.parse_in_place(input).unwrap(); - // no matter the input variant, the output format should be "true" - assert_eq!(v.as_string(), "true"); - } + v.parse_in_place("true").unwrap(); + assert_eq!(format!("{v}"), "true"); - for input in FALSE_INPUTS { - v.parse_in_place(input).unwrap(); - // no matter the input variant, the output format should be "false" - assert_eq!(v.as_string(), "false"); - } + v.parse_in_place("false").unwrap(); + assert_eq!(format!("{v}"), "false"); } #[test] @@ -298,13 +440,18 @@ mod test { let configs = generate_config( "esp-test", &[ - ("number", Value::UnsignedInteger(999), "NA"), - ("number_signed", Value::SignedInteger(-777), "NA"), - ("string", Value::String("Demo".to_owned()), "NA"), - ("bool", Value::Bool(false), "NA"), - ("number_default", Value::UnsignedInteger(999), "NA"), - ("string_default", Value::String("Demo".to_owned()), "NA"), - ("bool_default", Value::Bool(false), "NA"), + ("number", "NA", Value::Integer(999), None), + ("number_signed", "NA", Value::Integer(-777), None), + ("string", "NA", Value::String("Demo".to_owned()), None), + ("bool", "NA", Value::Bool(false), None), + ("number_default", "NA", Value::Integer(999), None), + ( + "string_default", + "NA", + Value::String("Demo".to_owned()), + None, + ), + ("bool_default", "NA", Value::Bool(false), None), ], false, ); @@ -312,14 +459,14 @@ mod test { // some values have changed assert_eq!( match configs.get("ESP_TEST_NUMBER").unwrap() { - Value::UnsignedInteger(num) => *num, + Value::Integer(num) => *num, _ => unreachable!(), }, 0xaa ); assert_eq!( match configs.get("ESP_TEST_NUMBER_SIGNED").unwrap() { - Value::SignedInteger(num) => *num, + Value::Integer(num) => *num, _ => unreachable!(), }, -999 @@ -342,7 +489,7 @@ mod test { // the rest are the defaults assert_eq!( match configs.get("ESP_TEST_NUMBER_DEFAULT").unwrap() { - Value::UnsignedInteger(num) => *num, + Value::Integer(num) => *num, _ => unreachable!(), }, 999 @@ -365,6 +512,107 @@ mod test { ) } + #[test] + fn builtin_validation_passes() { + temp_env::with_vars( + [ + ("ESP_TEST_POSITIVE_NUMBER", Some("7")), + ("ESP_TEST_NEGATIVE_NUMBER", Some("-1")), + ("ESP_TEST_NON_NEGATIVE_NUMBER", Some("0")), + ], + || { + generate_config( + "esp-test", + &[ + ( + "positive_number", + "NA", + Value::Integer(-1), + Some(Validator::PositiveInteger), + ), + ( + "negative_number", + "NA", + Value::Integer(1), + Some(Validator::NegativeInteger), + ), + ( + "non_negative_number", + "NA", + Value::Integer(-1), + Some(Validator::NonNegativeInteger), + ), + ], + false, + ) + }, + ); + } + + #[test] + fn custom_validation_passes() { + temp_env::with_vars([("ESP_TEST_NUMBER", Some("13"))], || { + generate_config( + "esp-test", + &[( + "number", + "NA", + Value::Integer(-1), + Some(Validator::Custom(Box::new(|value| { + let range = 10..20; + if !value.is_integer() || !range.contains(&value.as_integer()) { + Err(Error::validation("value does not fall within range")) + } else { + Ok(()) + } + }))), + )], + false, + ) + }); + } + + #[test] + #[should_panic] + fn builtin_validation_bails() { + temp_env::with_vars([("ESP_TEST_POSITIVE_NUMBER", Some("-99"))], || { + generate_config( + "esp-test", + &[( + "positive_number", + "NA", + Value::Integer(-1), + Some(Validator::PositiveInteger), + )], + false, + ) + }); + } + + #[test] + #[should_panic] + fn custom_validation_bails() { + temp_env::with_vars([("ESP_TEST_NUMBER", Some("37"))], || { + generate_config( + "esp-test", + &[( + "number", + "NA", + Value::Integer(-1), + Some(Validator::Custom(Box::new(|value| { + let range = 10..20; + if !value.is_integer() || !range.contains(&value.as_integer()) { + Err(Error::validation("value does not fall within range")) + } else { + Ok(()) + } + }))), + )], + false, + ) + }); + } + #[test] #[should_panic] fn env_unknown_bails() { @@ -376,7 +624,7 @@ mod test { || { generate_config( "esp-test", - &[("number", Value::UnsignedInteger(999), "NA")], + &[("number", "NA", Value::Integer(999), None)], false, ); }, @@ -389,7 +637,7 @@ mod test { temp_env::with_vars([("ESP_TEST_NUMBER", Some("Hello world"))], || { generate_config( "esp-test", - &[("number", Value::UnsignedInteger(999), "NA")], + &[("number", "NA", Value::Integer(999), None)], false, ); }); From ba709ab4bc1bda0d9122918b32622a7afb4dcb39 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Sat, 2 Nov 2024 12:15:43 +0100 Subject: [PATCH 3/5] Update build scripts to reflect new configuration API --- esp-hal-embassy/build.rs | 3 +- esp-hal/build.rs | 12 ++-- esp-ieee802154/build.rs | 5 +- esp-wifi/build.rs | 124 ++++++++++++++++++++++++++++++++------- 4 files changed, 116 insertions(+), 28 deletions(-) diff --git a/esp-hal-embassy/build.rs b/esp-hal-embassy/build.rs index ff1606cbeef..5a4cf918864 100644 --- a/esp-hal-embassy/build.rs +++ b/esp-hal-embassy/build.rs @@ -43,8 +43,9 @@ fn main() -> Result<(), Box> { "esp_hal_embassy", &[( "low-power-wait", - Value::Bool(true), "Enables the lower-power wait if no tasks are ready to run on the thread-mode executor. This allows the MCU to use less power if the workload allows. Recommended for battery-powered systems. May impact analog performance.", + Value::Bool(true), + None )], true, ); diff --git a/esp-hal/build.rs b/esp-hal/build.rs index c507a97bd13..9e0f49684c0 100644 --- a/esp-hal/build.rs +++ b/esp-hal/build.rs @@ -80,23 +80,27 @@ fn main() -> Result<(), Box> { &[ ( "place-spi-driver-in-ram", - Value::Bool(false), "Places the SPI driver in RAM for better performance", + Value::Bool(false), + None ), ( "spi-address-workaround", - Value::Bool(true), "(ESP32 only) Enables a workaround for the issue where SPI in half-duplex mode incorrectly transmits the address on a single line if the data buffer is empty.", + Value::Bool(true), + None ), ( "place-switch-tables-in-ram", - Value::Bool(true), "Places switch-tables, some lookup tables and constants related to interrupt handling into RAM - resulting in better performance but slightly more RAM consumption.", + Value::Bool(true), + None ), ( "place-anon-in-ram", - Value::Bool(false), "Places anonymous symbols into RAM - resulting in better performance at the cost of significant more RAM consumption. Best to be combined with `place-switch-tables-in-ram`.", + Value::Bool(false), + None ), ], true, diff --git a/esp-ieee802154/build.rs b/esp-ieee802154/build.rs index f20eeeab3aa..ee788fa6433 100644 --- a/esp-ieee802154/build.rs +++ b/esp-ieee802154/build.rs @@ -1,6 +1,6 @@ use std::{env, path::PathBuf}; -use esp_config::{generate_config, Value}; +use esp_config::{generate_config, Validator, Value}; fn main() { let out = PathBuf::from(env::var_os("OUT_DIR").unwrap()); @@ -11,8 +11,9 @@ fn main() { "esp_ieee802154", &[( "rx_queue_size", - Value::UnsignedInteger(50), "Size of the RX queue in frames", + Value::Integer(50), + Some(Validator::PositiveInteger), )], true, ); diff --git a/esp-wifi/build.rs b/esp-wifi/build.rs index 5197c6ffb41..1eae798b634 100644 --- a/esp-wifi/build.rs +++ b/esp-wifi/build.rs @@ -1,7 +1,7 @@ use std::{error::Error, str::FromStr}; use esp_build::assert_unique_used_features; -use esp_config::{generate_config, Value}; +use esp_config::{define, generate_config, Value}; use esp_metadata::{Chip, Config}; fn main() -> Result<(), Box> { @@ -99,34 +99,116 @@ fn main() -> Result<(), Box> { generate_config( "esp_wifi", &[ - ("rx_queue_size", Value::UnsignedInteger(5), "Size of the RX queue in frames"), - ("tx_queue_size", Value::UnsignedInteger(3), "Size of the TX queue in frames"), - ("static_rx_buf_num", Value::UnsignedInteger(10), "WiFi static RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("dynamic_rx_buf_num", Value::UnsignedInteger(32), "WiFi dynamic RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("static_tx_buf_num", Value::UnsignedInteger(0), "WiFi static TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("dynamic_tx_buf_num", Value::UnsignedInteger(32), "WiFi dynamic TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("ampdu_rx_enable", Value::Bool(true), "WiFi AMPDU RX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("ampdu_tx_enable", Value::Bool(true), "WiFi AMPDU TX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("amsdu_tx_enable", Value::Bool(false), "WiFi AMSDU TX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("rx_ba_win", Value::UnsignedInteger(6), "WiFi Block Ack RX window size. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)"), - ("max_burst_size", Value::UnsignedInteger(1), "See [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_burst_size)"), + ("rx_queue_size", "Size of the RX queue in frames", Value::UnsignedInteger(5), None), + ("tx_queue_size", "Size of the TX queue in frames", Value::UnsignedInteger(3), None), + ( + "static_rx_buf_num", + "WiFi static RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::UnsignedInteger(10), + None + ), + ( + "dynamic_rx_buf_num", + "WiFi dynamic RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::UnsignedInteger(32), + None + ), + ( + "static_tx_buf_num", + "WiFi static TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::UnsignedInteger(0), + None + ), + ( + "dynamic_tx_buf_num", + "WiFi dynamic TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::UnsignedInteger(32), + None + ), + ( + "ampdu_rx_enable", + "WiFi AMPDU RX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::Bool(true), + None + ), + ( + "ampdu_tx_enable", + "WiFi AMPDU TX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::Bool(true), + None + ), + ( + "amsdu_tx_enable", + "WiFi AMSDU TX feature enable flag. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::Bool(false), + None + ), + ( + "rx_ba_win", + "WiFi Block Ack RX window size. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", + Value::UnsignedInteger(6), + None + ), + ( + "max_burst_size", + "See [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_burst_size)", + Value::UnsignedInteger(1), + None + ), ( "country_code", - Value::String("CN".to_owned()), "Country code. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/wifi.html#wi-fi-country-code)", + Value::String("CN".to_owned()), + None ), ( "country_code_operating_class", - Value::UnsignedInteger(0), "If not 0: Operating Class table number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/wifi.html#wi-fi-country-code)", + Value::UnsignedInteger(0), + None + ), + ( + "mtu", + "MTU, see [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_transmission_unit)", + Value::UnsignedInteger(1492), + None + ), + ( + "tick_rate_hz", + "Tick rate of the internal task scheduler in hertz", + Value::UnsignedInteger(100), + None + ), + ( + "listen_interval", + "Interval for station to listen to beacon from AP. The unit of listen interval is one beacon interval. For example, if beacon interval is 100 ms and listen interval is 3, the interval for station to listen to beacon is 300 ms", + Value::UnsignedInteger(3), + None + ), + ( + "beacon_timeout", + "For Station, If the station does not receive a beacon frame from the connected SoftAP during the inactive time, disconnect from SoftAP. Default 6s. Range 6-30", + Value::UnsignedInteger(6), + None + ), + ( + "ap_beacon_timeout", + "For SoftAP, If the SoftAP doesn't receive any data from the connected STA during inactive time, the SoftAP will force deauth the STA. Default is 300s", + Value::UnsignedInteger(300), + None + ), + ( + "failure_retry_cnt", + "Number of connection retries station will do before moving to next AP. scan_method should be set as WIFI_ALL_CHANNEL_SCAN to use this config. Note: Enabling this may cause connection time to increase incase best AP doesn't behave properly. Defaults to 1", + Value::UnsignedInteger(1), + None + ), + ( + "scan_method", + "0 = WIFI_FAST_SCAN, 1 = WIFI_ALL_CHANNEL_SCAN, defaults to 0", + Value::UnsignedInteger(0), + None ), - ("mtu", Value::UnsignedInteger(1492), "MTU, see [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_transmission_unit)"), - ("tick_rate_hz", Value::UnsignedInteger(100), "Tick rate of the internal task scheduler in hertz"), - ("listen_interval", Value::UnsignedInteger(3), "Interval for station to listen to beacon from AP. The unit of listen interval is one beacon interval. For example, if beacon interval is 100 ms and listen interval is 3, the interval for station to listen to beacon is 300 ms"), - ("beacon_timeout", Value::UnsignedInteger(6), "For Station, If the station does not receive a beacon frame from the connected SoftAP during the inactive time, disconnect from SoftAP. Default 6s. Range 6-30"), - ("ap_beacon_timeout", Value::UnsignedInteger(300), "For SoftAP, If the SoftAP doesn’t receive any data from the connected STA during inactive time, the SoftAP will force deauth the STA. Default is 300s"), - ("failure_retry_cnt", Value::UnsignedInteger(1), "Number of connection retries station will do before moving to next AP. scan_method should be set as WIFI_ALL_CHANNEL_SCAN to use this config. Note: Enabling this may cause connection time to increase incase best AP doesn't behave properly. Defaults to 1"), - ("scan_method", Value::UnsignedInteger(0), "0 = WIFI_FAST_SCAN, 1 = WIFI_ALL_CHANNEL_SCAN, defaults to 0"), ], true ); From da281326b654eeb2fd8064531a8f4387a088a0e0 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Tue, 5 Nov 2024 12:36:03 +0100 Subject: [PATCH 4/5] Add a built-in range validator, re-document the `generate_config` function --- esp-config/src/generate.rs | 42 +++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/esp-config/src/generate.rs b/esp-config/src/generate.rs index 9e33752576d..1868cde1814 100644 --- a/esp-config/src/generate.rs +++ b/esp-config/src/generate.rs @@ -3,6 +3,7 @@ use std::{ env, fmt::{self, Write as _}, fs, + ops::Range, path::PathBuf, }; @@ -152,6 +153,8 @@ pub enum Validator { NonNegativeInteger, /// Only allow positive integers, i.e. any values greater than to 0. PositiveInteger, + /// Ensure that an integer value falls within the specified range. + IntegerInRange(Range), /// A custom validation function to run against any supported value type. Custom(Box Result<(), Error>>), } @@ -162,6 +165,7 @@ impl Validator { Validator::NegativeInteger => negative_integer(value)?, Validator::NonNegativeInteger => non_negative_integer(value)?, Validator::PositiveInteger => positive_integer(value)?, + Validator::IntegerInRange(range) => integer_in_range(range, value)?, Validator::Custom(validator_fn) => validator_fn(value)?, } @@ -214,7 +218,36 @@ fn positive_integer(value: &Value) -> Result<(), Error> { Ok(()) } -/// TODO: Document me! +fn integer_in_range(range: &Range, value: &Value) -> Result<(), Error> { + if !value.is_integer() || !range.contains(&value.as_integer()) { + Err(Error::validation(format!( + "Value '{}' does not fall within range '{:?}'", + value, range + ))) + } else { + Ok(()) + } +} + +/// Generate and parse config from a prefix, and an array tuples containing the +/// name, description, default value, and an optional validator. +/// +/// This function will parse any `SCREAMING_SNAKE_CASE` environment variables +/// that match the given prefix. It will then attempt to parse the [`Value`] and +/// run any validators which have been specified. +/// +/// Once the config has been parsed, this function will emit `snake_case` cfg's +/// _without_ the prefix which can be used in the dependant crate. After that, +/// it will create a markdown table in the `OUT_DIR` under the name +/// `{prefix}_config_table.md` where prefix has also been converted to +/// `snake_case`. This can be included in crate documentation to outline the +/// available configuration options for the crate. +/// +/// Passing a value of true for the `emit_md_tables` argument will create and +/// write markdown files of the available configuration and selected +/// configuration which can be included in documentation. +/// +/// Unknown keys with the supplied prefix will cause this function to panic. pub fn generate_config( prefix: &str, config: &[(&str, &str, Value, Option)], @@ -519,6 +552,7 @@ mod test { ("ESP_TEST_POSITIVE_NUMBER", Some("7")), ("ESP_TEST_NEGATIVE_NUMBER", Some("-1")), ("ESP_TEST_NON_NEGATIVE_NUMBER", Some("0")), + ("ESP_TEST_RANGE", Some("9")), ], || { generate_config( @@ -542,6 +576,12 @@ mod test { Value::Integer(-1), Some(Validator::NonNegativeInteger), ), + ( + "range", + "NA", + Value::Integer(0), + Some(Validator::IntegerInRange(5..10)), + ), ], false, ) From 94f0812f625478c244e1cea871ad9f9811000024 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Tue, 5 Nov 2024 13:18:12 +0100 Subject: [PATCH 5/5] Fix the thing --- esp-config/src/generate.rs | 2 +- esp-wifi/build.rs | 34 +++++++++++++++++----------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/esp-config/src/generate.rs b/esp-config/src/generate.rs index 1868cde1814..ede216f5d27 100644 --- a/esp-config/src/generate.rs +++ b/esp-config/src/generate.rs @@ -388,7 +388,7 @@ fn emit_configuration( selected_config: &mut String, ) { for (name, value) in configs.iter() { - let cfg_name = snake_case(name.trim_start_matches(prefix)); + let cfg_name = snake_case(name.trim_start_matches(&format!("{prefix}_"))); println!("cargo:rustc-check-cfg=cfg({cfg_name})"); if let Value::Bool(true) = value { diff --git a/esp-wifi/build.rs b/esp-wifi/build.rs index 1eae798b634..9b56ce6ff42 100644 --- a/esp-wifi/build.rs +++ b/esp-wifi/build.rs @@ -1,7 +1,7 @@ use std::{error::Error, str::FromStr}; use esp_build::assert_unique_used_features; -use esp_config::{define, generate_config, Value}; +use esp_config::{generate_config, Value}; use esp_metadata::{Chip, Config}; fn main() -> Result<(), Box> { @@ -99,30 +99,30 @@ fn main() -> Result<(), Box> { generate_config( "esp_wifi", &[ - ("rx_queue_size", "Size of the RX queue in frames", Value::UnsignedInteger(5), None), - ("tx_queue_size", "Size of the TX queue in frames", Value::UnsignedInteger(3), None), + ("rx_queue_size", "Size of the RX queue in frames", Value::Integer(5), None), + ("tx_queue_size", "Size of the TX queue in frames", Value::Integer(3), None), ( "static_rx_buf_num", "WiFi static RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", - Value::UnsignedInteger(10), + Value::Integer(10), None ), ( "dynamic_rx_buf_num", "WiFi dynamic RX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", - Value::UnsignedInteger(32), + Value::Integer(32), None ), ( "static_tx_buf_num", "WiFi static TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", - Value::UnsignedInteger(0), + Value::Integer(0), None ), ( "dynamic_tx_buf_num", "WiFi dynamic TX buffer number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", - Value::UnsignedInteger(32), + Value::Integer(32), None ), ( @@ -146,13 +146,13 @@ fn main() -> Result<(), Box> { ( "rx_ba_win", "WiFi Block Ack RX window size. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv418wifi_init_config_t)", - Value::UnsignedInteger(6), + Value::Integer(6), None ), ( "max_burst_size", "See [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_burst_size)", - Value::UnsignedInteger(1), + Value::Integer(1), None ), ( @@ -164,49 +164,49 @@ fn main() -> Result<(), Box> { ( "country_code_operating_class", "If not 0: Operating Class table number. See [ESP-IDF Programming Guide](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/wifi.html#wi-fi-country-code)", - Value::UnsignedInteger(0), + Value::Integer(0), None ), ( "mtu", "MTU, see [smoltcp's documentation](https://docs.rs/smoltcp/0.10.0/smoltcp/phy/struct.DeviceCapabilities.html#structfield.max_transmission_unit)", - Value::UnsignedInteger(1492), + Value::Integer(1492), None ), ( "tick_rate_hz", "Tick rate of the internal task scheduler in hertz", - Value::UnsignedInteger(100), + Value::Integer(100), None ), ( "listen_interval", "Interval for station to listen to beacon from AP. The unit of listen interval is one beacon interval. For example, if beacon interval is 100 ms and listen interval is 3, the interval for station to listen to beacon is 300 ms", - Value::UnsignedInteger(3), + Value::Integer(3), None ), ( "beacon_timeout", "For Station, If the station does not receive a beacon frame from the connected SoftAP during the inactive time, disconnect from SoftAP. Default 6s. Range 6-30", - Value::UnsignedInteger(6), + Value::Integer(6), None ), ( "ap_beacon_timeout", "For SoftAP, If the SoftAP doesn't receive any data from the connected STA during inactive time, the SoftAP will force deauth the STA. Default is 300s", - Value::UnsignedInteger(300), + Value::Integer(300), None ), ( "failure_retry_cnt", "Number of connection retries station will do before moving to next AP. scan_method should be set as WIFI_ALL_CHANNEL_SCAN to use this config. Note: Enabling this may cause connection time to increase incase best AP doesn't behave properly. Defaults to 1", - Value::UnsignedInteger(1), + Value::Integer(1), None ), ( "scan_method", "0 = WIFI_FAST_SCAN, 1 = WIFI_ALL_CHANNEL_SCAN, defaults to 0", - Value::UnsignedInteger(0), + Value::Integer(0), None ), ],