Merge pull request #4 from physwkim/fix/pvinfo-and-get-init

fix: pvinfo GET_FIELD bug, ChannelConn server_addr, GET init for data-less PVs

Good fixes, 

I re-isntated and updated some old p4p (pvxs under the hood) interops test I had, I plan to have full interop with all major pvaccess libraries in the long term as well as full protocol completness when needed.

Author: MatInGit

spvirit-client/src/client.rs

@@ -6,6 +6,7 @@ use crate::auth::{resolved_authnz_host, resolved_authnz_user};
 use crate::search::resolve_pv_server;
 use crate::transport::{read_packet, read_until};
 use crate::types::{PvGetError, PvGetOptions, PvGetResult};
+use spvirit_codec::spvd_encode::encode_pv_request;
 use spvirit_codec::epics_decode::{
     PvaPacket, PvaPacketCommand, decode_op_response_status as codec_decode_op_response_status,
 };
@@ -49,6 +50,7 @@ pub struct ChannelConn {
     pub sid: u32,
     pub version: u8,
     pub is_be: bool,
+    pub server_addr: std::net::SocketAddr,
 }
 
 pub async fn establish_channel(
@@ -129,10 +131,23 @@ pub async fn establish_channel(
         sid,
         version,
         is_be,
+        server_addr: target,
     })
 }
 
