Clean up binary_format help text (probe-rs#3766)

Author: bugadani

probe-rs-tools/src/bin/probe-rs/cmd/cargo_embed/mod.rs

@@ -271,7 +271,7 @@ async fn main_try(args: Vec<OsString>, config: Config, offset: UtcOffset) -> Res
     };
 
     let format_options = FormatOptions::default();
-    let format = FormatKind::from(format_options.to_format_kind(session.target()));
+    let format = format_options.to_format_kind(session.target());
     let elf = if matches!(format, FormatKind::Elf | FormatKind::Idf) {
         Some(fs::read(&path)?)
     } else {

probe-rs-tools/src/bin/probe-rs/cmd/dap_server/server/session_data.rs

@@ -3,7 +3,6 @@ use super::{
     core_data::{CoreData, CoreHandle},
 };
 use crate::{
-    FormatKind,
     cmd::{
         dap_server::{
             DebuggerError,
@@ -25,6 +24,7 @@ use anyhow::{Result, anyhow};
 use probe_rs::{
     BreakpointCause, CoreStatus, HaltReason, Session, VectorCatchCondition,
     config::{Registry, TargetSelector},
+    flashing::FormatKind,
     probe::list::Lister,
     rtt::ScanRegion,
 };

probe-rs-tools/src/bin/probe-rs/main.rs

@@ -7,7 +7,6 @@ use std::cmp::Reverse;
 use std::collections::HashMap;
 use std::fs;
 use std::path::Path;
-use std::str::FromStr;
 use std::{ffi::OsString, path::PathBuf};
 
 use anyhow::{Context, Result};
@@ -358,16 +357,15 @@ pub struct ElfCliOptions {
 #[derive(clap::Parser, Clone, Serialize, Deserialize, Debug, Default, Schema)]
 #[serde(default)]
 pub struct FormatOptions {
-    /// If a format is provided, use it.
-    /// If a target has a preferred format, we use that.
-    /// Finally, if neither of the above cases are true, we default to ELF.
+    /// The format of the firmware image.
     #[clap(
         value_enum,
         ignore_case = true,
+        default_value_t = FormatKind::Target,
         long,
         help_heading = "DOWNLOAD CONFIGURATION"
     )]
-    binary_format: Option<FormatKind>,
+    binary_format: FormatKind,
 
     #[clap(flatten)]
     bin_options: BinaryCliOptions,
@@ -380,54 +378,42 @@ pub struct FormatOptions {
 }
 
 /// A finite list of all the available binary formats probe-rs understands.
-#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, Schema)]
+#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, ValueEnum, Schema)]
 pub enum FormatKind {
-    /// Marks a file in binary format. This means that the file contains the contents of the flash 1:1.
-    /// [BinOptions] can be used to define the location in flash where the file contents should be put at.
-    /// Additionally using the same config struct, you can skip the first N bytes of the binary file to have them not put into the flash.
+    /// The image format is determined by the target chip's preference, which is usually ELF.
+    #[default]
+    Target,
+
+    /// The image is in binary format. This means that the file contains the contents of the flash 1:1.
+    #[value(alias("binary"))]
     Bin,
-    /// Marks a file in [Intel HEX](https://en.wikipedia.org/wiki/Intel_HEX) format.
+
+    /// The image is in Intel HEX format. For more information, see https://en.wikipedia.org/wiki/Intel_HEX
+    #[value(aliases(["ihex", "intelhex"]))]
     Hex,
-    /// Marks a file in the [ELF](https://en.wikipedia.org/wiki/Executable_and_Linkable_Format) format.
-    #[default]
+
+    /// The image is in the Executable and Linkable Format (ELF). For more information, see https://en.wikipedia.org/wiki/Executable_and_Linkable_Format
     Elf,
-    /// Marks a file in the [ESP-IDF bootloader](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/app_image_format.html#app-image-structures) format.
-    /// Use [IdfOptions] to configure flashing.
+
+    /// The image is an ELF file containing an ESP-IDF bootloader compatible application. For more information, see https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/app_image_format.html#app-image-structures
+    #[value(aliases(["esp-idf", "espidf"]))]
     Idf,
-    /// Marks a file in the [UF2](https://github.com/microsoft/uf2) format.
+
+    /// The image is in the Universal Flash Storage (UF2) format. For more information, see https://github.com/microsoft/uf2
     Uf2,
 }
 
 impl FormatKind {
-    /// Creates a new Format from an optional string.
-    ///
-    /// If the string is `None`, the default format is returned.
-    pub fn from_optional(s: Option<&str>) -> Result<Self, String> {
-        match s {
-            Some(format) => Self::from_str(format),
-            None => Ok(Self::default()),
-        }
-    }
-}
-
-impl FromStr for FormatKind {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match &s.to_lowercase()[..] {
-            "bin" | "binary" => Ok(Self::Bin),
-            "hex" | "ihex" | "intelhex" => Ok(Self::Hex),
-            "elf" => Ok(Self::Elf),
-            "uf2" => Ok(Self::Uf2),
-            "idf" | "esp-idf" | "espidf" => Ok(Self::Idf),
-            _ => Err(format!("Format '{s}' is unknown.")),
-        }
-    }
-}
+    fn to_probe_rs(self, target: &Target) -> probe_rs::flashing::FormatKind {
+        let this = if self == FormatKind::Target {
+            FormatKind::from_optional(target.default_format.as_deref())
+                .expect("Failed to parse a default binary format. This shouldn't happen.")
+        } else {
+            self
+        };
 
-impl From<FormatKind> for probe_rs::flashing::FormatKind {
-    fn from(kind: FormatKind) -> Self {
-        match kind {
+        match this {
+            FormatKind::Target => unreachable!(),
             FormatKind::Bin => probe_rs::flashing::FormatKind::Bin,
             FormatKind::Hex => probe_rs::flashing::FormatKind::Hex,
             FormatKind::Elf => probe_rs::flashing::FormatKind::Elf,
@@ -437,15 +423,24 @@ impl From<FormatKind> for probe_rs::flashing::FormatKind {
     }
 }
 
+impl FormatKind {
+    /// Creates a new Format from an optional string.
+    ///
+    /// If the string is `None`, the default format is returned.
+    pub fn from_optional(s: Option<&str>) -> Result<Self, String> {
+        match s {
+            Some(format) => Self::from_str(format, true),
+            None => Ok(Self::Elf),
+        }
+    }
+}
+
 impl FormatOptions {
     /// If a format is provided, use it.
     /// If a target has a preferred format, we use that.
     /// Finally, if neither of the above cases are true, we default to [`Format::default()`].
-    pub fn to_format_kind(&self, target: &Target) -> FormatKind {
-        self.binary_format.unwrap_or_else(|| {
-            FormatKind::from_optional(target.default_format.as_deref())
-                .expect("Failed to parse a default binary format. This shouldn't happen.")
-        })
+    pub fn to_format_kind(&self, target: &Target) -> probe_rs::flashing::FormatKind {
+        self.binary_format.to_probe_rs(target)
     }
 }
 

probe-rs-tools/src/bin/probe-rs/util/flash.rs

@@ -1,5 +1,5 @@
+use crate::FormatOptions;
 use crate::rpc::functions::flash::{FlashLayout, Operation, ProgressEvent};
-use crate::{FormatKind, FormatOptions};
 
 use super::common_options::{BinaryDownloadOptions, LoadedProbeOptions, OperationError};
 use super::logging;
@@ -13,7 +13,9 @@ use colored::Colorize;
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
 use parking_lot::Mutex;
 use probe_rs::InstructionSet;
-use probe_rs::flashing::{BinOptions, ElfOptions, FlashError, FlashProgress, Format, IdfOptions};
+use probe_rs::flashing::{
+    BinOptions, ElfOptions, FlashError, FlashProgress, Format, FormatKind, IdfOptions,
+};
 use probe_rs::{
     Session,
     flashing::{DownloadOptions, FileDownloadError, FlashLoader},