Skip to content

esp-config tui #3442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ jobs:
- uses: Swatinem/rust-cache@v2

# Run tests in esp-config
- run: cd esp-config && cargo test --features build
- run: cd esp-config && cargo test --features build,tui

# Run tests in esp-bootloader-esp-idf
- run: cd esp-bootloader-esp-idf && cargo test --features=std
33 changes: 32 additions & 1 deletion esp-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,26 @@ license = "MIT OR Apache-2.0"
bench = false
test = true

[[bin]]
name = "esp-config"
required-features = ["tui"]

[dependencies]
document-features = "0.2.11"

# used by the `build` and `tui` feature
serde = { version = "1.0.197", features = ["derive"], optional = true }
serde_json = { version = "1.0.0", optional = true }
serde_json = { version = "1.0.0", features = ["arbitrary_precision"], optional = true }

# used by the `tui` feature
clap = { version = "4.5.32", features = ["derive"], optional = true }
crossterm = { version = "0.28.1", optional = true }
env_logger = { version = "0.11.7", optional = true }
log = { version = "0.4.26", optional = true }
ratatui = { version = "0.29.0", features = ["crossterm", "unstable"], optional = true }
toml_edit = { version = "0.22.26", optional = true }
tui-textarea = { version = "0.7.0", optional = true }
walkdir = { version = "2.5.0", optional = true }

[dev-dependencies]
temp-env = "0.3.6"
Expand All @@ -24,3 +40,18 @@ pretty_assertions = "1.4.1"
[features]
## Enable the generation and parsing of a config
build = ["dep:serde","dep:serde_json"]

## The TUI
tui = [
"dep:clap",
"dep:crossterm",
"dep:env_logger",
"dep:log",
"dep:ratatui",
"dep:toml_edit",
"dep:tui-textarea",
"dep:walkdir",
"dep:serde",
"dep:serde_json",
"build",
]
249 changes: 249 additions & 0 deletions esp-config/src/bin/esp-config/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
use std::{
collections::HashMap,
error::Error,
path::{Path, PathBuf},
};

use clap::Parser;
use env_logger::{Builder, Env};
use esp_config::{ConfigOption, Value};
use serde::Deserialize;
use toml_edit::{DocumentMut, Formatted, Item, Table};
use walkdir::WalkDir;

mod tui;

