Skip to content

Add microbit v2 support #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

robyoung
Copy link
Contributor

@robyoung robyoung commented Apr 29, 2021

Relates to #27

Adds support for the v2 micro:bit within the same crate using feature flags. There are two things I really don't like about this.

1. There are two different APIs but only one can be documented

On the micro:bit v2 the LED column pins are split across the P0 and P1 ports. On the micro:bit v1 there is only one port, P0. This means the APIs to create an LED driver have to be different between the versions. However, because the features are mutually exclusive we can only fully document one. We can add module documentation that covers the others API but that feels a bit weak.

2. The non-blocking LED driver is working, but I don't know how

I do not have a lot of experience with bit twiddling and I found refactoring the display::control hard. I have managed to get it working but there is one little bit that makes absolutely no sense to me.

To do (if we want to continue with this route)

So far I only have the LED examples working.

  • convert the remaining examples
  • update README
  • update docs

@robyoung robyoung force-pushed the add-microbit-v2-support branch 2 times, most recently from 5ee9275 to cd947b1 Compare April 29, 2021 21:17
Add features for microbit-v1 and microbit-v2

Add microbit v2 support to display module
-----------------------------------------

This was hard and I am not super happy with the solution. It is much
more bit twiddling than I am comfortable with.

The issue that made it so difficult is that on the microbit v2 the
column pins are on the P0 and P1 ports so all the row wise writes have
to be split accross the two ports.

Get microbit v2 led examples working
------------------------------------

- Create a new v2 gpio module and conditionally use v1 or v2
- Move NUM_ROWS and NUM_COLS consts into gpio module so they can differ
  between v1 and v2
- Move degrading the LED pins into the gpio module so it can differ
  between v1 and v2

The code for the nonblocking display is a nightmare. The columns are
split between the two GPIO ports so the cols argument to
`display_row_leds` needs to be split between the two ports. This is a
bit painful but doable. The bit that is really confusing me is that
columns 2 and 3 seem to be swapped.
@robyoung robyoung force-pushed the add-microbit-v2-support branch from cd947b1 to db442a3 Compare April 29, 2021 21:28
@robyoung robyoung marked this pull request as ready for review April 29, 2021 21:32
@therealprof
Copy link
Contributor

bors ping

@bors
Copy link
Contributor

bors bot commented Apr 29, 2021

pong

@therealprof
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Apr 29, 2021
@therealprof
Copy link
Contributor

There's another problem which needs addressing. nrf52 will need a different memory.x. It might be time to give up the examples directly and turn this crate into a proper workspace with each program being their own member, similarly to how https://github.com/nrf-rs/nrf-hal/ works.

@robyoung
Copy link
Contributor Author

robyoung commented May 2, 2021

I agree, I think following the nrf-hal approach would be good. It would solve the documentation issue as well. I'm not sure why that would mean we need to give up the examples directory. There seem to be examples in nrf-hal. Are the more difficult to manage? I see why examples are more difficult in nrf-hal 😞

Also, I haven't found that I need a different memory.x for building examples for the nrf52, when would I expect to see issues?

@therealprof
Copy link
Contributor

therealprof commented May 2, 2021

Also, I haven't found that I need a different memory.x for building examples for the nrf52, when would I expect to see issues?

The nRF52833 has twice the amount of flash and 8 times the amount of memory. It is also a more powerful Cortex-M architecture which means that the generated code will be larger and the space required for e.g. the interrupt vectors is bigger. TL;DR it's much more likely you'll run out of flash/RAM with complex examples.

@bors
Copy link
Contributor

bors bot commented May 8, 2021

try

Timed out.

@robyoung
Copy link
Contributor Author

Closing in favour of #44

@robyoung robyoung closed this May 10, 2021
bors bot added a commit that referenced this pull request May 10, 2021
44: Add microbit v2 split crates r=therealprof a=robyoung

Relates to #27 and #41

# Add support for micro:bit v2

This change is a major rewrite. The crate is split into a multi-crate workspace organised in a similar way to [nrf-rs/nrf-hal](https://github.com/nrf-rs/nrf-hal).

- `microbit-common` almost all the code, feature flagged for the different major board versions (V1 and V2).
- `microbit` the V1 board support crate
- `microbit-v2` the V2 board support crate
- `xtask` cargo xtask crate for running CI accross the different versions.
- `examples/*` the examples now need to be crates as they may support both versions

## Code changes to support micro:bit V2

Add features `v1` and `v2`. Originally they were `microbit-v1` and `microbit-v2`, however having the feature name the same as the crate name requires the namespaced-features cargo feature which basically restricts us to nightly.

### Add micro:bit v2 support to display module

This was hard and I am not super happy with the solution. It is much more bit twiddling than I am comfortable with.

The issue that made it so difficult is that on the microbit v2 the column pins are on the P0 and P1 ports so all the row wise writes have to be split across the two ports.

### Get microbit v2 led examples working

- Create a new v2 gpio module and conditionally use v1 or v2
- Move NUM_ROWS and NUM_COLS consts into gpio module so they can differ between v1 and v2
- Move degrading the LED pins into the gpio module so it can differ between v1 and v2

The code for the nonblocking display is a nightmare. The columns are split between the two GPIO ports so the cols argument to
`display_row_leds` needs to be split between the two ports. This is a bit painful but doable. The bit that is really confusing me is that columns 2 and 3 seem to be swapped.

## CI changes

I have mostly copied CI from nrf-rs/nrf-hal with some minor differences. I didn't like having to list all the examples in the CI so it derives the features and targets required for each example from the cargo manifest. Copying nrf-hal also meant getting the doc tests building as well which is nice. For the docs I've favoured complexity in the source for the sake of docs that are clearer.

## Examples changes

The microbit-v2 crate does not yet support serial so I have chosen to favour probe-run and defmt for all the examples except those that are specifically about serial communication (those now just run on the v1 crate). I have not ported over the examples that I could not get working (receivedcf77 and the magnetometer ones). I plan on implementing the magnetometer for both versions in the near future so examples for that will hopefully come back soon.

Co-authored-by: Rob Young <rob@robyoung.digital>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants