rust-embedded-community / usbd-midi

MIT License
43 stars 18 forks source link

Roadmap for further development #18

Open sourcebox opened 1 month ago

sourcebox commented 1 month ago

I just want to put some notes here for further discussion, nothing set in stone.

sourcebox commented 1 month ago

I updated the list because https://github.com/rust-embedded-community/usbd-midi/pull/11 has been closed.

If we recommend the user to use an external types crate, we maybe should decide what to do with the ones that are now in. Possible options:

btrepp commented 1 month ago

I was thinking about this, it could be a good use-case for features?. Eg say midi notes, the bare minimum needed is the u7 type. User facing functions could take the type Into or return something that implements From. IIRC u7 is the full domain of a midi note.

This means the library is cut-down, minimal implementation, but the use can opt into a 'midi-types' feature that implements the traits from midi types. Users that wish to use external crates from note management either a) convert to/from in their own code, or b) add in a Pull request for the implementation of a new feature that allows people to 'seemlessly' use the external crate type.

As far as the change, a breaking change where they are dropped seems okay. I imagine this crate isn't used in a heap of things. So the affected parties are small.

Also I guess on types, everything in data is kinda 'if this type existed' we wouldn't need our own. I think the intent was always to have a more usbd-midi-data crate, with just the domain, and no real procedural logic. It just wasn't worth the effort at the time. If the data types in this crate are made into their own crate, other crates can implement the conversions aswell.

sourcebox commented 4 weeks ago

Regarding the SysEx processing, maybe the suggestion from @x37v mentioned in the original PR could be further investigated.

My initial idea here is to add an event() method to UsbMidiEventPacket that returns something like:

pub enum UsbMidiEvent<'a> {
  Regular(&'a [u8]),
  SysexStart(&'a [u8]),
  SysexContinue(&'a [u8]),
  SysexEnd(&'a [u8])
}

Because the enum variants contain &[u8] slices to the raw bytes, this would require to store them inside the packet as [u8; 4] instead of cable_number and message as it's done now. So it would be a breaking change because both fields are public. They could then be replaced by cable_number() and message() methods that create them on-the-fly.

For writing SysEx to the host, maybe a solution also based on iterators is possible. Not sure about that yet.