Open omelia-iliffe opened 7 months ago
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 :laughing:
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.
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);
}
For a project I am working on serde support for the
MidiMessage
is useful.I have implemented
deserialize
andserialize
for just that enum and its contents but if you were interested in this PR I could expand to impl more types.