Remove flip-link feature, replace by flip-link option (#3001)

* Fix renamed config options

* Make `flip-link` feature into a config-option

* Remove `psram-quad`/`psram-octal` - replaced by `psram`

* Changelogs

* Fix

* Add `esp_config::Validator::Enumeration`

* Fix HIL-tests

* HIL: use octal psram for the S3

* Fix xtask

* xtask

* examples

* Make `spi-address-workaround` only available on ESP32

* Always allow `cfg(spi_address_workaround)`

* Make config-options valid for all targets

Author: bjoernQ

.github/workflows/hil.yml

@@ -118,7 +118,13 @@ jobs:
           ldproxy: false
           version: 1.84.0.0
 
+      # remove once we have a proper solution for target specific configs
+      - name: Build tests ESP32S3
+        if: ${{ matrix.target.soc == 'esp32s3'}}
+        run: ESP_HAL_CONFIG_PSRAM_MODE="octal" cargo xtask build-tests ${{ matrix.target.soc }}
+
       - name: Build tests
+        if: ${{ matrix.target.soc != 'esp32s3'}}
         run: cargo xtask build-tests ${{ matrix.target.soc }}
 
       - uses: actions/upload-artifact@v4

esp-config/src/generate.rs

@@ -3,13 +3,14 @@ use std::{
     env,
     fmt::{self, Write as _},
     fs,
+    io::Write,
     ops::Range,
     path::PathBuf,
 };
 
 const DOC_TABLE_HEADER: &str = r#"
-| Name | Description | Default value |
-|------|-------------|---------------|
+| Name | Description | Default value | Allowed value |
+|------|-------------|---------------|---------------|
 "#;
 
 const SELECTED_TABLE_HEADER: &str = r#"
@@ -155,6 +156,8 @@ pub enum Validator {
     PositiveInteger,
     /// Ensure that an integer value falls within the specified range.
     IntegerInRange(Range<i128>),
+    /// String-Enumeration. Only allows one of the given Strings.
+    Enumeration(Vec<String>),
     /// A custom validation function to run against any supported value type.
     Custom(Box<dyn Fn(&Value) -> Result<(), Error>>),
 }
@@ -166,11 +169,59 @@ impl Validator {
             Validator::NonNegativeInteger => non_negative_integer(value)?,
             Validator::PositiveInteger => positive_integer(value)?,
             Validator::IntegerInRange(range) => integer_in_range(range, value)?,
+            Validator::Enumeration(values) => enumeration(values, value)?,
             Validator::Custom(validator_fn) => validator_fn(value)?,
         }
 
         Ok(())
     }
