Add a couple of to and from string functions

[jbangelo]

While perusing the Rust API guidelines I noticed that it recommended implementing the Display trait for all types that it makes sense for, and several of the signal types already have function to convert to and from strings. I've exposed those functions in this PR and implemented Display for the associated types as well. I've also tried to update the from_str() functions to operate on more typical Rust data instead of low level CStr or CString types.

[jbangelo]

To simplify things I'm just always pushing a null terminator to the end of the string. Does anyone have a better suggestion here?

[notoriaga]

could do something like

    pub fn from_bytes<T: AsRef<[u8]>>(s: T) -> Result<Constellation, InvalidConstellation> {
        let bytes = s.as_ref();
        let bytes = if matches!(bytes.last(), Some(0)) {
            Cow::Borrowed(bytes)
        } else {
            let mut bytes = bytes.to_vec();
            bytes.push(0);
            Cow::Owned(bytes)
        };
        let c_str = ffi::CStr::from_bytes_with_nul(&bytes).unwrap();
        let constellation = unsafe { swiftnav_sys::constellation_string_to_enum(c_str.as_ptr()) };

        Self::from_constellation_t(constellation)
    }

[notoriaga]

And we should probably also probably be using little helper functions to reduce the cost of monomorphization

    /// Attempts to make a [`Constellation`] from bytes
    pub fn from_bytes<T: AsRef<[u8]>>(s: T) -> Result<Constellation, InvalidConstellation> {
        fn inner(bytes: &[u8]) -> Result<Constellation, InvalidConstellation> {
            let bytes = if matches!(bytes.last(), Some(0)) {
                Cow::Borrowed(bytes)
            } else {
                let mut bytes = bytes.to_vec();
                bytes.push(0);
                Cow::Owned(bytes)
            };
            let c_str = ffi::CStr::from_bytes_with_nul(&bytes).unwrap();
            let constellation =
                unsafe { swiftnav_sys::constellation_string_to_enum(c_str.as_ptr()) };
            Constellation::from_constellation_t(constellation)
        }
        inner(s.as_ref())
    }

[notoriaga]

Might also make sense to implement FromStr if we have a method to parse from bytes

[jbangelo]

I think I'll just implement FromStr. The intention for these functions is to be an inverse to the to_str functions and not to necessarily operate on any arbitrary byte array. While the C interface has to blur the lines between strings and byte arrays I'd prefer to keep the Rust interface dealing in valid string types to make their use more clear.

I somehow missed CString::new() on my first pass which seems to deal with the null terminator problem itself, but also raises a different issue of a null character in the middle of the string. I think we can treat that like any other conversion error. For these functions both "BAD STRING" and "BAD\0STRING" should both result in the same error, which is simply a failed conversion.

[notoriaga]

The intention for these functions is to be an inverse to the to_str functions and not to necessarily operate on any arbitrary byte array.

Ahh yeah that makes sense to me

I somehow missed CString::new() on my first pass which seems to deal with the null terminator problem itself, but also raises a different issue of a null character in the middle of the string.

CString::new() would require taking ownership of the bytes to perform the conversion. If we go with CStr we don't have to move/clone the bytes in the case the string already has a null byte. CStr doesn't have the equivalent new function but the check I have above should be pretty similar? I guess we shouldn't be unwraping the result from from_bytes_with_nul as that might occur because of a null byte in the middle of the string

[jbangelo]

It seems like the FFI documentation suggests to use CString for converting Rust strings into C compatible strings and CStr when converting C strings into something Rust can easily use. I think this is because Rust strings will only very rarely have null bytes at the very end, so you'll always need to copy the string contents over and append a null byte to make is C compatible and CString::new() does this automatically for you. The chances of being able to avoid an allocation by using CStr seem small.

[silverjam]

Nothing to add over @notoriaga's comments