Skip to content
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

scaling out #623

Open
stappersg opened this issue Jan 1, 2025 · 7 comments
Open

scaling out #623

stappersg opened this issue Jan 1, 2025 · 7 comments

Comments

@stappersg
Copy link
Contributor

stappersg commented Jan 1, 2025

<preamble>this issue concerns avr-hal on the long run</preamble>

Pattern I think that I see is that the rust avr-hal project is getting more popular plus less involvement from Rahix.

I fully agree with Rahix on Hey, it works for me and I will be the last one blaming Rahix for less involvement.

Thing that I would like to see is that this cool project scales out further.

That I choose issue instead of discussion is because I want to solve the issue of scaling out, not just discuss it.

What are your opinions on this issue?

@innermatrix
Copy link

For context, I came into avr-hal as a software engineer (with 20+ years of non-embedded industry experience) and an embedded hobbyist (with 10+ years experience including designing my own boards with ATtiny 0/1/2-series and other AVR MCUs not typically seen in consumer hobbyist hardware). My goal is to migrate my toolchain from the Arduino IDE ecosystem (including megaTinyCore) to Rust, for all the usual Rust reasons.

There are two main technical things I ran into trying to add 0/1/2-series support to avr-hal.


First, the two MCU crates (attiny-hal, atmega-hal) organize their code by peripheral, so adding a new MCU means making changes in a bunch of different files; those peripheral-specific files are then gated on MCU-specific feature flags. This is frustrating to work with because code for a specific MCU is scattered in multiple places, and therefore it's difficult to find what is currently implemented for a given MCU, difficult to add a new MCU (unless it's almost identical to one of the already supported MCUs), and difficult to compare the implementation of a specific MCU to its datasheet (because datasheets are organized by MCU first, and by peripheral second).

For example, if you look at https://github.com/Rahix/avr-hal/tree/main/mcu/attiny-hal/src, there is no way to quickly tell which peripherals are implemented for a given MCU, and adding a new MCU means adding it to every file in there.

My proposed solution to this is to reorganize the MCU crates into two layers:

I started working on this in my fork; the relevant diff is at main...innermatrix:avr-hal:attiny-hal-additive-features.

The benefits of this approach include: support for a new MCU is self-contained (one MCU-specific files + adding some global bits like a feature flag and a top-level use), which makes it easier for different people to work on different MCUs; each MCU file becomes a de facto description of that MCU, which makes it easier to compare an MCU to its datasheet; etc.

There is a lot of additional restructuring that I can envision doing in the future (for example, it's not clear to me that having the ATtiny vs ATmega separation makes sense in today's world where ATtiny xmega devices, like the 0/1/2-series, really blur that boundary). But my starting point for now is that no matter what other restructuring we have in mind, I feel pretty confident that having MCU crates organized around one-MCU-one-module would both clarify the existing code and make future contributions easier (both for the contributors and for the maintainers).


Second, there is no support for unit testing right now. This makes it easy for someone to break one MCU while fixing another, without noticing.

My proposed solution to this is to make two changes; one easy, one hard. The easy change is to require all doctests to compile. I started working on this and the relevant diff is in innermatrix/avr-hal@attiny-hal-additive-features...innermatrix:avr-hal:attiny-fix-doctests. This alone would substantially increase the confidence of a new contributor that their changes will not catastrophically break existing MCU support.

