Closed microbit-matt-hillsdon closed 2 years ago
@microbit-matt-hillsdon This all looks good, and I'm happy to approve, unless you wanted to wait for further input on the i2c, uart and spi modules?
@microbit-matt-hillsdon This all looks good, and I'm happy to approve, unless you wanted to wait for further input on the i2c, uart and spi modules?
I have some comments from Carlos re i2c to incorporate but I'll track that separately as realistically most of the value is from the more commonly used APIs. Raised https://github.com/microbit-foundation/micropython-microbit-stubs/issues/34
Thanks for the review.
I kept them very concise.
I didn't worry about them generating output (e.g. using print or display.scroll all over the place).
I didn't give examples where there wasn't anything to add, e.g. for
temperature()
(on the assumption folks are using `from microbit import * as our docs recommend).I have written examples for e.g.
display.read_light_level()
on the basis that having thedisplay.
is useful. I'm not so sure that this was worthwhile and am considering removing them. You encounter them via autocomplete or signature help, in which case you have thedisplay
by definition, or in the V3 Python editor API docs, which could chose to makedisplay.
clearer in some other way. It feels like repetition there at the moment. See the preview: https://review-python-editor-next.microbit.org/examples/I haven't attempted to use return values, with one exception. I've unpacked tuples where a small fixed number of values are returned, e.g.
x, y, z = accelerometer.get_values()
. This seems worthwhile to me as if you miss that then the return value is harder to work with.I'd appreciate input on the i2c, uart and spi modules with a view to making the examples more meaningful if we can (e.g. a real accessory init line).
Outstanding queries/feedback:
Raised #33 which was spotted along the way.