Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Figure out a better way to keep track of the list of classes #9

Open
mvirkkunen opened this issue May 12, 2019 · 1 comment
Open

Figure out a better way to keep track of the list of classes #9

mvirkkunen opened this issue May 12, 2019 · 1 comment
Labels
design Design change or refactor

Comments

@mvirkkunen
Copy link
Collaborator

mvirkkunen commented May 12, 2019

UsbDevice::poll currently has a nasty warning:

/// Note: The list of classes passed in must be the same for every call while the device is
/// configured, or the device may enumerate incorrectly or otherwise misbehave. The easiest way
/// to do this is to call the `poll` method in only one place in your code, as follows:

Things will break in weird and wonderful ways if the user doesn't always pass in the same slice of classes in the same order.

What I would really want is for UsbDevice to hold references to the list of classes. However this is a problem because both UsbDevice and the user would want to have a &mut to each class. Earlier versions used immutable references but this meant all classes had to be written using interior mutability which was nasty.

Possible solutions:

  1. Use RefCells. This means the user has to borrow_mut every time they want to use their serial port or whatever though. Not ideal.
  2. Have UsbDevice own the classes instead. They're mixed types but a tuple might work. Then the user could ask UsbDevice for a temporary &mut to their class to talk to them. If compile time device/descriptor generation is implemented this could possibly be quite ergonomic. However the ability to access multiple classes in parallel might be lost, which is currently possible (reading/writing from multiple endpoints at the same time with DMA could be a thing in the future)
  3. Use something similar to smoltcp where they have a SocketSet type you pass to poll, and instead of the actual sockets you get socket handles you can then request access to via the SocketSet. This is probably even more convoluted to use than the RefCells though. (Example)
  4. Live with it.
@mvirkkunen mvirkkunen changed the title Design: Figure out a better way to keep track of the list of classes Figure out a better way to keep track of the list of classes Feb 6, 2020
@mvirkkunen mvirkkunen added the design Design change or refactor label Feb 6, 2020
@ianrrees
Copy link
Contributor

I like solution 2, because it helps fix a problem in the no-scheduler case:

Currently, the classes reference back to the UsbBus via endpoints, which at least in some cases means the UsbBus implementation needs something like a mutex (ATSAMD example) to prevent multiple "concurrent" accesses to the bus. The mutex is problematic when there's no scheduler, because locking it disables all interrupts for the duration of the lock. A mutex with a try_lock() could be implemented which doesn't block if the mutex is held already, returning an error instead, and which only disables all interrupts for a couple instructions while locking/unlocking. But, if the USB classes and bus were all owned by one struct, then I think we could get rid of the mutex requirement since Rust would ensure there's only one execution context that can access an instance of that struct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design change or refactor
Projects
None yet
Development

No branches or pull requests

2 participants