Skip to content

Commit

Permalink
Remove flip-link feature, replace by flip-link option (esp-rs#3001)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
bjoernQ authored Jan 23, 2025
1 parent e842ec4 commit f247b40
Show file tree
Hide file tree
Showing 30 changed files with 290 additions and 181 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/hil.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
167 changes: 148 additions & 19 deletions esp-config/src/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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#"
Expand Down Expand Up @@ -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>>),
}
Expand All @@ -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> {
Expand Down Expand Up @@ -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);
Expand All @@ -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() {
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"));
}
}
6 changes: 6 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 2 additions & 10 deletions esp-hal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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"]
Expand Down
35 changes: 35 additions & 0 deletions esp-hal/MIGRATING-0.23.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`:
Expand Down
Loading

0 comments on commit f247b40

Please sign in to comment.