a1ien / rusb

A safe Rust wrapper for libusb.
Other
382 stars 78 forks source link

Implemented Display trait for Speed and conversion to float Mbps #170

Closed fpagliughi closed 1 year ago

fpagliughi commented 1 year ago

Implemented a conversion of Speed to float Mbps - so no additional conversion function would be needed by examples and applications.

Also implemented a Display trait for Speed.

a1ien commented 1 year ago

Why you need this function? Also implement Display as Debug not looks like good idea.

fpagliughi commented 1 year ago

Why you need this function?

There are two reasons...

  1. I keep needing to re-implement the get_speed() function from the list_devices.rs example to display the speed to the users. But it's easier to turn the speed into a number and just use a simple println!("{}" speed.as_mbps()); or similar.
  2. We have a number of USB 3.x devices (mostly industrial cameras), and our customers keep plugging them into USB 2.0 ports. So it's convenient to have a single, simple comparison to say:
if dev.speed().as_mbps() < 5000.0 {
    show_error("Camera is plugged into a slow port!");
}

For # 2, since the values are in increasing order for speed, an alternate way would be to derive PartialOrd and Ord for the enum. Then we could compare the values directly:

if dev.speed() < Speed::Super {
    show_error("Camera is plugged into a slow port!");
}

Actually it's probably worth doing both - the Ord and as_mbps() so that you have both easier display capabilities and precise comparisons.

Also implement Display as Debug not looks like good idea.

I usually assume that the Display trait is what you might display to a user and the Debug trait is what you show another programmer (logs, etc). In the case of simple enums, they're often the same. I usually don't remember in my head the specifics for each type I use, to tend to implement both for types I know that I will often present to the user.

But I know this may just be a personal preference of mine, and if you don't like it, I won't argue to keep it.

fpagliughi commented 1 year ago

My other thought was to make the Display trait display the strings as Mbps:

impl fmt::Display for Speed {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        use Speed::*;
        match *self {
            Low => write!(f, "1.5"),
            Full => write!(f, "12.0"),
            High => write!(f, "480.0"),
            Super => write!(f, "5000.0"),
            SuperPlus => write!(f, "10000.0"),
            _ => write!(f, "(unknown)"),
        }
    }
}

Something like that. Maybe add the "Mbps" to each.

This would also be an alternate to reason # 1 above: Make it easier to display the speed to the user without needing to manually re-implement get_speed() in every application.