Some cleanup, improvements to CLI options/docs

src/main.rs

@@ -1,12 +1,5 @@
 use core::fmt;
-use std::{
+use std::{ffi::CString, fs::File, io::BufWriter, os::unix::prelude::OsStrExt, path::PathBuf};
-    ffi::CString,
-    fs::File,
-    fs::{self, OpenOptions},
-    io::BufWriter,
-    os::unix::prelude::OsStrExt,
-    path::PathBuf,
-};
 
 use anyhow::{anyhow, Context, Error};
 use clap_num::maybe_hex;
@@ -15,7 +8,6 @@ use file::ObjectFileReader;
 use log::{debug, error, info, log_enabled, warn, Level};
 
 use hidapi::{DeviceInfo, HidApi};
-use object::Object;
 
 use crate::file::{MemoryFileType, MemoryWriter};
 use clap::{
@@ -48,6 +40,18 @@ struct VidPid {
     vid: u16,
     pid: u16,
 }
+impl fmt::Display for VidPid {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_fmt(format_args!("{:04x}:{:04x}", self.vid, self.pid))
+    }
+}
+
+impl ValueParserFactory for VidPid {
+    type Parser = VidPidParser;
+    fn value_parser() -> Self::Parser {
+        VidPidParser
+    }
+}
 
 #[derive(Clone)]
 struct VidPidParser;
@@ -75,13 +79,6 @@ impl TypedValueParser for VidPidParser {
     }
 }
 
