canadensis icon indicating copy to clipboard operation
canadensis copied to clipboard

Too much complexity and too many type arguments

Open samcrow opened this issue 2 years ago • 1 comments

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)
  • 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 MinimalNode and rename other node types (the node types have confusing names, and I don't know when anyone would actually use MinimalNode instead 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

samcrow avatar Mar 31 '23 04:03 samcrow