The hard change is to start running unit tests on actual hardware. (I did consider qemu as an alternative, but its support for GPIO is fairly limited from what I can tell, which means that it's not a particularly useful test environment for IO peripherals.) I am currently researching related prior art, such as probe-rs and intend to formulate a proposal for the community to discuss. I would love for us to ultimately have a self-hosted GH action runner that can automatically execute unit tests on real hardware for each PR, but we are several major steps away from being able to do that.


In summary, I think the following two changes would be essential for scaling out the project:

  • Organize MCU support by MCU first, peripheral second.
  • Increase test coverage, starting with mandatory doctests and gradually heading towards automated unit testing on real hardware.

I think those are important because they are foundational and have long-term impact on scaling out; there are obviously many other improvements we can (and should) make (such as better documentation, API revisions, etc), all of which I would welcome.

And I know I said this before, but it never gets said enough so I will say it again: much gratitude to the current maintainers and contributors, love everything you've done so far!

@tones111
Copy link

tones111 commented Jan 1, 2025

I'd also like to introduce myself as someone that has been wanting to use Rust on AVR boards for a long time but patiently waiting for the support to mature. I echo the words above and would like to praise @Rahix and others that have put a lot of work in over the years to get to this point. The increase in interest (at least in my case) is directly related to the significant efforts this year to improve the LLVM codegen (also huge thanks to @Patryk27, @benshi001, and previous llvm-avr maintainers).

I also have 20+ years of systems programming experience and am trying to return to the embedded space from a hobbyist perspective. My primary interest and motivation to contribute is to enable async Rust for some AVR (arduino uno) boards in some robotics kits. The embassy project appears to be leading in this space and I'd like to help build out AVR support. Toward that end the first missing piece is an embassy-time-driver implementation that requires adding some embedded-hal-async support within avr-hal. From that lense I've started some PRs for cleanup opportunities that I've identified while becoming familiar with the crate organization.

I also share an interest in seeing AVR support continue to mature into upstream rustc (#471). I'd like to help contribute to removing the avr-gcc dependency, but would like some initial direction to get me started in the right direction. If there's any low-hanging fruit or avr-libc functionality that needs porting I'd like to give it a try.

As a new contributor one significant maintenance hurdle is the lack of testing infrastructure. For example in #618 and #621 I'd like some automation to ensure avr-hal code and examples still compile. Compiling each example manually is very time-consuming. Maybe we can take some inspiration from the embassy project's CI script as it appears to compile and run a lot of their code.

I'm also in agreement with @innermatrix re-org proposal above (still need to review #605). A recurring theme in the backlog of issues and open PRs is that there is a need to reduce the maintenance burden and evolve to support newer devices. I'm still learning avr-hals organization but the ideas presented above sound like great steps in that direction.

Thanks again to the AVR-Rust community.

@Patryk27
Copy link
Contributor

Patryk27 commented Jan 1, 2025

Second, there is no support for unit testing right now. This makes it easy for someone to break one MCU while fixing another, without noticing.

There sorta is, https://github.com/Patryk27/avr-tester - I've been recently busy on something totally unrelated, but I'm hoping to implement more features in avr-tester in the near future.

I'd also like to mention that the nearest milestone is getting AVR to tier 2, rust-lang/rust#131651; small steps forward!

@innermatrix
Copy link

@Patryk27 Oooh, good to know! Poking around it, I see that avr-tester uses simavr, which doesn't implement some of the cores that are already present in avr-hal (for example, tiny88), nor does it implement any 0/1/2-series cores. I am particularly concerned about the lack of 0/1/2-series, which have a number of new features that I don't see in simavr at all, like the event dispatch system. Do you have any sense of how painful those would be to add to simavr?

To be clear, I am not at all opposed to testing with simavr — I am mainly trying to get a sense of how I would balance testing in simulation vs testing on real hardware, given their tradeoffs.

@Patryk27
Copy link
Contributor

Patryk27 commented Jan 1, 2025

for example, tiny88

This one does seem to be implemented (https://github.com/buserror/simavr/blob/ae75edec3f4068f3d1a6c30130abef7ef8e83155/simavr/cores/avr/iotn88.h#L1) - I haven't been playing with simavr directly (besides implementing the support in avr-tester), so I'm not sure how much effort it'd take to implement more stuff there, though.

@innermatrix
Copy link

@Patryk27 Those look like declarations derived from upstream AVR sources; wouldn't there need to be something related to tiny88 in https://github.com/buserror/simavr/tree/ae75edec3f4068f3d1a6c30130abef7ef8e83155/simavr/cores ? (Could be I just don't quite understand how this code is structured.)

Anyway, I don't want to derail the main thread for this tangent. simavr has non-zero value, whether tiny88 specifically is supported or not; thanks for bringing it up!

@Patryk27
Copy link
Contributor

Patryk27 commented Jan 2, 2025

wouldn't there need to be something related to tiny88 in [...]

Huh, I'd say so as well! simavr --list-cores doesn't list tiny88, so maybe it's not totally ready.

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

No branches or pull requests

4 participants