Add default error function

CHANGELOG.md

@@ -15,6 +15,10 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how
     Fixed -- for any bug fixes.
     Security -- in case of vulnerabilities.
 -->
+## [0.15.3]
+
+- Fix issue where unrecognized model number causes kic-cli to never exit (TSP-645)
+
 ## [0.15.1]
 
 ### Changed

Cargo.toml

@@ -7,7 +7,7 @@ members = [
 resolver = "2"
 
 [workspace.package]
-version = "0.15.1"
+version = "0.15.3"
 authors = ["Keithley Instruments, LLC"]
 edition = "2021"
 repository = "https://github.com/tektronix/tsp-toolkit-kic-cli"
@@ -44,3 +44,15 @@ tracing = { version = "0.1.40", features = ["async-await"] }
 tracing-subscriber = "0.3.18"
 tsp-toolkit-kic-lib = { git = "https://github.com/tektronix/tsp-toolkit-kic-lib.git", tag = "v0.15.1" }
 
+[workspace.lints.rust]
+warnings = "deny"
+
+[workspace.lints.clippy]
+pedantic = { level = "deny", priority = -1 }
+nursery = { level = "deny", priority = -1 }
+undocumented_unsafe_blocks = "deny"
+arithmetic_side_effects = "deny"
+
+[workspace.lints.rustdoc]
+all = "warn"
+missing_doc_code_examples = "warn"

instrument-repl/Cargo.toml

@@ -17,4 +17,5 @@ shlex = "1.3.0"
 tracing = { workspace = true }
 tsp-toolkit-kic-lib = { workspace = true }
 chrono = "0.4.34"
-regex = "1.10.3"
+regex = "1.10.3"
+

instrument-repl/src/resources/kic_common.tsp

@@ -64,13 +64,14 @@ local models = {
     ["708B"] = _2600,
     ["VERSATEST-600"] = VERSATEST,
     ["TSPop"] = VERSATEST,
+    ["TSP"] = VERSATEST,
 }
 
 _KIC["is_tti"] = function() return models[localnode.model] == TTI end
 _KIC["is_2600"] = function() return models[localnode.model] == _2600 end
 _KIC["is_versatest"] = function() return models[localnode.model] == VERSATEST end
 
----interate over input string and escape special characters in a given string, 
+---interate over input string and escape special characters in a given string,
 ---making it safe for inclusion in JSON
 ---@param s string
 ---@return string
@@ -151,6 +152,22 @@ elseif _KIC.is_versatest() then
         errorstr = errorstr..[[ERM>DONE]]
         return errorstr
     end
+else
+    -- Default function declarations just in case we encounter an unknown instrument
+    -- model. Anything defined here should return a string that satisfies the
+    -- caller so kic-cli can complete.
+    _KIC["error_message"] = function()
+        local errorstr = [[ERM>START]].."\n"
+        for _err_num = 1, errorqueue.count do
+            error_code = 0
+            severity = 0
+            node_id = 0
+            message = "Model number not recognized: '"..localnode.model.."'"
+            errorstr = errorstr.. "ERM>" .._KIC.toJson({error_code=error_code, message=escape_str(message), severity=severity, node_id=node_id, time=nil}) .. "\n"
+        end
+        errorstr = errorstr..[[ERM>DONE]]
+        return errorstr
+    end
 
 end
 

kic-discover/Cargo.toml

@@ -32,3 +32,4 @@ tokio = { version = "1.36.0", features = ["full"] }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 tsp-toolkit-kic-lib = { workspace = true }
+

kic-discover/src/ethernet/mod.rs

@@ -20,6 +20,7 @@ pub const SERVICE_NAMES: [&str; 3] = [
     //"_scpi-telnet._tcp.local",
 ];
 
+#[allow(clippy::unsafe_derive_deserialize)]
 #[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash, Serialize, Deserialize)]
 pub struct LxiDeviceInfo {
     io_type: IoType,
@@ -36,7 +37,7 @@ impl LxiDeviceInfo {
     async fn discover_devices(
         service_name: &str,
         interface_ip: Ipv4Addr,
-        device_tx: UnboundedSender<LxiDeviceInfo>,
+        device_tx: UnboundedSender<Self>,
     ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
         // The hostname of the devices we are searching for.
         // Every lxi compliant instrument will respond to this service name.
@@ -81,24 +82,34 @@ impl LxiDeviceInfo {
     }
 
     #[must_use]
-    pub fn parse_lxi_xml(xml_data: &str, instr_addr: IpAddr) -> Option<LxiDeviceInfo> {
+    pub fn parse_lxi_xml(xml_data: &str, instr_addr: IpAddr) -> Option<Self> {
         const DEVICE_NS: &str = "http://www.lxistandard.org/InstrumentIdentification/1.0";
         if let Ok(root) = xml_data.parse::<Element>() {
             if root.is("LXIDevice", DEVICE_NS) {
-                let manufacturer = root.get_child("Manufacturer", DEVICE_NS).unwrap().text();
-                let model = root.get_child("Model", DEVICE_NS).unwrap().text();
-                let serial_number = root.get_child("SerialNumber", DEVICE_NS).unwrap().text();
+                let manufacturer = root
+                    .get_child("Manufacturer", DEVICE_NS)
+                    .unwrap_or(&minidom::Element::bare("FirmwareRevision", DEVICE_NS))
+                    .text();
+                let model = root
+                    .get_child("Model", DEVICE_NS)
+                    .unwrap_or(&minidom::Element::bare("FirmwareRevision", DEVICE_NS))
+                    .text();
+                let serial_number = root
+                    .get_child("SerialNumber", DEVICE_NS)
+                    .unwrap_or(&minidom::Element::bare("FirmwareRevision", DEVICE_NS))
+                    .text();
                 let firmware_revision = root
                     .get_child("FirmwareRevision", DEVICE_NS)
-                    .unwrap()
+                    .unwrap_or(&minidom::Element::bare("FirmwareRevision", DEVICE_NS))
                     .text();
 
                 let s1: Vec<&str> = xml_data.split("::SOCKET").collect();
                 let port_split: Vec<&str> = s1[0].split("::").collect();
-                let mut socket_port = ":5025".to_owned();
-                if port_split.is_empty() {
-                    socket_port = format!(":{}", port_split[port_split.len() - 1]);
-                }
+                let socket_port = if port_split.is_empty() {
+                    format!(":{}", port_split[port_split.len().saturating_sub(1)])
+                } else {
+                    ":5025".to_string()
+                };
 
                 //ToDo: test versatest when it's discoverable
                 let res = model_check(model.as_str());
@@ -133,11 +144,13 @@ impl LxiDeviceInfo {
 }
 
 impl LxiDeviceInfo {
+    ///Discover LXI devices
+    ///
+    ///# Errors
+    ///Possible errors include but are not limited to those generated by trying
+    ///to gather the network interface IPs to iterate over for our search.
     pub async fn discover(timeout: Option<Duration>) -> anyhow::Result<HashSet<Self>> {
         let timeout = timeout.unwrap_or(Duration::new(5, 0));
-        // let rt = tokio::runtime::Builder::new_current_thread()
-        //     .enable_all()
-        //     .build()?;
 
         let mut discover_futures = Vec::new();
 
@@ -148,12 +161,12 @@ impl LxiDeviceInfo {
 
         let (device_tx, mut device_rx) = unbounded_channel();
 
-        'interface_loop: for (_name, ip) in interfaces {
+        'interface_loop: for (name, ip) in interfaces {
             for service_name in SERVICE_NAMES {
                 #[cfg(debug_assertions)]
-                eprintln!("Looking for {service_name} on {_name} ({ip})");
+                eprintln!("Looking for {service_name} on {name} ({ip})");
                 if let IpAddr::V4(ip) = ip {
-                    discover_futures.push(LxiDeviceInfo::discover_devices(
+                    discover_futures.push(Self::discover_devices(
                         service_name,
                         ip,
                         device_tx.clone(),
@@ -164,7 +177,7 @@ impl LxiDeviceInfo {
             }
         }
 
-        let mut devices: HashSet<LxiDeviceInfo> = HashSet::new();
+        let mut devices: HashSet<Self> = HashSet::new();
 
         // ignore the error from the timeout since that is our method of stopping execution of the futures
         // This if statement prevents a must_use warning

kic-discover/src/instrument_discovery/mod.rs

@@ -13,8 +13,8 @@ pub struct InstrumentDiscovery {
 
 impl InstrumentDiscovery {
     #[must_use]
-    pub fn new(timeout: Duration) -> InstrumentDiscovery {
-        InstrumentDiscovery {
+    pub const fn new(timeout: Duration) -> Self {
+        Self {
             timeout: Some(timeout),
         }
     }
@@ -38,6 +38,10 @@ impl InstrumentDiscovery {
     //     Ok(discovery_results)
     // }
 
+    /// Discover instruments on the network.
+    ///
+    /// # Errors
+    /// If [`LxiDeviceInfo::discover`] fails, an error will be returned
     pub async fn lan_discover(&self) -> anyhow::Result<HashSet<InstrumentInfo>> {
         let mut discovery_results: HashSet<InstrumentInfo> = HashSet::new();
 
@@ -55,6 +59,10 @@ impl InstrumentDiscovery {
         Ok(discovery_results)
     }
 
+    /// Discover instruments over USB
+    ///
+    /// # Errors
+    /// If [`Usbtmc::usb_discover`] fails, and error will be returned.
     pub async fn usb_discover(&self) -> anyhow::Result<HashSet<InstrumentInfo>> {
         let mut discovery_results: HashSet<InstrumentInfo> = HashSet::new();
 

kic-discover/src/lib.rs

@@ -106,15 +106,21 @@ pub fn is_nimitz(in_str: &str) -> bool {
     false
 }
 
+/// Insert a discovered device into our map of instruments
+///
+/// # Errors
+/// If we fail to lock the `DISC_INSTRUMENTS` variable, a [`std::io::Error`]
+/// with [`std::io::ErrorKind::PermissionDenied`] will be returned.
 pub fn insert_disc_device(device: &str) -> Result<(), Error> {
-    let mut db = DISC_INSTRUMENTS.lock().map_err(|_| {
-        std::io::Error::new(
-            std::io::ErrorKind::PermissionDenied,
-            "failed to acquire".to_string(),
-        )
-    })?;
-
-    db.insert(device.to_string());
+    DISC_INSTRUMENTS
+        .lock()
+        .map_err(|_| {
+            std::io::Error::new(
+                std::io::ErrorKind::PermissionDenied,
+                "failed to acquire".to_string(),
+            )
+        })?
+        .insert(device.to_string());
     Ok(())
 }
 

kic-discover/src/main.rs

@@ -83,28 +83,28 @@ async fn main() -> anyhow::Result<()> {
             let lan_instruments = discover_lan(args).await?;
             println!("Discovered {} Lan instruments", lan_instruments.len());
             for instrument in lan_instruments {
-                println!("{}", instrument);
+                println!("{instrument}");
             }
         }
         SubCli::Usb(_) => {
             #[allow(clippy::mutable_key_type)]
             let usb_instruments = discover_usb().await?;
             for instrument in usb_instruments {
-                println!("{}", instrument);
+                println!("{instrument}");
             }
         }
-        SubCli::All(_args) => {
+        SubCli::All(args) => {
             #[allow(clippy::mutable_key_type)]
             let usb_instruments = discover_usb().await?;
             for instrument in usb_instruments {
-                println!("{}", instrument);
+                println!("{instrument}");
             }
 
             #[allow(clippy::mutable_key_type)]
-            let lan_instruments = discover_lan(_args).await?;
+            let lan_instruments = discover_lan(args).await?;
             println!("Discovered {} Lan instruments", lan_instruments.len());
             for instrument in lan_instruments {
-                println!("{}", instrument);
+                println!("{instrument}");
             }
         }
     }
@@ -117,9 +117,9 @@ async fn main() -> anyhow::Result<()> {
     Ok(())
 }
 
-fn require_exit_timer(sub: &SubCli) -> bool {
-    if let SubCli::All(_args) = sub {
-        if _args.exit {
+const fn require_exit_timer(sub: &SubCli) -> bool {
+    if let SubCli::All(args) = sub {
+        if args.exit {
             return true;
         }
     }
@@ -130,8 +130,8 @@ async fn init_rpc() -> anyhow::Result<ServerHandle> {
     let server = Server::builder().build("127.0.0.1:3030").await?;
 
     let mut module = RpcModule::new(());
-    module.register_method("get_instr_list", |_, _| {
-        let mut new_out_str = "".to_owned();
+    module.register_method("get_instr_list", |_, ()| {
+        let mut new_out_str = String::new();
 
         if let Ok(db) = DISC_INSTRUMENTS.lock() {
             db.iter()
@@ -152,15 +152,15 @@ async fn init_rpc() -> anyhow::Result<ServerHandle> {
 }
 
 async fn discover_lan(args: DiscoverCmd) -> anyhow::Result<HashSet<InstrumentInfo>> {
-    let mut instr_str = "".to_owned();
+    let mut instr_str = String::new();
     let dur = Duration::from_secs(args.timeout_secs.unwrap_or(20) as u64);
     let discover_instance = InstrumentDiscovery::new(dur);
     let instruments = discover_instance.lan_discover().await;
 
     match &instruments {
         Ok(instrs_set) => {
             for instr in instrs_set {
-                instr_str = format!("{}{}\n", instr_str, instr);
+                instr_str = format!("{instr_str}{instr}\n");
             }
         }
 
@@ -177,7 +177,7 @@ async fn discover_lan(args: DiscoverCmd) -> anyhow::Result<HashSet<InstrumentInf
 }
 
 async fn discover_usb() -> anyhow::Result<HashSet<InstrumentInfo>> {
-    let mut instr_str = "".to_owned();
+    let mut instr_str = String::new();
 
     let dur = Duration::from_secs(5); //Not used in USB
     let discover_instance = InstrumentDiscovery::new(dur);
@@ -186,7 +186,7 @@ async fn discover_usb() -> anyhow::Result<HashSet<InstrumentInfo>> {
     match &instruments {
         Ok(instrs_set) => {
             for instr in instrs_set {
-                instr_str = format!("{}{}\n", instr_str, instr);
+                instr_str = format!("{instr_str}{instr}\n");
             }
         }