Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ jobs:
target: riscv32imc-unknown-none-elf,riscv32imac-unknown-none-elf
components: clippy,rustfmt,rust-src

# Rust toolchain for RISC-V:
- if: ${{ !contains(fromJson('["esp32", "esp32s2", "esp32s3"]'), matrix.chip) }}
uses: dtolnay/rust-toolchain@nightly
with:
target: riscv32imc-unknown-none-elf,riscv32imac-unknown-none-elf
components: clippy,rustfmt,rust-src

# //Define a new environment variable called toolchain
- if: ${{ contains(fromJson('["esp32", "esp32s2", "esp32s3"]'), matrix.chip) }}
run: echo "TOOLCHAIN=+esp" >> $GITHUB_ENV
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- The visual style in certain terminals no longer uses emojis (#173)
- Add a description to the version check output (#178)

### Fixed

Expand Down
73 changes: 65 additions & 8 deletions src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ enum CheckResult {
NotFound,
}

pub fn check(chip: Chip) {
pub fn check(chip: Chip, probe_rs_required: bool) {
// TODO: check +nightly if needed
let rust_version = get_version(
"rustc",
Expand All @@ -27,20 +27,77 @@ pub fn check(chip: Chip) {
},
);

let rust_toolchain = if chip.is_xtensa() { "esp" } else { "stable" };

let espflash_version = get_version("espflash", &[]);
let probers_version = get_version("probe-rs", &[]);

println!("\nChecking installed versions");
print_result("Rust", check_version(rust_version, 1, 84, 0));
print_result("espflash", check_version(espflash_version, 3, 3, 0));
print_result("probe-rs", check_version(probers_version, 0, 25, 0));

let mut requirements_unsatisfied = false;
requirements_unsatisfied |= print_result(
&format!("Rust ({rust_toolchain})"),
check_version(rust_version, 1, 84, 0),
if chip.is_xtensa() {
"minimum required version is 1.84 - use `espup` to upgrade"
} else {
"minimum required version is 1.84 - use `rustup` to upgrade"
},
if chip.is_xtensa() {
"not found - use `espup` to install"
} else {
"not found - use `rustup` to install"
},
true,
);
requirements_unsatisfied |= print_result(
"espflash",
check_version(espflash_version, 3, 3, 0),
"minimum required version is 3.3.0 - see https://crates.io/crates/espflash",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"minimum required version is 3.3.0 - see https://crates.io/crates/espflash",
"minimum required version is 3.3.0 - see https://crates.io/crates/espflash",

Not sure on the wording here, older versions of espflash might also work but may have some bugs or missing features.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True - but it's probably hard to explain these details here - we might (or might not) want to explain those things in the book

"not found - see https://crates.io/crates/espflash for installation instructions",
true,
);
requirements_unsatisfied |= print_result(
"probe-rs",
check_version(probers_version, 0, 25, 0),
if probe_rs_required {
"minimum version required is 0.25.0 - see https://probe.rs/docs/getting-started/installation/ for how to upgrade"
} else {
"minimum suggested version is 0.25.0 - see https://probe.rs/docs/getting-started/installation/ for how to upgrade"
},
if probe_rs_required {
"not found - see https://probe.rs/docs/getting-started/installation/ for how to install"
} else {
"not found - while not required see https://probe.rs/docs/getting-started/installation/ for how to install"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

We can do a lot better than this mess of duplicated strings. Make print_result take impl AsRef<str> and the you can use format! to collapse these.

The "while not required" wording is a bit unfortunate, as a user I would question whether this is an ad or something, and why would I install something that I don't need?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can do a lot better than this mess of duplicated strings. Make print_result take impl AsRef and the you can use format! to collapse these.

mhhh - not too sure format! will make it much nicer and not just harder to read 🤔

Yeah - I somewhat struggled with the "not really required but maybe you want it" wording - not happy about it but ... Any suggestion? - we could certainly explain a lot of things here but probably we should avoid a wall-of-text - ideally, we will explain why, when and how to use it in the book (but we are not there yet)

Thinking about it I'm not sure we should have the links etc. at all here - just link to the book if any of the required / recommended checks fail

Copy link
Contributor

@bugadani bugadani May 26, 2025

Choose a reason for hiding this comment

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

I personally find this preferable to two separate strings, especially that we have at least 3 such decision in here, and you also use this pattern to print which toolchain is in question ("Rust ({rust_toolchain})").

let suggestion_kind = if probe_rs_required { "required" } else { "suggested" };
format!("minimum {suggestion_kind} version is 0.25.0 - see https://probe.rs/docs/getting-started/installation/ for how to upgrade");

probe_rs_required,
);

if requirements_unsatisfied {
println!("\nFor more details see https://docs.espressif.com/projects/rust/book/")
}
}

fn print_result(name: &str, check_result: CheckResult) {
fn print_result(
name: &str,
check_result: CheckResult,
wrong_version_help: &str,
not_found_help: &str,
required: bool,
) -> bool {
match check_result {
CheckResult::Ok => println!("🆗 {}", name),
CheckResult::WrongVersion => println!("🛑 {}", name),
CheckResult::NotFound => println!("❌ {}", name),
CheckResult::Ok => {
println!("🆗 {name}");
false
}
CheckResult::WrongVersion => {
println!("🛑 {name} ({wrong_version_help})");
required
}
CheckResult::NotFound => {
println!("❌ {name} ({not_found_help})");
required
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ fn main() -> Result<(), Box<dyn Error>> {
log::warn!("Current directory is already in a git repository, skipping git initialization");
}

check::check(args.chip);
check::check(args.chip, selected.contains(&"probe-rs".to_string()));

Ok(())
}
Expand Down