+
+    fn description(&self) -> Option<String> {
+        match self {
+            Validator::NegativeInteger => Some(String::from("Negative integer")),
+            Validator::NonNegativeInteger => Some(String::from("Positive integer or 0")),
+            Validator::PositiveInteger => Some(String::from("Positive integer")),
+            Validator::IntegerInRange(range) => {
+                Some(format!("Integer in range {}..{}", range.start, range.end))
+            }
+            Validator::Enumeration(values) => Some(format!("Any of {:?}", values)),
+            Validator::Custom(_) => None,
+        }
+    }
+
+    fn emit_cargo_extras<W: Write>(&self, stdout: &mut W, config_key: &str) {
+        match self {
+            Validator::Enumeration(values) => {
+                let config_key = snake_case(config_key);
+                for value in values {
+                    writeln!(
+                        stdout,
+                        "cargo:rustc-check-cfg=cfg({config_key}_{})",
+                        snake_case(value)
+                    )
+                    .ok();
+                }
+            }
+            _ => (),
+        }
+    }
+}
+
+fn enumeration(values: &Vec<String>, value: &Value) -> Result<(), Error> {
+    if let Value::String(value) = value {
+        if !values.contains(value) {
+            return Err(Error::validation(format!(
+                "Expected one of {:?}, found '{}'",
+                values, value
+            )));
+        }
+
+        Ok(())
+    } else {
+        return Err(Error::parse(
+            "Validator::Enumeration can only be used with string values",
+        ));
+    }
 }
 
 fn negative_integer(value: &Value) -> Result<(), Error> {
@@ -252,13 +303,22 @@ pub fn generate_config(
     crate_name: &str,
     config: &[(&str, &str, Value, Option<Validator>)],
     emit_md_tables: bool,
+) -> HashMap<String, Value> {
+    generate_config_internal(&mut std::io::stdout(), crate_name, config, emit_md_tables)
+}
+
+pub fn generate_config_internal<W: Write>(
+    stdout: &mut W,
+    crate_name: &str,
+    config: &[(&str, &str, Value, Option<Validator>)],
+    emit_md_tables: bool,
 ) -> HashMap<String, Value> {
     // Only rebuild if `build.rs` changed. Otherwise, Cargo will rebuild if any
     // other file changed.
-    println!("cargo:rerun-if-changed=build.rs");
+    writeln!(stdout, "cargo:rerun-if-changed=build.rs").ok();
 
     #[cfg(not(test))]
-    env_change_work_around();
+    env_change_work_around(stdout);
 
     let mut doc_table = String::from(DOC_TABLE_HEADER);
     let mut selected_config = String::from(SELECTED_TABLE_HEADER);
@@ -281,7 +341,7 @@ pub fn generate_config(
         })
         .collect::<HashMap<_, _>>();
 
-    let mut configs = create_config(&prefix, config, &mut doc_table);
+    let mut configs = create_config(stdout, &prefix, config, &mut doc_table);
     capture_from_env(&prefix, &mut configs);
 
     for (name, value) in configs.iter() {
@@ -290,7 +350,18 @@ pub fn generate_config(
         }
     }
 
-    emit_configuration(&prefix, &configs, &mut selected_config);
+    let validators: HashMap<String, &Validator> = config
+        .iter()
+        .filter_map(|(name, _, _, validator)| {
+            if validator.is_some() {
+                Some((snake_case(name), validator.as_ref().unwrap()))
+            } else {
+                None
+            }
+        })
+        .collect();
+
+    emit_configuration(stdout, &prefix, &configs, &validators, &mut selected_config);
 
     if emit_md_tables {
         let file_name = snake_case(crate_name);
@@ -304,7 +375,7 @@ pub fn generate_config(
 // This can be removed when https://github.com/rust-lang/cargo/pull/14058 is merged.
 // Unlikely to work on projects in workspaces
 #[cfg(not(test))]
-fn env_change_work_around() {
+fn env_change_work_around<W: Write>(stdout: &mut W) {
     let mut out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
 
     // We clean out_dir by removing all trailing directories, until it ends with
@@ -319,37 +390,53 @@ fn env_change_work_around() {
     let dotcargo = out_dir.join(".cargo/");
     if dotcargo.exists() {
         if dotcargo.join("config.toml").exists() {
-            println!(
+            writeln!(
+                stdout,
                 "cargo:rerun-if-changed={}",
                 dotcargo.join("config.toml").display()
-            );
+            )
+            .ok();
         }
         if dotcargo.join("config").exists() {
-            println!(
+            writeln!(
+                stdout,
                 "cargo:rerun-if-changed={}",
                 dotcargo.join("config").display()
-            );
+            )
+            .ok();
         }
     }
 }
 
-fn create_config(
+fn create_config<W: Write>(
+    stdout: &mut W,
     prefix: &str,
     config: &[(&str, &str, Value, Option<Validator>)],
     doc_table: &mut String,
 ) -> HashMap<String, Value> {
     let mut configs = HashMap::new();
 
-    for (name, description, default, _validator) in config {
+    for (name, description, default, validator) in config {
         let name = format!("{prefix}{}", screaming_snake_case(name));
+        let allowed_values = if let Some(validator) = validator {
+            validator.description()
+        } else {
+            None
+        }
+        .unwrap_or(String::from("-"));
+
         configs.insert(name.clone(), default.clone());
 
         // Write documentation table line:
         let default = default.to_string();
-        writeln!(doc_table, "|**{name}**|{description}|{default}|").unwrap();
+        writeln!(
+            doc_table,
+            "|**{name}**|{description}|{default}|{allowed_values}"
+        )
+        .unwrap();
 
         // Rebuild if config environment variable changed:
-        println!("cargo:rerun-if-env-changed={name}");
+        writeln!(stdout, "cargo:rerun-if-env-changed={name}").ok();
     }
 
     configs
@@ -382,25 +469,38 @@ fn capture_from_env(prefix: &str, configs: &mut HashMap<String, Value>) {
     }
 }
 
-fn emit_configuration(
+fn emit_configuration<W: Write>(
+    stdout: &mut W,
     prefix: &str,
     configs: &HashMap<String, Value>,
+    validators: &HashMap<String, &Validator>,
     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})");
+        writeln!(stdout, "cargo:rustc-check-cfg=cfg({cfg_name})").ok();
 
         if let Value::Bool(true) = value {
-            println!("cargo:rustc-cfg={cfg_name}");
+            writeln!(stdout, "cargo:rustc-cfg={cfg_name}").ok();
+        }
+
+        if let Value::String(value) = value {
+            if let Some(Validator::Enumeration(_)) = validators.get(&cfg_name) {
+                let value = format!("{}_{}", cfg_name, snake_case(value));
+                writeln!(stdout, "cargo:rustc-cfg={value}").ok();
+            }
         }
 
         let value = value.to_string();
 
         // Values that haven't been seen will be output here with the default value:
-        println!("cargo:rustc-env={}={}", name, value);
+        writeln!(stdout, "cargo:rustc-env={}={}", name, value).ok();
         writeln!(selected_config, "|**{name}**|{value}|").unwrap();
     }
+
+    for (name, validator) in validators {
+        validator.emit_cargo_extras(stdout, &name);
+    }
 }
 
 fn write_config_tables(prefix: &str, doc_table: String, selected_config: String) {
@@ -696,4 +796,33 @@ mod test {
             },
         );
     }
+
+    #[test]
+    fn enumeration_validator() {
+        let mut stdout = Vec::new();
+        temp_env::with_vars([("ESP_TEST_CONFIG_SOME_KEY", Some("variant-0"))], || {
+            generate_config_internal(
+                &mut stdout,
+                "esp-test",
+                &[(
+                    "some-key",
+                    "NA",
+                    Value::String("variant-0".to_string()),
+                    Some(Validator::Enumeration(vec![
+                        "variant-0".to_string(),
+                        "variant-1".to_string(),
+                    ])),
+                )],
+                false,
+            );
+        });
+
+        let cargo_lines: Vec<&str> = std::str::from_utf8(&stdout).unwrap().lines().collect();
+        println!("{:#?}", cargo_lines);
+        assert!(cargo_lines.contains(&"cargo:rustc-check-cfg=cfg(some_key)"));
+        assert!(cargo_lines.contains(&"cargo:rustc-env=ESP_TEST_CONFIG_SOME_KEY=variant-0"));
+        assert!(cargo_lines.contains(&"cargo:rustc-check-cfg=cfg(some_key_variant_0)"));
+        assert!(cargo_lines.contains(&"cargo:rustc-check-cfg=cfg(some_key_variant_1)"));
+        assert!(cargo_lines.contains(&"cargo:rustc-cfg=some_key_variant_0"));
+    }
 }

esp-hal/CHANGELOG.md

@@ -28,6 +28,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Async` drivers are no longer `Send` (#2980)
 - GPIO drivers now take configuration structs, and their constructors are fallible (#2990)
+- `flip-link` feature is now a config option
+- `flip-link` feature is now a config option (`ESP_HAL_CONFIG_FLIP_LINK`)
+
+- `flip-link` feature is now a config option (`ESP_HAL_CONFIG_FLIP_LINK`) (#3001)
+
+- Removed features `psram-quad` and `psram-octal` - replaced by `psram` and the `ESP_HAL_CONFIG_PSRAM_MODE` (`quad`/`octal`) (#3001)
 
 ### Fixed
 

esp-hal/Cargo.toml

@@ -120,11 +120,6 @@ esp32s2 = ["dep:esp32s2", "portable-atomic/critical-section", "procmacros/has-ul
 # Target the ESP32-S3.
 esp32s3 = ["dep:esp32s3", "procmacros/has-ulp-core", "procmacros/rtc-slow", "usb-otg", "xtensa-lx-rt/esp32s3"]
 
-#! ### RISC-V Exclusive Feature Flags
-## Move the stack to start of RAM to get zero-cost stack overflow protection
-## (ESP32-C6 and ESPS32-H2 only!).
-flip-link = ["esp-riscv-rt/fix-sp"]
-
 #! ### Trait Implementation Feature Flags
 ## Implement `defmt::Format` on certain types.
 defmt = [
@@ -145,11 +140,8 @@ defmt = [
 ]
 
 #! ### PSRAM Feature Flags
-## Use externally connected Quad PSRAM
-quad-psram = []
-
-## Use externally connected Octal RAM
-octal-psram = []
+## Use externally connected PSRAM (`quad` by default, can be configured to `octal` via ESP_HAL_CONFIG_PSRAM_MODE)
+psram = []
 
 # This feature is intended for testing; you probably don't want to enable it:
 ci = ["defmt", "bluetooth"]

esp-hal/MIGRATING-0.23.md

@@ -128,6 +128,41 @@ Use `DataMode::SingleTwoDataLines` to get the previous behavior.
 
 `Spi` now offers both, `with_mosi` and `with_sio0`. Consider using `with_sio` for half-duplex SPI except for [DataMode::SingleTwoDataLines] or for a mixed-bus.
 
+## Removed `flip-link` Feature
+
+The `flip-link` feature is removed and replaced by the `ESP_HAL_CONFIG_FLIP_LINK` option.
+
+Cargo.toml
+```diff
+- esp-hal = { version = "0.23.0", features = ["flip-link"]}
++ esp-hal = "0.23.0"
+```
+
+config/config.toml
+```diff
+[env]
++ ESP_HAL_CONFIG_FLIP_LINK = "true"
+```
+
+## Removed `psram-quad`/`prsram-octal` Feature
+
+The features `psram-quad`/`prsram-octal` are replaced by a single `psram` feature and an additional config option (`ESP_HAL_CONFIG_PSRAM_MODE`).
+
+`ESP_HAL_CONFIG_PSRAM_MODE` defaults to `quad` and (for ESP32-S3) also allows `octal`.
+
+Cargo.toml
+```diff
+- esp-hal = { version = "0.23.0", features = ["psram-octal"]}
++ esp-hal = { version = "0.23.0", features = ["psram"]}
+```
+
+config/config.toml
+```diff
+[env]
++ ESP_HAL_CONFIG_PSRAM_MODE = "octal"
+```
+
+
 ## UART halves have their configuration split too
 
 `Uart::Config` structure now contains separate `RxConfig` and `TxConfig`: