Add atmega-hal examples #524
Conversation
There was a problem hiding this comment.
This is a very good idea, thanks a lot :)
I don't really have any comments regarding the code, just a few minor details:
- Maybe add a README to the example directory which explains what this is about?
- Please also add these examples to the CI, with the following change:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5345c91936..c33d20ab57 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -51,6 +51,10 @@ jobs:
- type: board
name: nano168
examples: true
+ - type: board
+ # Not really a board, but also an examples crate
+ name: atmega2560
+ examples: true
- type: mcu
name: atmega1280
spec: atmega1280
| type I2c = atmega_hal::i2c::I2c<clock::MHz16>; | ||
|
|
||
| #[avr_device::entry] | ||
| fn main() -> ! { | ||
| let dp = atmega_hal::Peripherals::take().unwrap(); | ||
| let pins = atmega_hal::pins!(dp); | ||
|
|
||
| // set up serial interface for text output | ||
| let mut serial = Usart::new( | ||
| dp.USART0, | ||
| pins.pe0, | ||
| pins.pe1.into_output(), | ||
| Baudrate::<clock::MHz16>::new(57600), | ||
| ); |
There was a problem hiding this comment.
Ah, not entirely correct, I do have a comment ;)
You're hard coding the clock-rate in different parts of the code here. This works, but it will make changes very tedious later on. Instead, I would recommend to define a project-wide clock type alias which is then referenced everywhere else. Maybe you can do this in the examples to demonstrate what people should be doing?
In code, it would look like this:
/// The project-global CPU clock speed. This type alias should be defined once
/// in a project and then referenced whereever types need the clock speed.
type CoreClock = atmega_hal::clocl::MHz16;
type I2c = atmega_hal::i2c::I2c<crate::CoreClock>;
// ...
let mut serial = Usart::new(
dp.USART0,
pins.pe0,
pins.pe1.into_output(),
Baudrate::<crate::CoreClock>::new(57600),
);There was a problem hiding this comment.
That's a good point, thanks!
| fn main() -> ! { | ||
| let dp = atmega_hal::Peripherals::take().unwrap(); | ||
| let pins = atmega_hal::pins!(dp); | ||
| let mut delay = Delay::<clock::MHz16>::new(); |
There was a problem hiding this comment.
Just a though: We could use one of the examples to show off a nicer way for dealing with delays: Using the previously mentioned CoreClock, you can define more things:
type CoreClock = atmega_hal::clock::MHz16;
type Delay = atmega_hal::delay::Delay<crate::CoreClock>;
fn delay_ms(ms: u16) {
crate::Delay::new().delay_ms(u32::from(ms))
}
fn delay_us(us: u32) {
crate::Delay::new().delay_us(us)
}There was a problem hiding this comment.
Yes, I saw this one in the Arduino HAL implementation. Will update blink example.
At first I was a bit puzzled about this crate being called
avr-hal, while everything focuses onarduino-hal. As I prefer to use "plain" AVR, I took some time to figure out how to use the HAL and port the examples.Hopefully, this will help people working on non-Arduino boards.