traited sdo writing

[jbbjarnason]

Prereq, let's use this enum for example purposes:

#[derive(Debug, EtherCrabWireReadWrite, Copy, Clone, Serialize, Deserialize, JsonSchema)]
#[repr(u16)]
enum Filter {
    FIR50 = 0,
    FIR60 = 1,
    IIR1 = 2,
    IIR2 = 3,
    IIR3 = 4,
    IIR4 = 5,
    IIR5 = 6,
    IIR6 = 7,
    IIR7 = 8,
    IIR8 = 9,
    DynamicIIR = 10,
    PDOFilterFrequency = 11,
}
impl Default for Filter {
    fn default() -> Self {
        Self::FIR50
    }
}
impl Index for Filter {
    const INDEX: u16 = 0x8000;
    const SUBINDEX: u8 = 0x11;
}

I found it to be repetative to write the following, since I write filter in

        device.sdo_write(0x8000, 0x11, filter).await?;

So I added those two traits

pub trait Index {
    const INDEX: u16;
    const SUBINDEX: u8;
}

pub trait WriteValueIndex {
    async fn sdo_write_value_index<T>(&mut self, value: T) -> Result<(), ethercrab::error::Error>
    where
        T: Index + ethercrab_wire::EtherCrabWireWrite;
}

impl<SubDeviceType> WriteValueIndex for SubDeviceRef<'_, SubDeviceType>
where
    SubDeviceType: std::ops::Deref<Target = SubDevice>,
{
    async fn sdo_write_value_index<T>(&mut self, value: T) -> Result<(), ethercrab::error::Error>
    where
        T: Index + ethercrab_wire::EtherCrabWireWrite,
    {
        self.sdo_write(T::INDEX, T::SUBINDEX, value).await
    }
}

So then I could use it like the following:

        device.sdo_write_value_index(filter).await?;

Is this something you would like added to ethercrab?

If so I can make a PR, and the namings can be discussed.

[jamwaffles]

Better SDO init is something I would very much like in EtherCrab. The current API is a low level, simple implementation to just get a feel for how everything needs to work. That said, I'm not sure what a higher level design should look like right now. I need to begin implementing a better DS402 interface soon and that work will either include or guide a higher level SDO init API, so while it's a bit painful in its current state, please save your efforts in writing a new API method :)

[jbbjarnason]

For future references, SDO read/write is not something is only needed in initialization, for example for configuring motor parameters live during commissioning aswell as calibrating weigher cells (Beckhoff EL3356)

[jamwaffles]

Good point, thanks! I'll bear that in mind when I get round to working on this

[jbbjarnason]

For more future references, I made a simple trait to make type safe configuration of sdo parameters, using the above trait.

#[macro_export]
macro_rules! define_value_type_internal {
    (
        $name:ident,
        $type:ty,
        $bytes:expr,
        $bits:expr,
        $default:expr,
        $index:expr,
        $subindex:expr
    ) => {
        #[derive(
            Debug,
            ethercrab_wire::EtherCrabWireWrite,
            serde::Serialize,
            serde::Deserialize,
            schemars::JsonSchema,
            Copy,
            Clone,
        )]
        #[wire(bytes = $bytes)]
        #[serde(transparent)]
        struct $name {
            #[wire(bits = $bits)]
            value: $type,
        }

        impl Default for $name {
            fn default() -> Self {
                Self { value: $default }
            }
        }

        impl Index for $name {
            const INDEX: u16 = $index;
            const SUBINDEX: u8 = $subindex;
        }
    };
}
#[macro_export]
macro_rules! define_value_type {
    ($name:ident, u16, $default:expr, $index:expr, $subindex:expr) => {
        $crate::define_value_type_internal!($name, u16, 2, 16, $default, $index, $subindex);
    };
    ($name:ident, u32, $default:expr, $index:expr, $subindex:expr) => {
        $crate::define_value_type_internal!($name, u32, 4, 32, $default, $index, $subindex);
    };
    ($name:ident, f32, $default:expr, $index:expr, $subindex:expr) => {
        $crate::define_value_type_internal!($name, f32, 4, 32, $default, $index, $subindex);
    };
}

Example of usage,

// Struct name, value type, default value, index, subindex
define_value_type!(NominalValue, f32, 1.0, 0x8000, 0x23);
define_value_type!(Gravity, f32, 9.806650, 0x8000, 0x26);
define_value_type!(ZeroBalance, f32, 0.0, 0x8000, 0x25);

#[derive(Serialize, Deserialize, JsonSchema, Debug)]
struct Config {
    filter: Filter,
    nominal_load: f32,     // kg
    calibration_load: f32, // kg
    #[schemars(
        description = "Nominal characteristic value of the sensor element, Set to 1 if you wan't raw value from load cell. mV/V"
    )]
    nominal_value: Option<NominalValue>,
    #[schemars(
        description = "Gravity of earth, default: None is 9.80665 m/s^2. Set to 1 if you want raw value from load cell"
    )]
    gravity: Option<Gravity>,
    #[schemars(
        description = "Zero balance of the sensor element. mV/V. Set to 0 if you want raw value from load cell"
    )]
    zero_balance: Option<ZeroBalance>,
}

async fn setup<'maindevice, 'group>(
    &mut self,
    device: &mut SubDeviceRef<'maindevice, AtomicRefMut<'group, SubDevice>>,
) -> Result<(), Box<dyn Error>> {
        if let Some(nominal_value) = self.config.read().nominal_value {
            device.sdo_write_value_index(nominal_value).await?;
        }
        if let Some(gravity) = self.config.read().gravity {
            device.sdo_write_value_index(gravity).await?;
        }
        if let Some(zero_balance) = self.config.read().zero_balance {
            device.sdo_write_value_index(zero_balance).await?;
        }
}

Not related to the subject but I am really sad that metaprogramming features of rust are so far from cpp.