Starting to flesh out a uart API #1
Conversation
|
|
||
| /// The corresponding UART instance this pin can be configured to. | ||
| /// - However... What happens when multiple Uarts can be routed to the same pin? | ||
| /// - There's an argument this function shouldn't exist, and it's on the user | ||
| /// to correctly configure the corresponding pins before using the UART driver, | ||
| /// and specify the correct Uart instance themselves. | ||
| pub fn to_uart_instance(self: Pin) !uart.Instance; |
There was a problem hiding this comment.
Imho, "Pin to peripherial conversion" is the wrong way, and we should either go the other way round or just provide a generic purpose function selector or something like that
| // Seems like a useful abstraction instead of limiting via a "uN" integer type, | ||
| // there's usually a very reasonable number of UARTs on any one chip so this | ||
| // enum shouldn't be too tedious to populate by hand. | ||
| pub const Instance = enum { | ||
| uart0, | ||
| uart1, | ||
| uart2, | ||
| }; |
There was a problem hiding this comment.
How could we handle different properties of different uarts like "uart0 and uart2 are of type a, but uart1 is completely different" (LPC1768, or even worse: AVR)
There was a problem hiding this comment.
Hmmm little confused what you mean, like the UART peripheral itself varies by instance (USART vs UART on ST for instance), or you're talking to different devices on each instance?
| pub const Parity = enum { | ||
| none, | ||
| odd, | ||
| even, | ||
| }; |
There was a problem hiding this comment.
all legal values:
- none
- even
- odd
- mark
- space
There was a problem hiding this comment.
Oh yeah, this isn't an exhaustive list, was more just noodling on a "rough" structure for what a peripheral driver looks like
| pub const Configuration = struct { | ||
| baud_rate: u32, | ||
| parity: Parity, | ||
| stop_bit: bool, |
| // Allows specific configurations for something like DMA, etc. | ||
| mode_specific: ?ExtendedConfig, |
There was a problem hiding this comment.
i would just "inline" them. if they are available, it works, otherwise it won't
There was a problem hiding this comment.
I think that makes sense. What I was trying to avoid here was the ST HAL style massive configuration structs that contain any and every possible option and it's up to you to pick through the code to see which fields are used when. A tagged enum lets you be a little more explicit that "I'm providing configuration for this specific mode". Food for thought.
| /// What is yall's opinion on this piece of boilerplate? | ||
| /// Pros: | ||
| /// - blocks users from calling write_blockingly/read_blockingly with an error | ||
| /// if they try to use the HAL without calling init() | ||
| /// Cons: | ||
| /// - There are some edge cases where some users might want to do their own low level config | ||
| /// themselves at register level, and skip calling init() but still use write_blockingly/read_blockingly | ||
| /// - But if this is the case, would they be using the HAL to begin with...? | ||
| initialized: bool, |
There was a problem hiding this comment.
I would not define fields, because it migh the totally reasonable to implement a UART as opaque {} or enum(…) {}
There was a problem hiding this comment.
Hmmmmmm.... This one is interesting... Do we never need fields for any peripheral? That seems like it might be super restrictive if you need to hold some kind of state for a given instance. I'm not necessarily disagreeing I just don't want to code ourselves into a corner.
|
|
||
| /// Should put the UART peripheral into a state where it's ready to call methods that actually | ||
| /// do something (write some bytes, read some bytes, whatever) | ||
| pub fn init(self: *UART, config: Configuration) !void { |
There was a problem hiding this comment.
naming scheme here would be apply, and i'd remove deinit and implement a reset
No description provided.