Skip to content

Add a nonblocking LED display driver #14

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 7 commits into from

Conversation

mattheww
Copy link
Contributor

This is the core of the code from https://github.com/mattheww/microbit-blinkenlights, as discussed in #13.

I've left out the font and scrolling support to start with.

Notes:

  • I haven't done anything with the existing 'blocking' led.rs module.

  • The rustdoc is using RFC1946-style links, which aren't stable but do work on docs.rs . I can remove those if you don't want to require nightly rustdoc.

mattheww added 3 commits March 6, 2019 20:16
New modules:
  display
  display::image

Adds a dependency on the tiny-led-matrix crate.
Demonstrates the display module.
@therealprof
Copy link
Contributor

This looks good. Two things I'm not too happy about is funny formatting in some places and the custom timer implementation, it would be great if we could work that into the regular timer code.

@mattheww
Copy link
Contributor Author

Formatting: would it make sense for me to run the whole thing through rustfmt? With the default settings?

Can you say more about where the timer code could move to?

Use the fact that TIMER0, TIMER1, and TIMER2 are all
Deref <Target = nrf51::timer0::RegisterBlock>
to avoid having to implement everything three times.
@mattheww
Copy link
Contributor Author

For what it's worth, I've pushed a much simpler version of microbit_timer.rs, now I've looked a bit harder about what svd2rust is actually generating.

@therealprof
Copy link
Contributor

Formatting: would it make sense for me to run the whole thing through rustfmt? With the default settings?

Yes, please.

@therealprof
Copy link
Contributor

Can you say more about where the timer code could move to?

The timer code lives mostly in the underlying nrf51-hal crate: https://github.com/nrf-rs/nrf51-hal/blob/master/src/timer.rs

There're a number of issues and PRs floating around here and in the nrf51-hal crate which were never finished.

@mattheww
Copy link
Contributor Author

I've run rustfmt.

@mattheww
Copy link
Contributor Author

I've looked at the embedded-hal timers.

The current timer-related traits aren't suitable:

  • they don't provide a way to request an interrupt, only an nb-style wait() method
  • they only support programming a single capture/compare register, but the display code needs two.

Supporting interrupts looks like the more serious limitation.

I could propose something for inclusion in nrf51-hal based on what's currently in microbit_timer.rs (but a bit more general), but it wouldn't be related to any currently-defined embedded-hal interfaces.

(I've read rust-embedded/embedded-hal#57.)

@therealprof
Copy link
Contributor

@mattheww Providing platform specific interfaces on top of the embedded-hal traits is expected and desired. The advantage of putting that code into nrf51-hal is that boards other than the micro:bit can also use that functionality, possibly for completely unrelated hardware.

Of course if there was a way to create a universal abstraction over that would be excellent but that is entirely optional...

@mattheww
Copy link
Contributor Author

mattheww commented Apr 6, 2019

Closing in favour of draft PR #16

@mattheww mattheww closed this Apr 6, 2019
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