Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added serde feature for MidiMessage #30

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

omelia-iliffe
Copy link

For a project I am working on serde support for the MidiMessage is useful.

I have implemented deserialize and serialize for just that enum and its contents but if you were interested in this PR I could expand to impl more types.

@kovaxis
Copy link
Owner

kovaxis commented Jun 15, 2024

I like it. It would be nice to implement the serde traits for at least all of the "live" types (that is, LiveEvent and all of its components).

Additionally, we are not using any serde functionality from version 1.0.198, so I think it's safe to just require serde 1.0.0 to allow maximum flexibility. It would be a good idea to test this though (and the same goes with rayon, I would like to test it with rayon 1.0.0).

As an aside, implementing serde traits for these structs is a little ironic because all this crate is about is implementing serialization and deserialization of the MIDI format 😆

@omelia-iliffe
Copy link
Author

omelia-iliffe commented Jul 3, 2024

Hello, I've implemented Serialize and Deserialize on the LiveEvent enum and its components.
I couple of things I have had difficulty with and so aren't resolved yet.

Deserializing SystemCommon::SysEx and SystemCommon::Undefined are challenging as they both contain &'a [u7] with doesn't implement Deserialize. I will look at ways to add this impl but for now I've just put serde(skip_deserialize) on them.

@omelia-iliffe
Copy link
Author

I impl Deserialize for SystemCommon manually then ran into issues when trying to use serde_json to deserialize it. Not sure of a way to deseralize any borrowed byte array from json. I'm unsure where to go from here.

#[cfg(feature = "serde")]
impl<'de: 'a, 'a> serde::Deserialize<'de> for SystemCommon<'a>{
    fn deserialize<D>(deserializer: D) -> StdResult<Self, D::Error>
        where
            D: serde::Deserializer<'de>,
    {
        #[derive(Debug, serde::Deserialize)]
        enum Internal<'a> {
            SysEx(&'a [u8]),
            MidiTimeCodeQuarterFrame(MtcQuarterFrameMessage, u4),
            SongPosition(u14),
            SongSelect(u7),
            TuneRequest,
            Undefined(u8, &'a [u8])
        }

        let internal = Internal::deserialize(deserializer)?;

        match internal {
            Internal::SysEx(data) => Ok(SystemCommon::SysEx(u7::slice_from_int(data))),
            Internal::MidiTimeCodeQuarterFrame(msg, data) => Ok(SystemCommon::MidiTimeCodeQuarterFrame(msg, data)),
            Internal::SongPosition(pos) => Ok(SystemCommon::SongPosition(pos)),
            Internal::SongSelect(song) => Ok(SystemCommon::SongSelect(song)),
            Internal::TuneRequest => Ok(SystemCommon::TuneRequest),
            Internal::Undefined(status, data) => Ok(SystemCommon::Undefined(status, u7::slice_from_int(data))),
        }
    }
}
    #[test]
    fn test_deserialize_system_common() {
        let sys_ex = serde_json::from_str(r#"{"Common":{"SysEx":[1, 2, 3, 4, 5, 6]}}"#).unwrap();

        assert_eq!(LiveEvent::Common(crate::live::SystemCommon::SysEx(crate::num::u7::slice_from_int(&[1, 2, 3, 4, 5, 6]))), sys_ex);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants