Large match/switch-case vs. fn pointers

help
1

Hello, I have a design question I'm looking for input on. Warning, somewhat long question.

Context
I'm experimenting rewriting a driver for USB mice (not really a driver, more just a configuration tool in user space) in Rust. There are certain features, like:

  • Get polling rate
  • Set polling rate
  • Set LED effects (Single Color, Wave, ...) on different LEDs (some support just 1, others more)
  • etc..

There are a ton of devices, many of which don't support the same feature set. Additionally, for the same feature, many groups of devices have different implementations. Some are minor differences (a single byte change) and some major (needing multiple USB messages sent differently).

The goal is to have an API that can expose supported features for a device, and then also functions implementing those features.

Background
The current driver in C implements a feature with a switch/case handling different device's behaviors (example). Supported features per device are exposed with a switch/case with fallthrough (example) and a build script that manually extracts those names from the source code for use from Python.

My tool is written in Rust and supports the device I have. I want to experiment with supporting more, and I have two approaches I've thought of:

Approach 1
Specify supported features per device at compile time. Implement features with match statements essentially like the existing C driver:

async fn chroma_effect_breath(/*...*/) {
    match self.device.id {
        SOME_MOUSE | OTHER_MOUSE | OTHER_MOUSE2 => {/*...*/},
        SOME_MOUSE2 => {/*...*/},
        _ => {/*...*/}
    }
}
/* Specify supported features per device somewhere - not used internally, kept up to date manually */
const DEVICE_FEATURES = PerfectHashMap {
    /*...*/
}

+ It's simpler (and more readable?)
- Adding a device requires changes to every feature's implementation (even if just a match arm)
- Device implementation is spread out across every feature's code
- Supported features would be specified separately and not tied directly to implementation, which could have bugs letting them be out of sync (ex: feature is specified as supported but forgot to add a match arm for the device to use the correct impl, so it doesn't work)

Approach 2
Specify supported features per device at compile time. Internally, those features also point to their implementations. This would look something like

struct FeatureSet {
    get_dpi: Option</*fn pointer*/>,
    set_dpi: Option</*fn pointer*/>,
    chroma_breath: Option</*fn pointer*/>,
    supported_leds: &[LED],
}

async fn chroma_effect_breath(/*...*/) {
    if let Some(impl) = DEVICE_FEATURES[self.device.id].chroma_breath {
        /* use impl */
    } else {
        /* err - unsupported feature */
    }
}

// Shared by some devices
fn chroma_effect_breath_impl_1 {}

// Shared by some devices
fn chroma_effect_breath_impl_2 {}

// Shared by other devices
fn chroma_effect_breath_impl_default {}

/* global using pfh */
const DEVICE_FEATURES = PerfectHashMap {
    DEATHADDER => FeatureSet{}, // probably a macro to make this too
    /*...*/
}

This could also be traits like Dpi, Chroma, then structs that implement those, and FeatureSet would contain Option's holding dyn Dpi, etc.

+ Adding a new device requires one localized change to the global map (assuming no new USB behavior)
+ Device behavior is visible in one place
+ Supported features API and actual implementation can't become out of sync -> less room for bugs
- More complex, macros, etc.
- Less readable?

Both approaches would have an API for querying supported features and the same API for actually doing things.

Question
Any input on best design decision here? Or even a different design too.

2

personally I would prefer method 2, but with a trait based design instead of function pointers. in my experience, adding support for new device models is more common than adding new feature, once the program reaches certain maturity level. with a trait, even if I want to add new features, I can add an default implementation, and update the models incrementally.

/// default impls corresponds to an empty feature set
/// drivers for specific model can overwrite as needed
trait FeatureSet {
    fn get_dpi(&self) -> Option<DPI> { None }
    fn set_dpi(&mut self, dpi: DPI) -> Result<()> { Err(Error::NotSupported) }
    //...
}

as long you make the trait dyn compatible, it's almost trivial to switch to different devices with a command line option. however, sometimes, only compile time configuration is needed, I might use associated types and/or associated constants, which may give a different API style.

UPDATE:

see also this IDET writeup for other possible solutions to deal with optional features.

1 Like
3

Thanks! This does seem like the cleanest to start with. It does have the limitation of not being able to see what's supported without running the function, but that link you shared seems to mention that too. Thanks for the link, I'll take a look

EDIT: Also I just realized they won't be dyn compatible with async fn's in the trait. I can redesign to not need them to be async fn's, but it will be messier

4

one of the considerations of selecting one approach over the other is configurability. if you want the user to be able to customize the program, what aspect of it do should be configurable? for example:

  • user can select what functionalities to enable or disable:

    • example of build command
    $ cargo build --features dpi,chroma-effect-breath
    • easier to implement if grouped by features,
    #[cfg(feature = "dpi")]
async fn get_dpi() -> Result<DPI> {
    match self.device.id {
        //...
    }
}
#[cfg(feature = "chroma-effect-breath")]
async fn chroma_effect_breath() -> Result<()> {
    match self.device.id {
        //...
    }
}
    • very tedious if grouped by devices, need to repeat #[cfg(...)] for every device

  • user can select what devices to support:

    • build like this:
    $ cargo build --features logitech-foo,razer-bar
    • easier if grouped by devices:
    // drivers/logitech/mod.rs
#[cfg(feature = "logitech-foo")]
mod foo;

// drivers/razer/mod.rs
#[cfg(feature = "razer-bar")]
mod bar;

    // drivers/logitech/foo.rs
struct Foo {
    //...
}
impl FeatureSet for Foo {
    //...
}

// same goes `drivers/razer/bar.rs`

  • both functionalities and devices can be configured:

    • I probably don't want to do this

another reason is, for this kind of programs, I would usually start a prototype with a single (or a few) device for testing. in the early phase, I can spend most of the time to implement features for the single device, and I can play with and evolve the API design very quickly.

once the features are mostly complete, and I'm happy with the API design, it will become somewhat stable, at which point (sometimes called maintenance mode), adding new devices should be relatively streamlined. this also makes it easier for other people to contribute, e.g. they can even use your crate as a library and add support for new devices in a separate crate.

this is a valid concern, but if you use the IDET pattern, at least it's simple enough to write "probe" functions in a device agnostic way, e.g.:

impl dyn FeatureSet {
    fn supports_dpi(&self) -> bool { self.dpi_ext().is_some() }
    fn supports_chroma(&self) -> bool { self.chroma_ext().is_some() }
    fn supported_features(&self) -> Vec<&'static str> {
        let mut features = vec![]
        if self.supports_dpi() {
            features.push("dpi")
        }
        //...
        features
    }
}

unfortunately, the language support for dyn async traits isn't coming anytime soon. meanwhile, if dyn compatibility is desired, you can use type erasure on the Futures, e.g. with the help of the async-trait crate.

1 Like