Serialization of None option in the middle of the command

[matoous]

Hello! Asking here before opening a PR just in case the behavior is intended.

When serializing struct with an optional field, e.g.:

#[derive(Clone, PartialEq, Serialize)]
pub struct WithOption {
    a: u8,
    b: Option<u8>,
    c: u8,
}

let value = WithOption { a: 0, b: None, c: 1 };

let s: String<32> = to_string(&value, "+CMD", SerializeOptions::default()).unwrap();

The result (s) currently is AT+CMD=0,1\r.

The specific command set I am working with (gm02s) expects optional params to instead be represented as an empty, but still delimited field in the command. E.g. for the example above: AT+CMD=0,,1\r.

This seems to be result of:

atat/serde_at/src/ser/mod.rs

Line 325 in 791d3d8

self.written -= 1;

. I am not sure if the intention was to only get rid of the = separator in case the struct has only one field that's optional and is None (as tested by the existing test cases) or whether the above is desired behavior.

If this is a bug, it can be fixed for this specific case by changing the code around the line above to:

fn serialize_none(self) -> Result<Self::Ok> {
    if self.written == self.cmd.len() + self.options.cmd_prefix.len() + 1 {
        self.written -= 1;
    }
    Ok(())
}

which keeps the current behavior for removing = but addresses the case outlined above.

Or maybe even:

fn serialize_none(self) -> Result<Self::Ok> {
    if self.written == self.cmd.len() + self.options.cmd_prefix.len() + 1 && self.options.value_sep {
        self.written -= 1;
    }
    Ok(())
}

But this approach falls apart in case of trailing None, addressing which would require a bit more work.

Please let me know if I am misunderstanding and this is intended behavior, in which case I would ask please: how to best handle this? E.g. if I have optional u8 in the middle of the command parameters, how can I ensure the result will be previous_fields,,following_fields for None.

[matoous]

Thinking about this some more, I think changing the behavior would be quite dangerous. I ended up using:

#[derive(Debug, Clone, Copy, PartialEq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum Nullable<T: AtatLen> {
    /// No value.
    None,
    /// Some value of type `T`.
    Some(T),
}

impl<T: AtatLen> AtatLen for Nullable<T> {
    const LEN: usize = T::LEN;
}

impl<'de, T> Deserialize<'de> for Nullable<T>
where
    T: AtatLen + Deserialize<'de>,
{
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        match T::deserialize(deserializer) {
            Ok(v) => Ok(Nullable::Some(v)),
            Err(_) => Ok(Nullable::None),
        }
    }
}

impl<T> Serialize for Nullable<T>
where
    T: AtatLen + Serialize,
{
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        match self {
            Nullable::None => serializer.serialize_bytes(&[]),
            Nullable::Some(t) => t.serialize(serializer),
        }
    }
}

impl<T: AtatLen> Nullable<T> {
    pub fn from_option(opt: Option<T>) -> Self {
        match opt {
            Some(v) => Nullable::Some(v),
            None => Nullable::None,
        }
    }

    pub fn into_option(self) -> Option<T> {
        match self {
            Nullable::Some(v) => Some(v),
            Nullable::None => None,
        }
    }

    pub fn as_option(&self) -> Option<&T> {
        match self {
            Nullable::Some(v) => Some(v),
            Nullable::None => None,
        }
    }

    pub fn as_option_mut(&mut self) -> Option<&mut T> {
        match self {
            Nullable::Some(v) => Some(v),
            Nullable::None => None,
        }
    }
}

impl<T: AtatLen> From<Option<T>> for Nullable<T> {
    fn from(opt: Option<T>) -> Self {
        Nullable::from_option(opt)
    }
}

impl<T: AtatLen> From<Nullable<T>> for Option<T> {
    fn from(n: Nullable<T>) -> Self {
        n.into_option()
    }
}

Which works for this case. Feel free to close.