#[derive(Parser, Debug)]
#[command(author, version, about, long_about = None)]
struct Args {
/// Root of the project
#[arg(short = 'P', long)]
path: Option<PathBuf>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CrateConfig {
name: String,
options: Vec<ConfigItem>,
}

#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
struct ConfigItem {
option: ConfigOption,
actual_value: Value,
}

fn main() -> Result<(), Box<dyn Error>> {
Builder::from_env(Env::default().default_filter_or(log::LevelFilter::Info.as_str()))
.format_target(false)
.init();

let args = Args::parse();

let work_dir = args.path.clone().unwrap_or(".".into());

ensure_fresh_build(&work_dir)?;

let mut configs = parse_configs(&work_dir)?;
let initial_configs = configs.clone();
let mut previous_config = initial_configs.clone();

let mut errors_to_show = None;

loop {
let repository = tui::Repository::new(configs.clone());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this is in a loop

Copy link
Contributor Author

@bjoernQ bjoernQ May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is that a user could change something which is checked in the build.rs (e.g. esp-wifi does it) and the build check below will fail - in that case we show the TUI again with the error message from the build - the user can fix the problem or revert all changes in that case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if any such issue could occur, is an issue with the options and their validations.

The users' projects can fail to compile for a multitude of reasons, for example the build could require a feature flag to be set. The TUI will not know how to build the project properly, so a check like this can turn the tool completely unuseable.


// TUI stuff ahead
let terminal = tui::init_terminal()?;

// create app and run it
let updated_cfg = tui::App::new(errors_to_show, repository).run(terminal)?;

tui::restore_terminal()?;

// done with the TUI
if let Some(updated_cfg) = updated_cfg {
configs = updated_cfg.clone();
apply_config(&work_dir, updated_cfg.clone(), previous_config.clone())?;
previous_config = updated_cfg;
} else {
println!("Reverted configuration...");
apply_config(&work_dir, initial_configs, vec![])?;
break;
}

if let Some(errors) = check_build_after_changes(&work_dir) {
errors_to_show = Some(errors);
} else {
println!("Updated configuration...");
break;
}
}

Ok(())
}

fn apply_config(
path: &Path,
updated_cfg: Vec<CrateConfig>,
previous_cfg: Vec<CrateConfig>,
) -> Result<(), Box<dyn Error>> {
let config_toml = path.join(".cargo/config.toml");

let mut config = std::fs::read_to_string(&config_toml)?
.as_str()
.parse::<DocumentMut>()?;

if !config.contains_key("env") {
config.insert("env", Item::Table(Table::new()));
}

let envs = config.get_mut("env").unwrap().as_table_mut().unwrap();

for cfg in updated_cfg {
let prefix = cfg.name.to_ascii_uppercase().replace("-", "_");
let previous_crate_cfg = previous_cfg.iter().find(|c| c.name == cfg.name);

for option in cfg.options {
let previous_option = previous_crate_cfg.and_then(|c| {
c.options
.iter()
.find(|o| o.option.name == option.option.name)
});

let key = format!(
"{prefix}_CONFIG_{}",
option.option.name.to_ascii_uppercase().replace("-", "_")
);

// avoid updating unchanged options to keep the comments (if any)
if Some(&option.actual_value) != previous_option.map(|option| &option.actual_value) {
if option.actual_value != option.option.default_value {
let value = toml_edit::Value::String(Formatted::new(format!(
"{}",
option.actual_value
)));

envs.insert(&key, Item::Value(value));
} else {
envs.remove(&key);
}
}
}
}

std::fs::write(&config_toml, config.to_string().as_bytes())?;

Ok(())
}

fn parse_configs(path: &Path) -> Result<Vec<CrateConfig>, Box<dyn Error>> {
// we cheat by just trying to find the latest version of the config files
// this should be fine since we force a fresh build before
let mut candidates: Vec<_> = WalkDir::new(path.join("target"))
.into_iter()
.filter_entry(|entry| {
entry.file_type().is_dir() || {
if let Some(name) = entry.file_name().to_str() {
name.ends_with("_config_data.json")
} else {
false
}
}
})
.filter(|entry| !entry.as_ref().unwrap().file_type().is_dir())
.map(|entry| entry.unwrap())
.collect();
candidates.sort_by_key(|entry| entry.metadata().unwrap().modified().unwrap());

let mut crate_config_table_to_json: HashMap<String, PathBuf> = HashMap::new();

for e in candidates {
if e.file_name()
.to_str()
.unwrap()
.ends_with("_config_data.json")
{
let crate_name = e
.file_name()
.to_str()
.unwrap()
.replace("_config_data.json", "")
.replace("_", "-");
crate_config_table_to_json.insert(crate_name.clone(), e.path().to_path_buf());
}
}

let mut configs = Vec::new();

for (crate_name, path) in crate_config_table_to_json {
let options =
serde_json::from_str::<Vec<ConfigItem>>(std::fs::read_to_string(&path)?.as_str())
.map_err(|e| {
format!(
"Unable to read config file {:?} - try `cargo clean` first ({e:?})",
path
)
})?
.iter()
.filter(|option| option.option.active)
.cloned()
.collect();

configs.push(CrateConfig {
name: crate_name,
options,
});
}
configs.sort_by_key(|entry| entry.name.clone());

if configs.is_empty() {
return Err("No config files found.".into());
}

Ok(configs)
}

fn ensure_fresh_build(path: &PathBuf) -> Result<(), Box<dyn Error>> {
let status = std::process::Command::new("cargo")
.arg("build")
.current_dir(path)
.status()?;

if !status.success() {
return Err("Your project doesn't build. Fix the errors first.".into());
}

Ok(())
}

fn check_build_after_changes(path: &PathBuf) -> Option<String> {
println!("Check configuration...");

let status = std::process::Command::new("cargo")
Copy link
Contributor

@bugadani bugadani May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably shouldn't build the project, or we should make it optional. It can be very slow, or have special needs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making it optional sounds good - then we should also make the ensure_fresh_build optional - the reason we do it is to detect invalid settings which are only checked in build.rs (esp-config initially wasn't built with tooling in mind)

we could also allow the user to pass features needed for building (while a vanilla esp-generate generated project won't require that) - I guess with a lot of customizations sooner or later a user will get to the point that they need to manage the config manually - on the other hand I assume most users will be fine with what esp-generate can do for them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.b.h.I wouldn't be sad to remove the "build-my-project" steps from the tool :)

Currently it builds before launching the TUI since it's the easiest way to guarantee the config-json is up to date .... the original plan was to check the "freshness" of the config-json and bail if needed instead - I guess that would be fine, too

For the build after applying the changes (and the cursed loop): users can modify the config in a way that makes the project fail to build (e.g.

esp-hal/esp-wifi/src/lib.rs

Lines 236 to 249 in 29060ae

// Validate the configuration at compile time
#[allow(clippy::assertions_on_constants)]
const _: () = {
// We explicitely use `core` assert here because this evaluation happens at
// compile time and won't bloat the binary
core::assert!(
CONFIG.rx_ba_win < CONFIG.dynamic_rx_buf_num,
"WiFi configuration check: rx_ba_win should not be larger than dynamic_rx_buf_num!"
);
core::assert!(
CONFIG.rx_ba_win < (CONFIG.static_rx_buf_num * 2),
"WiFi configuration check: rx_ba_win should not be larger than double of the static_rx_buf_num!"
);
};
)

Without the check users will need to manually fix/revert the config change because the UI won't launch (because the config-json is outdated - I guess in the example above this isn't much of a problem since build.rs will write out the config-json before we fail the build in esp-wifi's lib.rs ???).

Probably that is

  • not too common
  • we can expect someone who is able to write Rust code to get the project back to a buildable state

So, this might be acceptable.

We could also go the extra mile and make the" build-my-project" steps opt-out - most users will benefit from the additional convenience while those who really need to can avoid it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this tool doesn't know how to build my project 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this tool doesn't know how to build my project

Not yours but the projects of most people just using a vanilla esp-generate generated one - so if you could pass --no-build (subject to bikeshed) you could use it?

.arg("build")
.current_dir(path)
.stdout(std::process::Stdio::inherit())
.output();

if let Ok(status) = &status {
if status.status.success() {
return None;
}
}

let mut errors = String::new();

for line in String::from_utf8(status.unwrap().stderr)
.unwrap_or_default()
.lines()
{
if line.contains("the evaluated program panicked at '") {
let error = line[line.find('\'').unwrap() + 1..].to_string();
let error = error[..error.find("',").unwrap_or(error.len())].to_string();
errors.push_str(&format!("{error}\n"));
}
}

Some(errors)
}
Loading
Loading