Skip to content

Migrate old config #883

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Migrate old config #883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b65cab1
feat: Migrate old config
SergioGasquez Jun 13, 2025
df2e742
docs: Udpate changelog
SergioGasquez Jun 13, 2025
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Run some arguments checks for monitoring flags. (#842)
- Add support for the ESP32-C5 (#863)
- `--after` options now work with `espflash board-info`, `espflash read-flash` and `espflash checksum-md5` (#867)
- Add support for serial port configuration files. (#777)
- Add support for serial port configuration files. (#777, #883)
- Add a `check-app-descriptor` bool option to `ImageArgs` and add the flag to `flash` commad(#872)

### Changed
Expand All @@ -49,6 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated bootloaders with `release/v5.4` ones from IDF (#857)
- Refactor image formatting to allow supporting more image formats in a backward compatible way (#877)
- Avoid having ESP-IDF format assumptions in the codebase (#877)
- Automatically migrate `espflash@3` configuration files to the new format (#883)

### Fixed

Expand Down
20 changes: 20 additions & 0 deletions espflash/src/cli/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,26 @@ impl Config {

let mut port_config = if let Ok(data) = read_to_string(&port_config_file) {
toml::from_str(&data).into_diagnostic()?
} else if let Ok(data) = read_to_string(&project_config_file) {
if data.contains("[connection]") || data.contains("[[usb_device]]") {
Copy link
Member

Choose a reason for hiding this comment

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

I think this as an idea is good, but I think we should at least parse the toml because they might have these commented out or something like that.

We can do this in two ways, we "recreate" the old config struct to try and parse it, or we use https://docs.rs/toml/0.8.23/toml/#parsing-toml to just parse the file as a whole and we can just inspect the keys before making a decision.

Copy link
Member Author

Choose a reason for hiding this comment

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

because they might have these commented out or something like that.

If they are commented out, then let port_config = toml::from_str(&data).into_diagnostic()?; would not read it. But maybe its better your approach

Copy link
Member

Choose a reason for hiding this comment

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

Once we decide that we need to do the migration, we also need to remove the keys from the project config, once the ports file has been succesfully saved. I'm pretty sure if we just call Config::save_with all this will be resolved right? It will write over the existing config file with the new format, plus write out the port info into the new file without any of this extra code?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm Config::save_with doesn't save the project config... but maybe it should?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm Config::save_with doesn't save the project config... but maybe it should?

Not sure if it should, the only scenario when we write config is to store port config when the users selects a port and wants to save it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay, if it doesn't make sense to save the project config we can keep the special case save here then.

debug!(
"espflash@3 configuration detected. Migrating port config to port_config_file: {:#?}",
&port_config_file
);
let port_config = toml::from_str(&data).into_diagnostic()?;
let serialized = toml::to_string(&port_config)
.into_diagnostic()
.wrap_err("Failed to serialize config")?;
create_dir_all(port_config_file.parent().unwrap())
.into_diagnostic()
.wrap_err("Failed to create config directory")?;
write(&port_config_file, serialized)
.into_diagnostic()
.wrap_err("Failed to write config")?;
port_config
} else {
PortConfig::default()
}
} else {
PortConfig::default()
};
Expand Down
Loading