embassy icon indicating copy to clipboard operation
embassy copied to clipboard

Allow constructing BxcanInstance outside HAL

Open chemicstry opened this issue 2 years ago • 4 comments

I'm using bxcan traits to implement my own can driver and #842 made them impossible to use outside the HAL. This PR just adds pub methods to construct BxcanInstance.

chemicstry avatar Jul 25 '22 20:07 chemicstry

What're you using this for exactly? you're writing your own CAN driver and need to access the value of REGISTERS, NUM_FILTER_BANKS?

Dirbaio avatar Jul 29 '22 16:07 Dirbaio

So the main reason is that I want to construct my own embassy_stm32::can::Can driver, which also takes IRQs, extra buffers, etc. Inside it I use the bxcan crate to access peripheral like embassy does with Deref, but for it to work I need the bxcan::Instantce and bxcan::FilterOwner to be implemented by embassy-stm32. I can't implement those traits myself, becauase macro tables are private. Moreover, bxcan::FilterOwner implementation has some custom logic for each family, so I rather have it implemented by embassy-stm32 instead of duplicating all conditions.

chemicstry avatar Aug 01 '22 12:08 chemicstry

If it's something widely-applicable it should Ideally be in-tree though.

If it's not widely applicable (specific to a project) then lack of macrotables shouldn't be an issue because the project will be using one particular chip anyway.

Dirbaio avatar Aug 01 '22 12:08 Dirbaio

It's an async CAN driver, however, I don't think I can upstream it into embassy-stm32, because:

  • CAN uses four interrupts (RX0, RX1, TX, SCE) so PeripheralMutex does not work and I had to hack around that with more unsafe than usual. Ideally PeripheralMutex could support multiple IRQs, but that requires some Rust generic and macro gymnastics to implement.
  • I also needed RX and TX loopback timestamping, which pulls in dependency on Instant::now()
  • Async driver requires additional queue to store frames between RX IRQ and when user consumes it

TLDR: I think the driver gets too beefy to suit everyone's needs. Unless it is implemented as separate driver (like BufferedUart).

What if BxcanInstance is moved to low_level module, like other peripherals under unstable-pac feature?

chemicstry avatar Aug 01 '22 12:08 chemicstry