klingtnet / rosc

An OSC library for Rust.
Apache License 2.0
173 stars 25 forks source link

Add some helpful impls to OSC types to make working with them easier #1

Closed eeeeeta closed 7 years ago

eeeeeta commented 7 years ago

This PR adds methods to get a value of a given type from an OscType enum without writing a tedious match, and adds From impls for every OscType variant. Furthermore, From<String> and From<&str> are now implemented for OscMessage, to allow quickly making messages with just an address and no arguments.

klingtnet commented 7 years ago

Thank you very much for this PR 🦀 I'll check it out and let you know if something has to be added/changed. Otherwise, I will merge it. Have a nice sunday!

klingtnet commented 7 years ago

@eeeeeta Can you implement From from for OscType::Time?

UPDATE The macro expects one parameter for variant match but two are required.

klingtnet commented 7 years ago

Manually implementing From for OscType::Time is straightforward and looks like this:


impl From<(u32, u32)> for OscType {
    fn from(time: (u32, u32)) -> Self {
        OscType::Time(time.0, time.1)
    }
}
impl OscType {
    #[allow(dead_code)]
    pub fn time(self) -> Option<(u32, u32)> {
        match self {
            OscType::Time(sec, frac) => Some((sec, frac)),
            _ => None,
        }
    }
}

Can you add this?

eeeeeta commented 7 years ago

Don't we (also) want a std::time::Duration impl as well? I'll implement this when I get back to my computer, afk at the moment.

On 19 Mar 2017 13:01, "Andreas Linz" notifications@github.com wrote:

Manually implementing From for OscType::Time is straightforward and looks like this:

impl From<(u32, u32)> for OscType { fn from(time: (u32, u32)) -> Self { OscType::Time(time.0, time.1) } }impl OscType {

[allow(dead_code)]

pub fn time(self) -> Option<(u32, u32)> {
    match self {
        OscType::Time(sec, frac) => Some((sec, frac)),
        _ => None,
    }
}

}

Can you add this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/klingtnet/rosc/pull/1#issuecomment-287615156, or mute the thread https://github.com/notifications/unsubscribe-auth/AI959gzND-OvkaUTuu7qB11YIzaqd4Gmks5rnSc2gaJpZM4MhgQR .

klingtnet commented 7 years ago

Great Idea! But we may add a unit test for this conversion.

eeeeeta commented 7 years ago

@klingtnet I've added the simple conversion you detailed above; let's just merge this PR for now, and I'll open another one when I get around to working out std::time::Duration conversion and add some tests for that. (It looks to be quite finicky: OSC uses 1/232-second precision, whereas Rust uses nanoseconds, so we'll have to do some awkward conversion maths and add tests to check validity).

klingtnet commented 7 years ago

@eeeeeta Alright, I will merge this.

klingtnet commented 7 years ago

PS: I will push a new release to crates.io when From is implemented for std::time::Duration and maybe some others things were done.