-impl ValueParserFactory for VidPid {
-    type Parser = VidPidParser;
-    fn value_parser() -> Self::Parser {
-        VidPidParser
-    }
-}
-
 #[derive(Parser, Debug)]
 #[command(author, version, about)]
 struct Cli {
@@ -92,7 +89,7 @@ struct Cli {
 #[derive(Args, Debug)]
 struct UsbDeviceSpecifier {
     /// USB VID:PID as colon-separated pair
-    #[arg(short = 'i', long, default_value = "1fc9:0021", group = "device")]
+    #[arg(short = 'i', long, default_value_t = VidPid{vid:0x1fc9,pid:0x0021}, group = "device")]
     usb_id: VidPid,
 
     /// hidraw device node path
@@ -105,9 +102,20 @@ enum ObjectFileType {
     Elf,
     Ihex,
     Bin,
+    /// Derive file type from file name
     Auto,
 }
 
+#[derive(ValueEnum, Debug, Clone)]
+enum WriteEraseMethod {
+    /// Erase the entire flash
+    All,
+    /// Erase only the area to be written
+    Region,
+    /// Don't do an erase. Memory to be written must be erased already if it's flash.
+    None,
+}
+
 impl ObjectFileType {
     fn mem_writer(&self, filename: &PathBuf) -> Box<dyn MemoryWriter> {
         match self {
@@ -152,21 +160,18 @@ struct WriteArgs {
     /// USB device to act on
     #[command(flatten)]
     devspec: UsbDeviceSpecifier,
-    /// Base memory address on microcontroller (defaults to start of flash)
+    /// Base memory address on microcontroller to write to (defaults to start of flash)
     #[arg(short = 'a', long = "write-address", value_parser=maybe_hex::<u32>)]
     addr: Option<u32>,
-    /// Base memory address in <FILE> (defaults to the start of the first TEXT block in the file)
-    #[arg(long="read-address", value_parser=maybe_hex::<u32>)]
-    read_addr: Option<u32>,
     /// Size to write in bytes (defaults to size of flash)
     #[arg(short, long, value_parser=maybe_hex::<u32>)]
     size: Option<u32>,
     /// Don't reset the microcontroller after writing
     #[arg(short, long, action=ArgAction::SetFalse)]
     no_reset: bool,
-    #[arg(short, long)]
+    #[arg(short, long, default_value = "region")]
-    /// Erase all flash before writing
+    /// Erase method to use before writing
-    erase: bool,
+    erase: WriteEraseMethod,
 }
 
 #[derive(Args, Debug)]
@@ -182,10 +187,15 @@ struct EraseArgs {
     #[command(flatten)]
     devspec: UsbDeviceSpecifier,
 
+    #[arg(long = "all", default_value_t = true)]
+    /// Erase the entire chip
+    erase_all: bool,
+
     /// Address to start erasing at
-    #[arg(short = 'a', long="base-address", value_parser=maybe_hex::<u32>, requires="size")]
+    #[arg(short = 'a', long="base-address", value_parser=maybe_hex::<u32>, conflicts_with="erase_all")]
     addr: Option<u32>,
-    #[arg(short, long, value_parser=maybe_hex::<u32>)]
+    /// Number of bytes to erase
+    #[arg(short, long, value_parser=maybe_hex::<u32>, conflicts_with="erase_all")]
     size: Option<u32>,
 }
 
@@ -221,16 +231,33 @@ fn write_file_to_flash(args: &WriteArgs) -> Result<(), Error> {
     let api = HidApi::new()?;
     let isp = connect_device(&api, &args.devspec)?;
 
-    let infile = args.filetype.file_reader(&args.file)
+    let mut infile = args
+        .filetype
+        .file_reader(&args.file)
         .with_context(|| format!("Opening {} for reading", args.file.display()))?;
 
+    let contents = infile
+        .read_all(None)
+        .or_else(|e| Err(anyhow!("Unable to read file contents: {}", e)))?;
+
+    println!("Input file first section contents:");
+    for (i, line) in contents.chunks(16).enumerate() {
+        print!("{:08x} ", i * 16);
+        for block in line.chunks(8) {
+            for s in block.iter().map(|x| format!("{:02x} ", x)) {
+                print!("{}", s);
+            }
+            print!(" ");
+        }
+        println!();
+    }
 
     let flash_start = args.addr.unwrap_or(isp.GetFlashStartAddress()?);
     let flash_size = args.size.unwrap_or(isp.GetFlashSizeInBytes()?);
 
-    if !args.no_reset {
+    // if !args.no_reset {
-        isp.reset()?;
+    //     isp.reset()?;
-    }
+    // }
 
     Ok(())
 }
@@ -298,7 +325,7 @@ fn erase_flash(args: &EraseArgs) -> Result<(), Error> {
     let erase_pb = ProgressBar::new(1)
         .with_style(read_write_style())
         .with_prefix("Erasing flash");
-    if args.addr.is_none() && args.size.is_none() {
+    if args.erase_all {
         isp.flash_erase_all(None)?;
         erase_pb.finish();
     } else {

src/protocol.rs

@@ -1,19 +1,19 @@
+// Allow non snake case to match with NXP documentation
 #![allow(non_snake_case)]
 
 use anyhow::{anyhow, Error};
-use byteorder::{ByteOrder, LittleEndian, NativeEndian};
+use hidapi::HidDevice;
-use hidapi::{HidApi, HidDevice};
 use log::debug;
-use num_enum::{FromPrimitive, IntoPrimitive, TryFromPrimitive};
+use num_enum::{FromPrimitive, IntoPrimitive};
 use std::fmt::{Debug, Display};
-use std::io::{Cursor, Write};
+use std::io::Write;
 use strum::IntoEnumIterator;
 use strum_macros::EnumIter;
 use uuid::Uuid;
 
 use crate::packet::{
-    self, GenericResponseParams, GetPropertyResponseParams, Packet, ResponsePacket,
+    self, GetPropertyResponseParams, Packet, ResponsePacket, ResponseParameters, StatusCode,
-    ResponseParameters, StatusCode, UsbPacket,
+    UsbPacket,
 };
 use crate::packet::{CommandFlags, CommandPacket, CommandTag};
 
@@ -329,7 +329,7 @@ impl UsbIsp {
         usb_packet.write(&mut buf)?;
 
         //FIXME: don't expect a response on data packets
-        let rsize = self.device.write(&buf)?;
+        let _ = self.device.write(&buf)?;
         let mut rbuf = [0u8; 64];
         let res_size = self.device.read(&mut rbuf[..])?;
         let parsed = packet::usb_packet(&rbuf[..res_size]).unwrap(); //TODO: handle more gracefully, but requires ownership of I