+/// Convenience wrapper: GET with no field filtering.
 pub async fn pvget(opts: &PvGetOptions) -> Result<PvGetResult, PvGetError> {
+    pvget_fields(opts, &[]).await
+}
+
+/// GET with optional field filtering.
+///
+/// If `fields` is empty, requests all fields (equivalent to `-r ""`).
+/// Otherwise, encodes a pvRequest like `field(value,alarm,timeStamp)`.
+pub async fn pvget_fields(
+    opts: &PvGetOptions,
+    fields: &[&str],
+) -> Result<PvGetResult, PvGetError> {
     let target = resolve_pv_server(opts).await?;
 
     let conn = establish_channel(target, opts).await?;
@@ -141,15 +156,21 @@ pub async fn pvget(opts: &PvGetOptions) -> Result<PvGetResult, PvGetError> {
         sid,
         version,
         is_be,
+        ..
     } = conn;
 
     let ioid = 1u32;
-    // Match EPICS pvget GET init payload (extra pvRequest bytes observed in capture).
+    let pv_request = if fields.is_empty() {
+        // Empty pvRequest — request all fields
+        vec![0xfd, 0x02, 0x00, 0x80, 0x00, 0x00]
+    } else {
+        encode_pv_request(fields, is_be)
+    };
     let get_init_req = encode_get_request(
         sid,
         ioid,
         0x08,
-        &[0xfd, 0x02, 0x00, 0x80, 0x00, 0x00],
+        &pv_request,
         version,
         is_be,
     );

spvirit-client/src/lib.rs

@@ -52,7 +52,7 @@ pub use pva_client::{
 pub use pvlist::PvListSource;
 
 // --- Re-exports: client core ---
-pub use client::{ChannelConn, build_client_validation, establish_channel, pvget};
+pub use client::{ChannelConn, build_client_validation, establish_channel, pvget, pvget_fields};
 pub use format::{OutputFormat, RenderOptions, format_output};
 pub use search::{SearchTarget, build_auto_broadcast_targets, resolve_pv_server, search_pv};
 pub use transport::read_packet;

spvirit-client/src/pva_client.rs

@@ -233,6 +233,16 @@ impl PvaClient {
         low_level_pvget(&opts).await
     }
 
+    /// Fetch a PV with field filtering (equivalent to `pvget -r "field(value,alarm)"`).
+    pub async fn pvget_fields(
+        &self,
+        pv_name: &str,
+        fields: &[&str],
+    ) -> Result<PvGetResult, PvGetError> {
+        let opts = self.opts(pv_name);
+        crate::client::pvget_fields(&opts, fields).await
+    }
+
     // ─── pvput ───────────────────────────────────────────────────────────
 
     /// Write a value to a PV.
@@ -250,6 +260,7 @@ impl PvaClient {
             sid,
             version: _,
             is_be,
+            ..
         } = self.open_channel(pv_name).await?;
 
         let ioid = alloc_ioid();
@@ -298,6 +309,7 @@ impl PvaClient {
             sid,
             version,
             is_be,
+            ..
         } = self.open_channel(pv_name).await?;
 
         let ioid = alloc_ioid();
@@ -383,6 +395,7 @@ impl PvaClient {
             sid,
             version: _,
             is_be,
+            ..
         } = self.open_channel(pv_name).await?;
 
         let ioid = alloc_ioid();
@@ -445,11 +458,21 @@ impl PvaClient {
 
     /// Retrieve the field/structure description (introspection) for a PV.
     pub async fn pvinfo(&self, pv_name: &str) -> Result<StructureDesc, PvGetError> {
+        let result = self.pvinfo_full(pv_name).await?;
+        Ok(result.0)
+    }
+
+    /// Retrieve introspection and server address for a PV.
+    pub async fn pvinfo_full(
+        &self,
+        pv_name: &str,
+    ) -> Result<(StructureDesc, SocketAddr), PvGetError> {
         let ChannelConn {
             mut stream,
             sid,
             version: _,
             is_be,
+            server_addr,
         } = self.open_channel(pv_name).await?;
 
         let ioid = alloc_ioid();
@@ -459,11 +482,34 @@ impl PvaClient {
         let resp_bytes = read_until(
             &mut stream,
             self.timeout,
-            |cmd| matches!(cmd, PvaPacketCommand::Op(op) if op.command == 17),
+            |cmd| matches!(cmd, PvaPacketCommand::GetField(_)),
         )
         .await?;
 
-        decode_init_introspection(&resp_bytes, "GET_FIELD")
+        let mut pkt = PvaPacket::new(&resp_bytes);
+        let cmd = pkt
+            .decode_payload()
+            .ok_or_else(|| PvGetError::Decode("GET_FIELD response decode failed".to_string()))?;
+        match cmd {
+            PvaPacketCommand::GetField(payload) => {
+                if let Some(ref st) = payload.status {
+                    if st.is_error() {
+                        let msg = st
+                            .message
+                            .clone()
+                            .unwrap_or_else(|| format!("code={}", st.code));
+                        return Err(PvGetError::Protocol(format!("GET_FIELD error: {msg}")));
+                    }
+                }
+                let desc = payload
+                    .introspection
+                    .ok_or_else(|| PvGetError::Decode("missing GET_FIELD introspection".to_string()))?;
+                Ok((desc, server_addr))
+            }
+            _ => Err(PvGetError::Protocol(
+                "unexpected GET_FIELD response".to_string(),
+            )),
+        }
     }
 
     // ─── pvlist ──────────────────────────────────────────────────────────

spvirit-client/src/pvlist.rs

@@ -306,6 +306,7 @@ async fn list_pvs_via_server_rpc_channel(
         sid,
         version,
         is_be,
+        ..
     } = establish_channel(server_addr, &rpc_opts).await?;
 
     let ioid = 1u32;
@@ -421,6 +422,7 @@ pub async fn list_pvs_via_server_get(
             sid,
             version,
             is_be,
+            ..
         } = establish_channel(server_addr, &get_opts).await?;
 
         let ioid = 1u32;

spvirit-codec/src/spvd_decode.rs

@@ -293,11 +293,17 @@ impl fmt::Display for DecodedValue {
 /// PVD Decoder state
 pub struct PvdDecoder {
     is_be: bool,
+    /// IntrospectionRegistry: maps int16 keys to previously seen FieldTypes.
+    /// Populated when parsing `0xFD` (full-with-id) entries, looked up on `0xFE` (only-id).
+    registry: std::cell::RefCell<std::collections::HashMap<u16, FieldType>>,
 }
 
 impl PvdDecoder {
     pub fn new(is_be: bool) -> Self {
-        Self { is_be }
+        Self {
+            is_be,
+            registry: std::cell::RefCell::new(std::collections::HashMap::new()),
+        }
     }
 
     /// Decode a size value (PVA variable-length encoding)
@@ -382,24 +388,36 @@ impl PvdDecoder {
         // Full-with-id from IntrospectionRegistry:
         // 0xFD + int16 key + type descriptor payload.
         if type_byte == 0xFD {
-            if data.len() < 4 {
+            if data.len() < 3 {
                 return None;
             }
-            // Preferred parsing path: skip tag + int16 key.
+            let key = if self.is_be {
+                u16::from_be_bytes([data[1], data[2]])
+            } else {
+                u16::from_le_bytes([data[1], data[2]])
+            };
             if let Some((field_type, consumed)) = self.parse_type_desc(&data[3..]) {
+                self.registry.borrow_mut().insert(key, field_type.clone());
                 return Some((field_type, 3 + consumed));
             }
-            // Legacy fallback for older non-keyed streams.
-            if let Some((field_type, consumed)) = self.parse_type_desc(&data[1..]) {
-                return Some((field_type, 1 + consumed));
-            }
             return None;
         }
 
         // Only-id from IntrospectionRegistry:
-        // 0xFE + int16 key, requires connection-level registry state.
+        // 0xFE + int16 key — reference to a previously seen type.
         if type_byte == 0xFE {
-            debug!("Type descriptor uses ONLY_ID (0xFE) without registry context");
+            if data.len() < 3 {
+                return None;
+            }
+            let key = if self.is_be {
+                u16::from_be_bytes([data[1], data[2]])
+            } else {
+                u16::from_le_bytes([data[1], data[2]])
+            };
+            if let Some(ft) = self.registry.borrow().get(&key) {
+                return Some((ft.clone(), 3));
+            }
+            debug!("Type descriptor ONLY_ID (0xFE) key={} not found in registry", key);
             return None;
         }
 
@@ -549,26 +567,49 @@ impl PvdDecoder {
         // Full-with-id from IntrospectionRegistry:
         // 0xFD + int16 key + field type descriptor payload.
         if type_byte == 0xFD {
-            if data.len() < 4 {
+            if data.len() < 3 {
                 return None;
             }
-            // Preferred parsing path: skip tag + int16 key.
+            let key = if self.is_be {
+                u16::from_be_bytes([data[1], data[2]])
+            } else {
+                u16::from_le_bytes([data[1], data[2]])
+            };
             if let Some((desc, consumed)) = self.parse_introspection_with_len(&data[3..]) {
+                // Register this structure type for later 0xFE references
+                if !desc.fields.is_empty() {
+                    self.registry.borrow_mut().insert(
+                        key,
+                        FieldType::Structure(desc.clone()),
+                    );
+                } else {
+                    self.registry.borrow_mut().insert(
+                        key,
+                        FieldType::Structure(desc.clone()),
+                    );
+                }
                 return Some((desc, 3 + consumed));
             }
-            // Legacy fallback for older non-keyed streams.
-            if let Some((desc, consumed)) = self.parse_introspection_with_len(&data[1..]) {
-                return Some((desc, 1 + consumed));
-            }
             return None;
         }
 
         // Only-id from IntrospectionRegistry:
-        // 0xFE + int16 key, requires connection-level registry state.
+        // 0xFE + int16 key — reference to a previously seen type.
         if type_byte == 0xFE {
-            debug!(
-                "Introspection uses ONLY_ID (0xFE), but no registry is available in this decoder"
-            );
+            if data.len() < 3 {
+                return None;
+            }
+            let key = if self.is_be {
+                u16::from_be_bytes([data[1], data[2]])
+            } else {
+                u16::from_le_bytes([data[1], data[2]])
+            };
+            if let Some(ft) = self.registry.borrow().get(&key) {
+                if let FieldType::Structure(desc) = ft {
+                    return Some((desc.clone(), 3));
+                }
+            }
+            debug!("Introspection ONLY_ID (0xFE) key={} not found in registry", key);
             return None;
         }
 

spvirit-codec/src/spvd_encode.rs

@@ -889,6 +889,10 @@ fn encode_codec_parameters(
     out
 }
 
+pub fn nt_ndarray_desc_default() -> StructureDesc {
+    nt_ndarray_desc(&NtNdArray::empty())
+}
+
 pub fn nt_ndarray_desc(_nt: &NtNdArray) -> StructureDesc {
     StructureDesc {
         struct_id: Some("epics:nt/NTNDArray:1.0".to_string()),