nrf-rs / microbit

A Rust crate for BBC micro:bit development
BSD Zero Clause License
276 stars 61 forks source link

Allow noncontiguous pins in display #34

Closed robyoung closed 3 years ago

robyoung commented 3 years ago

In the micro:bit v2 the LED matrix pins are not placed next to each other. This change updates display::control so that it is configured with arrays of pin IDs rather than start and end IDs.

I would ideally like to change the interface of the display module so that it doesn't take a mutable reference to the pac::Periferals::GPIO instance. As far as I can tell taking a mutable reference to GPIO means that the hal can't be used for the other pins as hal::gpio::p0::Parts requires ownership of GPIO. I think it's done like this for performance reasons so that multiple pins can be set high or low at once but it seems like a very high price to pay.

If we could change the interface to be around hal::gpio::Pin instances we could reuse the Pins struct from #33

I am not confident in my assessment here though and have some questions:

mattheww commented 3 years ago

It might be worth looking at how I ended up grouping the pins in rmicrobit for ideas.

https://github.com/mattheww/rmicrobit/blob/master/src/gpio.rs https://github.com/mattheww/rmicrobit/blob/master/src/display/display_port.rs

You're welcome to copy any of the code from there if you think it's useful.

robyoung commented 3 years ago

If that's working how I think that's very neat.

Can I double check I understand it? You require the pins as arguments not to use them but just to confirm they're not used by anything else. And then get make unsafe calls to the GPIO peripheral directly safe in the knowledge that we're not clobbering anything else.

mattheww commented 3 years ago

That's how I remember it, yes. Though it's been a while.

robyoung commented 3 years ago

Nice, I'm going to try and steal that then. Thanks