Split Run Cmd Options. Add help headings

probe-rs · MabezDev · Mar 20, 2024 · Mar 15, 2024 · Mar 15, 2024 · Mar 15, 2024
commit 80495fefc77a436f9c3b0b1150ba8e65cb23f338
diff --git a/probe-rs/src/bin/probe-rs/cmd/run/mod.rs b/probe-rs/src/bin/probe-rs/cmd/run/mod.rs
@@ -25,40 +25,57 @@ const RTT_RETRIES: usize = 10;
 
 #[derive(clap::Parser)]
 pub struct Cmd {
+    /// The path to the ELF file to flash and run
+    pub(crate) path: String,
+
+    /// Options only used when in normal run mode
     #[clap(flatten)]
-    pub(crate) probe_options: ProbeOptions,
+    pub(crate) run_options: NormalRunOptions,
+
+    // ---- General Options ahead ----
+    #[clap(flatten)]
+    pub(crate) common_options: CommonOptions,
 
     #[clap(flatten)]
     pub(crate) download_options: BinaryDownloadOptions,
 
-    /// The path to the ELF file to flash and run
-    pub(crate) path: String,
+    #[clap(flatten)]
+    pub(crate) format_options: FormatOptions,
 
+    /// Whether to erase the entire chip before downloading
+    #[clap(long, help_heading = "DOWNLOAD CONFIGURATION")]
+    pub(crate) chip_erase: bool,
+
+    #[clap(flatten)]
+    pub(crate) probe_options: ProbeOptions,
+}
+
+/// Options only used in normal run mode
+#[derive(Debug, clap::Parser, Clone)]
+pub struct NormalRunOptions {
+    /// Enable reset vector catch if its supported on the target.
+    #[clap(long, help_heading = "RUN OPTIONS")]
+    pub catch_reset: bool,
+    /// Enable hardfault vector catch if its supported on the target.
+    #[clap(long, help_heading = "RUN OPTIONS")]
+    pub catch_hardfault: bool,
+}
+
+// Options used for all run modes
+#[derive(Debug, clap::Parser)]
+pub struct CommonOptions {
     /// Always print the stacktrace on ctrl + c.
     #[clap(long)]
     pub(crate) always_print_stacktrace: bool,
 
-    /// Whether to erase the entire chip before downloading
-    #[clap(long)]
-    pub(crate) chip_erase: bool,
-
     /// Suppress filename and line number information from the rtt log
     #[clap(long)]
     pub(crate) no_location: bool,
 
-    #[clap(flatten)]
-    pub(crate) format_options: FormatOptions,
-
+    /// The default format string to use for decoding defmt logs.
     #[clap(long)]
     pub(crate) log_format: Option<String>,
 
-    /// Enable reset vector catch if its supported on the target.
-    #[arg(long)]
-    pub catch_reset: bool,
-    /// Enable hardfault vector catch if its supported on the target.
-    #[arg(long)]
-    pub catch_hardfault: bool,
-
     /// Scan the memory to find the RTT control block
     #[clap(long)]
     pub(crate) rtt_scan_memory: bool,
@@ -91,21 +108,21 @@ impl Cmd {
         }
 
         let memory_map = session.target().memory_map.clone();
-        let rtt_scan_regions = match self.rtt_scan_memory {
+        let rtt_scan_regions = match self.common_options.rtt_scan_memory {
             true => session.target().rtt_scan_regions.clone(),
             false => Vec::new(),
         };
         let mut core = session.core(0)?;
 
-        if self.catch_hardfault || self.catch_reset {
+        if self.run_options.catch_hardfault || self.run_options.catch_reset {
             core.halt(Duration::from_millis(100))?;
-            if self.catch_hardfault {
+            if self.run_options.catch_hardfault {
                 match core.enable_vector_catch(VectorCatchCondition::HardFault) {
                     Ok(_) | Err(Error::NotImplemented(_)) => {} // Don't output an error if vector_catch hasn't been implemented
                     Err(e) => tracing::error!("Failed to enable_vector_catch: {:?}", e),
                 }
             }
-            if self.catch_reset {
+            if self.run_options.catch_reset {
                 match core.enable_vector_catch(VectorCatchCondition::CoreReset) {
                     Ok(_) | Err(Error::NotImplemented(_)) => {} // Don't output an error if vector_catch hasn't been implemented
                     Err(e) => tracing::error!("Failed to enable_vector_catch: {:?}", e),
@@ -123,9 +140,9 @@ impl Cmd {
             &rtt_scan_regions,
             path,
             timestamp_offset,
-            self.always_print_stacktrace,
-            self.no_location,
-            self.log_format.as_deref(),
+            self.common_options.always_print_stacktrace,
+            self.common_options.no_location,
+            self.common_options.log_format.as_deref(),
         )?;
 
         Ok(())

diff --git a/probe-rs/src/bin/probe-rs/main.rs b/probe-rs/src/bin/probe-rs/main.rs
@@ -37,10 +37,10 @@ struct Cli {
     /// Location for log file
     ///
     /// If no location is specified, the behaviour depends on `--log-to-folder`.
-    #[clap(long, global = true)]
+    #[clap(long, global = true, help_heading = "LOG CONFIGURATION")]
     log_file: Option<PathBuf>,
     /// Enable logging to the default folder. This option is ignored if `--log-file` is specified.
-    #[clap(long, global = true)]
+    #[clap(long, global = true, help_heading = "LOG CONFIGURATION")]
     log_to_folder: bool,
     #[clap(subcommand)]
     subcommand: Subcommand,
@@ -109,21 +109,26 @@ 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.
-    #[clap(value_enum, ignore_case = true, long)]
+    #[clap(
+        value_enum,
+        ignore_case = true,
+        long,
+        help_heading = "DOWNLOAD CONFIGURATION"
+    )]
     // TODO: remove this alias in the next release after 0.24 and release of https://github.com/probe-rs/vscode/pull/86
     #[serde(deserialize_with = "format_from_str", alias = "format")]
     binary_format: Option<Format>,
     /// The address in memory where the binary will be put at. This is only considered when `bin` is selected as the format.
-    #[clap(long, value_parser = parse_u64)]
+    #[clap(long, value_parser = parse_u64, help_heading = "DOWNLOAD CONFIGURATION")]
     pub base_address: Option<u64>,
     /// The number of bytes to skip at the start of the binary file. This is only considered when `bin` is selected as the format.
-    #[clap(long, value_parser = parse_u32, default_value = "0")]
+    #[clap(long, value_parser = parse_u32, default_value = "0", help_heading = "DOWNLOAD CONFIGURATION")]
     pub skip: u32,
     /// The idf bootloader path
-    #[clap(long)]
+    #[clap(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub idf_bootloader: Option<PathBuf>,
     /// The idf partition table path
-    #[clap(long)]
+    #[clap(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub idf_partition_table: Option<PathBuf>,
 }
 

diff --git a/probe-rs/src/bin/probe-rs/util/common_options.rs b/probe-rs/src/bin/probe-rs/util/common_options.rs
@@ -83,20 +83,24 @@ pub struct FlashOptions {
 /// Common options when flashing a target device.
 #[derive(Debug, clap::Parser)]
 pub struct BinaryDownloadOptions {
-    #[arg(long)]
+    #[arg(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub disable_progressbars: bool,
     /// Use this flag to disable double-buffering when downloading flash data. If
     /// download fails during programming with timeout errors, try this option.
-    #[arg(long)]
+    #[arg(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub disable_double_buffering: bool,
     /// Enable this flag to restore all bytes erased in the sector erase but not overwritten by any page.
-    #[arg(long)]
+    #[arg(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub restore_unwritten: bool,
     /// Requests the flash builder to output the layout into the given file in SVG format.
-    #[arg(value_name = "filename", long = "flash-layout")]
+    #[arg(
+        value_name = "filename",
+        long = "flash-layout",
+        help_heading = "DOWNLOAD CONFIGURATION"
+    )]
     pub flash_layout_output_path: Option<String>,
     /// After flashing, read back all the flashed data to verify it has been written correctly.
-    #[arg(long)]
+    #[arg(long, help_heading = "DOWNLOAD CONFIGURATION")]
     pub verify: bool,
 }
 
@@ -126,12 +130,13 @@ pub struct ReadWriteOptions {
 /// Common options and logic when interfacing with a [Probe].
 #[derive(clap::Parser, Debug)]
 pub struct ProbeOptions {
-    #[arg(long, env = "PROBE_RS_CHIP")]
+    #[arg(long, env = "PROBE_RS_CHIP", help_heading = "PROBE CONFIGURATION")]
     pub chip: Option<String>,
     #[arg(
         value_name = "chip description file path",
         long,
-        env = "PROBE_RS_CHIP_DESCRIPTION_PATH"
+        env = "PROBE_RS_CHIP_DESCRIPTION_PATH",
+        help_heading = "PROBE CONFIGURATION"
     )]
     pub chip_description_path: Option<PathBuf>,
 
@@ -150,13 +155,21 @@ pub struct ProbeOptions {
     pub speed: Option<u32>,
     /// Use this flag to assert the nreset & ntrst pins during attaching the probe to
     /// the chip.
-    #[arg(long, env = "PROBE_RS_CONNECT_UNDER_RESET")]
+    #[arg(
+        long,
+        env = "PROBE_RS_CONNECT_UNDER_RESET",
+        help_heading = "PROBE CONFIGURATION"
+    )]
     pub connect_under_reset: bool,
-    #[arg(long, env = "PROBE_RS_DRY_RUN")]
+    #[arg(long, env = "PROBE_RS_DRY_RUN", help_heading = "PROBE CONFIGURATION")]
     pub dry_run: bool,
     /// Use this flag to allow all memory, including security keys and 3rd party
     /// firmware, to be erased even when it has read-only protection.
-    #[arg(long, env = "PROBE_RS_ALLOW_ERASE_ALL")]
+    #[arg(
+        long,
+        env = "PROBE_RS_ALLOW_ERASE_ALL",
+        help_heading = "PROBE CONFIGURATION"
+    )]
     pub allow_erase_all: bool,
 }