nrf-rs / microbit

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

Updated non-blocking display documenation and added Changelog entry #122

Closed videbar closed 10 months ago

videbar commented 11 months ago

Fixes #109

winksaville commented 11 months ago

@videbar as a retired programmer but noob to microbit, I don't feel fully qualified to give a formal review, but this LGTM and can approve this if you'd like.

therealprof commented 10 months ago

Hm, need to check why the CI passes are failing. Seems the doc tests are invalid.

videbar commented 10 months ago

Hm, need to check why the CI passes are failing. Seems the doc tests are invalid.

As far as I can tell, the error seems to be in this portion of blocking.rs:

//! The coordiante system is oriented so the 'bottom' (x,4) row is the edge with the edge
//! connector. That means that
//!
//! ```no_run
//! display.show(
//!    &mut timer,
//!    [
//!        [0, 0, 1, 0, 0],
//!        [0, 1, 1, 1, 0],
//!        [1, 0, 1, 0, 1],
//!        [0, 0, 1, 0, 0],
//!        [0, 0, 1, 0, 0],
//!    ],
//!    1000,
//!);
//! ```

I'm not super familiar with doc tests, but to me it seems like the CI is trying to compile the example above but fails because it can't find display or timer.

videbar commented 10 months ago

Upon further inspection, it seems like the faulty doc test was recently introduced in #119. For some reason, it seems like the CI only checked the changelog in that PR and the error wasn't caught. I think it should be enough to add

//! # use microbit_common as microbit;
//! # use microbit::{
//! #     Board,
//! #     hal,
//! #     display::blocking::Display,
//! # };
//! let board = Board::take().unwrap();
//! let mut timer = hal::Timer::new(board.TIMER0);
//! let mut display = Display::new(board.display_pins);

at the beginning of the example.