search

nrf-rs / microbit

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

Moving to embassy-nrf and proposed changes #125

Open lulf opened 5 months ago

lulf commented 5 months ago

I recentliy volunteered to work on moving this crate over to using Embassy HAL (see https://github.com/nrf-rs/nrf-hal/issues/432#issuecomment-1907780104), and having spent some time going through the existing crate and comparing it to my own experiments in microbit-bsp, I'd like to propose some specific changes that I think improves the usability for what I believe is the primary audience: people new to Rust and maybe also embedded.

  1. Replace nrf-hal usage with embassy-nrf. embassy-nrf uses the same PACs, so examples that use the PAC directly can still do that. Embassy-nrf also supports nrf51 now so we can hopefully port most of that too.
  2. Change Board::take() -> Board::new(embassy_nrf::Config), add Board::default(). This is IMHO a simpler way of constructing the board instance and allows for some level of configuring that the embassy HAL allows (for clock setup).
  3. Replace probe-runwith probe-rs.
  4. Remove flip-link? Feels like an additional cognitive thing that doesn't really need to be there.
  5. Convert most non-blocking examples to use async, add some async examples
  6. Remove most core peripherals from cortex-m crate from this crate. Is it worth the cognitive load to exposing these? Seems that if you need access to those you can get that from the underlying HALs and PACs rather than re-exposing it in the BSP. And you generally know what you're doing at that point, but most users I argue does not need to care about it.
  7. Name pins after the names used in the microbit tech documentation (p1, p2, p3, p4 etc) instead of using nRF names. Basically align as much as possible with names used in https://tech.microbit.org/hardware/

There are more things that would be great to sort out, but I would be interested to know what others think of the above fundamentals first.

mattheww commented 5 months ago

What, in your view as a maintainer, should the scope of the microbit crate be?

Is it one Rust board support crate among many, for people who want an nRF5x dev board and happen to have a micro:bit for that purpose?

Is it for people interested in the educational purposes the micro:bit was designed for, and happen to think that Rust is a good language to use?

lulf commented 5 months ago

What, in your view as a maintainer, should the scope of the microbit crate be?

That's a good question, and I would like to hear what others think too!

Is it one Rust board support crate among many, for people who want an nRF5x dev board and happen to have a micro:bit for that purpose?

Is it for people interested in the educational purposes the micro:bit was designed for, and happen to think that Rust is a good language to use?

I think probably closest to the latter, but with a twist that it's for people new to Rust itself as well, not only for introducing microcontroller programming.

I view a board support crate mostly relevant for the use cases where you are unfamiliar with the hardware and it can simplify the experience since it uses terminology close to the board documentation.

Since we have crates.io in Rust, the traditional 'all the code you would need to use the board' is less useful because adding dependencies and new crates is super easy.

And if you are doing a production firmware at a company, you're most likely not going to be using the microbit. Probably you're already using the Nordic devkits for prototyping, and then you have your own PCB etc.

If my vision is in conflict with existing uses of the crate, I'm fine with that and will not 'force my world view' in any way. This is why I raised this proposal now to see what others think.

mattheww commented 5 months ago

Here are my opinions:

But:

Because:

lulf commented 5 months ago

Thanks for sharing!

Here are my opinions:

* It would be good to have one main project where people who want to make a Rust runtime for the micro:bit can collaborate.

* It makes sense for that project to be entirely Rust-based, rather than building on the C++ DAL and CODAL as other languages do.

* Embassy looks like it would be a good base for such a project.

But:

* I think it probably doesn't make sense to try to turn this `microbit` crate into that Embassy-based project.

Because:

* It looks like a big change, and perhaps little of the existing code (at least in the crate itself, rather than the examples) would end up being used.

Since existing code is primarily mapping a Board and other types to a HAL, yes it would most likely be replaced. It's true that existing users of the microbit would probably need to update their code for it to compile. We could keep the type names to make that transition easier, but I think naming pins after the microbit docs is more intuitive considering the official documentation for the board.

* There are people [1](https://github.com/nrf-rs/nrf-hal/issues/432#issuecomment-1912856313) [2](https://github.com/nrf-rs/nrf-hal/pull/431) [3](https://github.com/nrf-rs/microbit/pull/123) who are doing the work needed to keep this crate working more or less as it currently is (in particular, without a "runtime"), at least in the short term; I think it would be better not to get in their way by rewriting everything.

I want to emphasize that there is no runtime that would be added to the microbit crate, it would only be using the embassy HAL (embassy-nrf) for peripherals and pins, which is comparable to nrf-hal, but with both blocking and async support. The runtime (embassy-executor) is something that could be added per-example (i.e. many examples would remain without a runtime, others might have it added depending on what makes most sense).

The nrf-hal maintainer issue remains unsolved though even if the above PRs are merged. My scope is more long term though, and I really think the usability of the microbit could improve with these changes.

mattheww commented 5 months ago

Given a choice between rewriting most of the code in this project, and starting a new project (which could of course copy in any code from this one it found valuable), what are the advantages of the first option?

I think the advantage of the second option is that it makes it easier for people who are currently using this crate to choose when they want to switch to the rewrite.

lulf commented 5 months ago

Given a choice between rewriting most of the code in this project, and starting a new project (which could of course copy in any code from this one it found valuable), what are the advantages of the first option?

This is already the case, https://github.com/lulf/microbit-bsp. I'd like to unify these into one because I think there are advantages to joining the efforts as you pointed out initially, especially since it would remove the dependency on the unmaintained nrf-hal.

I think the advantage of the second option is that it makes it easier for people who are currently using this crate to choose when they want to switch to the rewrite.

Maybe there is a path forward where we can keep the disruptions at a minimum? I'm will create a PR trying switching the HAL only, no other changes than necessary, to show what it would look like in practice and help the discussion further.

vavanade commented 4 months ago

I don't know how on-topic this is but I've been programming micro:bit with MicroPython and since I'm also learning Rust, I wanted to experiment with running Rust on this platform. I have close to zero experience with embedded apart from tinkering with the micro:bit.

For my use-case, @lulf 's crate seems to be way more ergonomic since AFAIK (looking at examples) this one does not implement the more high-level stuff like writing text to the display or abstracting away details of the sensors. My use-case is "idea to working code as soon as possible" and then if I want to dig into details how the abstractions work, I can. For that it seems way more useful to have interfaces for micro:bit similar to the MicroPython one.

I know that there are other crates for stuff like writing text to the display (microbit-text) or crates for different sensors but for me the selling point of micro:bit is that I don't have to spend hours trying to make different sensors and libraries work together, it "just works".

I suspect that mine is a common opinion among people using micro:bit.

mattheww commented 4 months ago

I think "something like micro:bit MicroPython but for Rust" is a good goal, and that's the sort of project I'd like to contribute to.

I agree that that means it would be better to have all the bells and whistles in one project. I think it would also make sense for the project to be "opinionated" (in particular, to pick a "runtime" to use). And quite likely it should take over one or more of the peripherals (eg timers) for its own use, like the C++-based runtime that most other languages use for the micro:bit does.

But it's also reasonable for people to want something that's less opinionated, and is basically just a "Board Support Crate".

In particular, the rust-embedded "discovery book" currently uses this microbit crate for that purpose. As I understand it their aim is more to teach Rust embedded programming in the ways recommended by the Rust Embedded working group, and the micro:bit is a convenient vehicle for this purpose. From that point of view, teaching people how to find their way around the crate ecosystem is part of the point.

That's why I think it might be better to make a clean start rather than changing the direction of this project.

NB the only reason that microbit-text is a separate crate is that this project's maintainers preferred it this way. If we do carry on in this project, I'd be happy to merge it in (just moving it into the workspace would be an easy first step). The same goes for tiny-led-matrix.

BartMassey commented 4 months ago

Thanks much for all your feedback.

Please don't start merging external crate source into the project. Linking them is fine.

I think this crate is a good basis for a more full-featured one, if one can figure out what the API should look like and what functionality would be supported. I am working on a fork of nrf-hal these days to get current PRs in. I'm hoping either to get permission to upgrade that crate or to rebuild this crate on that fork. Then I'll start thinking about what a microbit-tutorial crate would look like.

There's also Embassy, if folks can put up with some async/await in tutorials.

lulf commented 4 months ago

Thanks much for all your feedback.

Please don't start merging external crate source into the project. Linking them is fine.

I'm not sure what/which external crate sources you're referring to here?

I think this crate is a good basis for a more full-featured one, if one can figure out what the API should look like and what functionality would be supported. I am working on a fork of nrf-hal these days to get current PRs in. I'm hoping either to get permission to upgrade that crate or to rebuild this crate on that fork. Then I'll start thinking about what a microbit-tutorial crate would look like.

There's also Embassy, if folks can put up with some async/await in tutorials.

I've made a preview PR porting the v2 board and a few examples to embassy here https://github.com/nrf-rs/microbit/pull/130 . There is no async/await required for embassy.

BartMassey commented 4 months ago

There is no async/await required for embassy.

No for embassy-nrf, but yes for microbit-bsp.

I really need to look more closely at your PR and try it out: give me a day or two I think.

BartMassey commented 4 months ago

For "external crate sources", I was responding to @mattheww's

NB the only reason that microbit-text is a separate crate is that this project's maintainers preferred it this way. If we do carry on in this project, I'd be happy to merge it in (just moving it into the workspace would be an easy first step). The same goes for tiny-led-matrix.

I 'm not sure this is ideal? tiny-led-matrix I think may be used in other projects. I need to look more closely at microbit-text, which maybe has no use outside the microbit crate.

mattheww commented 4 months ago

The other thing visible on crates.io depending on tiny-led-matrix is rmicrobit, which is just the project the nonblocking display code was developed in before I submitted it here. As far as I know nobody has ever used it.

(I think the only thing of any value left in it that hasn't made it to this project or microbit-text is some friendly button handling, which I'd like to find a better home for.)

If there's no appetite for integrating the code any more closely, I think it still might sense to move those crates into this project now that it's a workspace, so that people can modify them without needing me to do it.

BartMassey commented 4 months ago

Ah, I had missed that you wrote these crates. Apologies and thank you!

Yeah, in that case I think it makes sense to move these crates into this workspace and maintain them here. Folks may still want to use them without the BSP crate, so maybe not merge the source into this crate.

I for one would love a cleaner button-handling API for the MB2 BSP. I started to write a thing but wasn't happy with it. Let me know if I can help somehow with yours.

mattheww commented 4 months ago

The buttons stuff is visible here. This was all in the micro:bit v1 days.

If I don't get round to doing anything with it I'd be perfectly happy if anyone else wants to copy it out and work it into shape.