nrf-rs / microbit

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

micro:bit v2 support #27

Closed Liamolucko closed 3 years ago

Liamolucko commented 3 years ago

The micro:bit v2 was released in late 2020, and is incompatible with the v1. It uses the nRF52833, and has various other changes to the implementation.

Should this crate support it? It would have to be some kind of a v2 feature flag, so the user could cross-compile by switching the flag. Or it might be better as a separate crate.

therealprof commented 3 years ago

Thanks for bringing this up. I have considered a bit back and forth but I'm still undecided. Both approaches have their cons ans pros and I don't see either see either side weighing out the other. If we go for the feature variant we'd also have to have to add a mutually exclusive v1 feature flag, otherwise people would end up very confused because of the dual-binary selection feature they're implementing upstream which we won't be able to support.

HTGAzureX1212 commented 3 years ago

I am also looking into support for micro:bit v2. I am about to participate in a competition that will make use of micro:bit v2. Could you possibly get things up and running asap? Thanks.

robyoung commented 3 years ago

I had a go at implementing this last night and I've hit a bit of a hurdle. I am a complete novice at this stuff so I may well have missed something crucial.

The PIN layout has changed significantly (see v1 and v2 schematics). Not just the numbers but there is an additional port on the v2 board, the way the ports are accessed has changed between the nrf51 and nrf52833 hal crates, and on the v2 board related PINs are often split across the two ports. Together I think this means that it will not be possible to keep exactly the same interface between the two versions. As a small example I looked at converting serial_port to be a macro with a similar interface between v1 and v2. However, on v2 RX is on P0.06 and TX is on P1.08 so I need to pass in both ports.

So, what would a difference in interface mean? I have done a bit of an investigation of other crates that use features and as far as I can tell documentation is built for a feature set rather than showing all possible features with a way to filter. I think this will result in effectively only being able to document the v1 or v2 board not both.

I'm now thinking that two separate crates hosted in this repo with a shared common (like how the nrf-rs/nrf-hal crate is organised) crate might be a better way to go.

On the plus side

On the negative side

So, what do you think? Should I go ahead and start v2 as a separate crate in here?

therealprof commented 3 years ago

@robyoung Thanks for your interest in bringing this crate back from a dormant state. I've tried some weeks ago to sink a few hours into porting this over to the new common nrf-hal but some infrastructure around the timers changed significantly so it would currently only be possible with a major effort or by drastically cutting support.

Unfortunately I'm very constrained in time at the moment but I think I might find enough time mentoring someone to bring this back into shape.

If you look at the examples in nrf-hal https://github.com/nrf-rs/nrf-hal/blob/master/examples/twi-ssd1306/Cargo.toml you can see that most of them are actually MCU independent and can be compiled for different MCUs, some also for nrf51 and nrf52. I think this scheme could be adopted for the micro:bit as well, making it possible to use cfg gates and MCU specific types to still allow a common abstraction for both types.

If I had to draw a roadmap, it'd contain these steps:

@robyoung If you're interested I'd be happy grant you maintainer access on this crate and free rein to shape it to taste.

robyoung commented 3 years ago

Thank you for the pointers, that's really helpful. I think I can start looking at the first three steps on that roadmap (as soon as my v1 microbit arrives).

I've had a look at the nrf-hal crates and examples and I'm still struggling on how we'd get to a common interface when the boards are so different. I am hoping that it largely down to my inexperience with the domain and it will click a bit later down the line.

If you're interested I'd be happy grant you maintainer access on this crate and free rein to shape it to taste.

Sure. Although I would still like all my PRs reviewed as I am a total novice in this area.

therealprof commented 3 years ago

I've had a look at the nrf-hal crates and examples and I'm still struggling on how we'd get to a common interface when the boards are so different.

You can do something like (just an example, not verified for correctness):

#[cfg(feature = "microbit-v1")]
type USART_RX = P0_25<gpio::Input<Floating>>;
#[cfg(feature = "microbit-v1")]
type USART_TX = P0_24<gpio::Output<PushPull>>;

#[cfg(feature = "microbit-v2")]
type USART_RX = P0_06<gpio::Input<Floating>>;
#[cfg(feature = "microbit-v2")]
type USART_TX = P0_08<gpio::Output<PushPull>>;

and potentially also add convenience accessor functions to set them up accordingly. So in the application code you just need to pass USART_RX and USART_TX and you'll end up with the correct pins on either hardware, if you chose the right microbit-vX feature that is. ;)

The bigger problem is the different memory.x and different compiler flags but maybe we can simply have a workspace which contains both the generic crate and 2 workspace members for device specific examples and aux support files.

Sure. Although I would still like all my PRs reviewed as I am a total novice in this area.

No worries, not planning to go anywhere. 😅

Liamolucko commented 3 years ago

