Patterns for robustness principle

[Schard]

I am currently implementing the Zigbee Cluster Library in Rust.

In the protocol, there are various numeric values, which are either limited in range or represent enum values in a subset of the range of the respective integer type. E.g.:

use crate::units::Mireds;

pub const PREVIOUS: u16 = 0xffff;

/// The startup color temperature to use.
pub enum StartupColorTemperature {
    /// Set the color temperature to the specified value in mireds.
    Value(Mireds),
    /// Use the previous color temperature value.
    Previous,
}

impl From<StartupColorTemperature> for u16 {
    fn from(value: StartupColorTemperature) -> Self {
        match value {
            StartupColorTemperature::Value(mireds) => mireds.into(),
            StartupColorTemperature::Previous => PREVIOUS,
        }
    }
}

impl TryFrom<u16> for StartupColorTemperature {
    type Error = u16;

    fn try_from(value: u16) -> Result<Self, Self::Error> {
        if value == PREVIOUS {
            Ok(Self::Previous)
        } else {
            Mireds::try_from(value).map(Self::Value)
        }
    }
}

Given the robustness principle, I want the user of my library only to be able to provide valid values.

On the other hand, I want to handle invalid values received from third-party devices gracefully.

Now, the above enum is used in a variant of another enum which identifies certain attributes that can be read and set:

/// Color information attribute for the Color Control cluster.
///
/// TODO: Add respective associated data.
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
#[repr(u16)]
#[derive(FromLeStreamTagged)]
pub enum ColorInformationAttribute {
    <SNIP>

    /// The desired startup color temperature in mireds.
    StartUpColorTemperature(MaybeStartupColorTemperature) = 0x4010,
}

As you can see, I did not use the type directly, since the underlying u16 value might not be a valid StartupColorTemperature which can be deserialized from it.

I considered using an Option here, but this would:

1. Make it impossible for me to implement traits for it due to the orphan rule.
2. Allow the user to provide None, which is not what I want.

Hence, I implemented the following wrapper struct as you can see in above variant:

#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, FromLeStream, ToLeStream)]
#[repr(transparent)]
pub struct MaybeStartupColorTemperature(u16);

impl TryFrom<MaybeStartupColorTemperature> for StartupColorTemperature {
    type Error = u16;

    fn try_from(value: MaybeStartupColorTemperature) -> Result<Self, Self::Error> {
        Self::try_from(value.0)
    }
}

impl From<StartupColorTemperature> for MaybeStartupColorTemperature {
    fn from(value: StartupColorTemperature) -> Self {
        Self(value.into())
    }
}

Since the Maybe* type is library-internal, this allows graceful parsing of potentially invalid data received, but forces the user of the library to specify a valid StartupColorTemperature (by e.g. calling .into() when instantiating the variant of ColorInformationAttribute.

My issue is, that I encounter this issue a lot, so I have a lot of these internal wrapper types.

Do you know of a better way to handle this kind of pattern?

Edit: Mireds is:

/// Represents a color temperature in mireds (micro reciprocal degrees).
#[derive(
    Clone, Copy, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd, FromLeStream, ToLeStream,
)]
#[repr(transparent)]
pub struct Mireds(u16);

impl Mireds {
    /// Minimum value for mireds.
    pub const MIN: u16 = 0x0000;
    /// Maximum value for mireds.
    pub const MAX: u16 = 0xffef;
}

impl From<Mireds> for u16 {
    fn from(value: Mireds) -> Self {
        value.0
    }
}

impl TryFrom<u16> for Mireds {
    type Error = u16;

    fn try_from(value: u16) -> Result<Self, Self::Error> {
        if value <= Self::MAX {
            Ok(Self(value))
        } else {
            Err(value)
        }
    }
}

[ZiCog]

If you want values used in your code to be restricted so as to not cause surprises while running then use the usual techniques to ensure that.

If you want to “handle invalid values received from third-party devices gracefully” then you have a paradox. A contradictory requirement.

Seems to me that:

a) You could accept those invalid values from other devices, essentially unknown random numbers, but change them to some valid value before passing them into your code. Resulting in some random but otherwise correct behaviour.

b) Create a new valid value specifically to represent those unknown random numbers, pass those suspicious values into your code, as that special value, along with the rest and behave accordingly.

[simonbuchan]

@Schard

My issue is, that I encounter this issue a lot, so I have a lot of these internal wrapper types.

Do you know of a better way to handle this kind of pattern?

What you've done is roughly the standard from what I've seen. You can simplify the code a bit with macro crates like num_enum but it depends on how much control vs “magic” you're comfortable with.

[Schard]

@simonbuchan Thanks. That at least confirms that I’m not doing something completely insane.

To reduce the amount of boilerplate code, I whipped up this bad boy:

use core::marker::PhantomData;

use le_stream::{FromLeStream, ToLeStream};

/// A wrapper type that allows parsing a source type into a destination type.
#[derive(Clone, Copy, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[repr(transparent)]
pub struct Parsable<Src, Dst> {
    src: Src,
    dst: PhantomData<Dst>,
}

impl<Src, Dst> Parsable<Src, Dst> {
    /// Parse the source value into the destination type.
    pub fn parse(self) -> Result<Dst, Src>
    where
        Dst: TryFrom<Src>,
        Dst::Error: Into<Src>,
    {
        Dst::try_from(self.src).map_err(Into::into)
    }
}

impl<Src, Dst> From<Dst> for Parsable<Src, Dst>
where
    Dst: Into<Src>,
{
    fn from(value: Dst) -> Self {
        Self {
            src: value.into(),
            dst: PhantomData,
        }
    }
}

impl<Src, Dst> FromLeStream for Parsable<Src, Dst>
where
    Src: FromLeStream,
{
    fn from_le_stream<T>(bytes: T) -> Option<Self>
    where
        T: Iterator<Item = u8>,
    {
        Src::from_le_stream(bytes).map(|src| Self {
            src,
            dst: PhantomData,
        })
    }
}

impl<Src, Dst> ToLeStream for Parsable<Src, Dst>
where
    Src: ToLeStream,
{
    type Iter = <Src as ToLeStream>::Iter;

    fn to_le_stream(self) -> Self::Iter {
        self.src.to_le_stream()
    }
}

pub enum ColorInformationAttribute {
    <SNIP>

    /// The desired startup color temperature in mireds.
    StartUpColorTemperature(Parsable<u16, StartupColorTemperature>) = 0x4010,
}

@ZiCog

I might have not been clear enough in my wording. The *Attribute enum can be de-/serialized from/to a little endian byte stream. When deserializing, I want to handle possible invalid values gracefully, which have been sent by other Zigbee devices. When serializing, however, I want to ensure that only valid data is sent, i.e. is in the enum.

[simonbuchan]

Don't forget declarative macros either. Often a bunch of fiddly traits can be replaced with a single trivial macro.

In the same vein, if you don't find an existing crate for deriving those conversion/validation methods that serves your purpose and you have enough of these types (maybe from around 30?) you might want to look into creating your own. It's not as rough as you might think: create a proc-macro crate, export a function with a derive macro attribute, use the syn crate to parse out the information you need and the quote create to enjoy the code you want with dynamic parts.

If you're feeling spicy you could also try facet but that's still early.