Support for querying scoped fields
The current implementation of Device::field_values_for is too limiting. The caller can only pass in a slice of field IDs to populate nvmlFieldValue_t::fieldId while all other struct members are set to zero, while nvmlDeviceGetFieldValues actually also uses nvmlFieldValue_t::scopeId as input when querying several fields such as NVLink remote IDs, NVLink ECC counters, or power draw and power limits.
I would like to contribute a function that allows passing in both fieldId and scopeId and was wondering whether that should replace Device::field_values_for or be a separate function, e.g. Device::scoped_field_values_for.
Thanks for the heads up, looks like scopeId was added after I initially created that wrapper 🙂. We should definitely take that into account.
What do you think about changing:
pub fn field_values_for(
&self,
id_slice: &[FieldId],
) -> Result<Vec<Result<FieldValueSample, NvmlError>>, NvmlError> {
to:
pub fn field_values_for(
&self,
requests_slice: &[FieldValueRequest],
) -> Result<Vec<Result<FieldValueSample, NvmlError>>, NvmlError> {
Where FieldValueRequest is:
/// Specify a field ID and an optional scope ID for requesting data samples
/// from a device.
///
/// Used in [`crate::struct_wrappers::device::FieldValueSample`] and
/// [`crate::device::Device::field_values_for()`].
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub struct FieldValueRequest {
/// Populate this newtype with the constants `nvml_wrapper::sys_exports::field_id::*`.
pub id: FieldId,
/// Optionally populate this with a `scopeId` appropriate for the associated [`FieldId`].
///
/// See NVIDIA's field ID constant docs (`NVML_FI_*`) to understand what scope
/// IDs may be valid for which field IDs.
pub scope_id: Option<ScopeId>,
}
impl FieldValueRequest {
pub fn id(id: FieldId) -> Self {
Self { id, scope_id: None }
}
pub fn id_with_scope(id: FieldId, scope_id: ScopeId) -> Self {
Self {
id,
scope_id: Some(scope_id),
}
}
}
That makes sense. I have submitted PR #55 which changes field_values_for to accept an Iterator of FieldValueRequests. I have also changed FieldValueSample to contain the ScopeId as well to avoid ambiguities.