The bigger problem is the different memory.x and different compiler flags

The memory.x is provided by nrf-hal now, so that bit shouldn't be an issue.

robyoung commented 3 years ago

I think porting src/display/timer.rs over to the latest nrf-hal directly is going to be pretty hard. The interface has been pared back quite a bit and I'm struggling to figure out how to enable the compare interrupt. I'm pretty certain that with the new reduced interface it will require manually poking bits into the CC registers.

I'm now thinking of just handling it with a counter in the MicrobitDisplayTimer itself. Does that sound like a terrible idea?

therealprof commented 3 years ago

I was also struggling the most with that bit. 😅

I'm now thinking of just handling it with a counter in the MicrobitDisplayTimer itself. Does that sound like a terrible idea?

It doesn't sound terrible but I'm not sure whether it's quick and efficient enough to drive the matrix display. Certainly worth a try.

Liamolucko commented 3 years ago

I implemented it a while ago when I was tinkering with this, so you can use it for reference if you want:

pub struct MicrobitDisplayTimer<T: Instance>(pub T);
impl<T: Instance> DisplayTimer for MicrobitDisplayTimer<T> {
    fn initialise_cycle(&mut self, ticks: u16) {
        // Stop the timer.
        self.0
            .as_timer0()
            .tasks_stop
            .write(|w| w.tasks_stop().set_bit());
        // Clear the timer.
        self.0
            .as_timer0()
            .tasks_clear
            .write(|w| w.tasks_clear().set_bit());
        // Set frequency to 1MHz (16MHz / 2^4). This is the frequency used by the official runtime.
        // It's called a prescaler because it's still using the same underlying, 16MHz clock,
        // but only counts every 4th tick (in this case). So, it's just _scaling_ the output.
        self.0
            .as_timer0()
            .prescaler
            .write(|w| unsafe { w.prescaler().bits(4) });
        // Set the timer's first Capture/Compare register to `ticks`, and then enable the corresponding interrupt.
        // Every time the counter is incremented, the timer will check if it equals this value, and if so trigger the interrupt.
        // So this sets up the timer to send an interrupt once its counter reaches `ticks`.
        self.0.as_timer0().cc[0].write(|w| unsafe { w.cc().bits(ticks.into()) });
        self.0.as_timer0().intenset.write(|w| w.compare0().set());
        // Set the timer to clear itself when it reaches `ticks`.
        // If we don't do this, the counter will just continue until it overflows.
        self.0
            .as_timer0()
            .shorts
            .write(|w| w.compare0_clear().enabled());
        // Start the timer.
        self.0
            .as_timer0()
            .tasks_start
            .write(|w| w.tasks_start().set_bit());
    }

    fn enable_secondary(&mut self) {
        self.0
            .as_timer0()
            .intenset
            .write(|w| w.compare1().set());
    }

    fn disable_secondary(&mut self) {
        self.0
            .as_timer0()
            .intenset
            .write(|w| w.compare1().clear_bit());
    }

    fn program_secondary(&mut self, ticks: u16) {
        self.0.as_timer0().cc[1].write(|w| unsafe { w.cc().bits(ticks.into()) });
    }

    fn check_primary(&mut self) -> bool {
        let reg = &self.0.as_timer0().events_compare[0];
        let fired = reg.read().bits() != 0;
        if fired {
            reg.reset();
        }
        fired
    }

    fn check_secondary(&mut self) -> bool {
        let reg = &self.0.as_timer0().events_compare[1];
        let fired = reg.read().bits() != 0;
        if fired {
            reg.reset();
        }
        fired
    }
}
robyoung commented 3 years ago

oh wow @Liamolucko that's amazing! 😍

I had made a start at doing something similar by looking at how the old nrf-51 hal was working. Out of interest, is there a knack to navigating the PAC documentation? I couldn't find a way to navigate to what was available on each writer. Also, do you have the work you did in a branch somewhere? Is it in a state that I could pick up where you left off?

Liamolucko commented 3 years ago

Also, do you have the work you did in a branch somewhere? Is it in a state that I could pick up where you left off?

I wasn't actually working on this crate, I was just messing about with the v2, so no. I've uploaded what I was working on anyway, though.

eldruin commented 3 years ago

Great work with the V2 support @robyoung! I wanted to add that I wrote a driver for the LSM303AGR accelerometer/magnetometer (micro:bit v1.5, v2) and also for the MMA8x5x accelerometer (micro:bit v1.3) so they can be used here.

robyoung commented 3 years ago

Brilliant! That was going to be one of the next things I was going to look at. 😀

eldruin commented 3 years ago

@robyoung if you are interested, there are some examples of using the LSM303AGR accelerometer and magnetometer running on the micro:bit v1.5 here

robyoung commented 3 years ago

Resolved by #44