canadensis
canadensis copied to clipboard
Too much complexity and too many type arguments
A lot of the Cyphal implementation code in this repository has interfaces that look confusing and are difficult to use correctly.
In particular:
- Clock, Instant, and Duration types wind their way through the entire stack from the application to the driver.
- There are many options for transfer ID trackers, numbers of various things that can be stored, and other settings that lead to very long types like this:
pub type UavcanNode = BasicNode<
CoreNode<
Stm32MicrosecondClock,
CanTransmitter,
CanReceiver,
TransferIdFixedMap<CanTransport, TRANSFER_IDS>,
QueueDriver,
MAX_PUBLISHERS,
MAX_REQUESTERS,
>,
>;
- There are too many error types, and they are too complex. They handle driver errors, subscription-related errors, and out of memory errors from many different layers.
These are some possible changes that would help with this:
- Make Instant and Duration non-generic. They could just be 32-bit numbers of microseconds. That would overflow about every 35 minutes, which probably won't be a problem.
- Is there any good way to make the Clock type parameter not so widespread?
- Use a function pointer instead:
fn() -> Instant(for embedded applications, this would not be compatible with drivers that use objects to restrict access to resources)
- Use a function pointer instead:
- Is there any good way to make the Clock type parameter not so widespread?
- Move the Clock and other type-based settings and constants into some kind of Config type, so the Nodes don't need so many different type parameters
- Remove
MinimalNodeand rename other node types (the node types have confusing names, and I don't know when anyone would actually useMinimalNodeinstead of one of the other two) - Make the error types a little less informative so they can be simpler, possibly by having a single non-generic value for driver errors