avr-hal
avr-hal copied to clipboard
Adds parametrized duty for `impl_pwm` macro
32u4 require closer look. Not sure what to do with10 bit counter. Other chips verified with datasheets, at least timer1 on 328p works like a charm with 16 bit counter.
related to #134
As I wrote in the other issue, this goes against what the TimerPwm abstraction is currently trying to archive: It is supposed to provide a unified interface for using any compatble pin for PWM without caring too much about the details.
That is the reason why all timers are configured for a MAX of 255. This way, no matter what pin you're using, the set_duty() method always gets a value from 0 to 255 representing 0 to full duty cycle.
If we were to change this as you propose here, downstream users would need to know what timer they are using for a specific pin and whether this timer is 8 or 16 (or 10) bits wide. I think this is not acceptable for an abstraction which tries to hide exactly those details and allow homogeneously treating all PWM pins the same.
As I mentioned over in #132, the way forwards is a separate abstraction (or even multiple) for the different applications of the timer peripherals. Also ref. #44.
at least timer1 on 328p works like a charm with 16 bit counter.
Are you sure? The timer is still configured for mode 5 (= "Fast PWM, 8-bit") and will wrap at 0xFF:
https://github.com/Rahix/avr-hal/blob/b1aedf824fcb59078f8022d4464b2835ae4bd83a/chips/atmega328p-hal/src/pwm.rs#L95-L97
Thus, calling set_duty() with a value above 255 will just lead to saturating behavior.
@Rahix Code I was using
#![feature(llvm_asm)]
#![no_std]
#![no_main]
#![feature(abi_avr_interrupt)]
use arduino_uno::{
delay_ms, prelude::*,
};
use panic_halt as _;
const MAX_CYCLES: u16 = 40_000; // 50 Hz
// SG 90 servo -90..90 angle mapping to 50..250 byte values
fn duty(value: u8) -> u16 {
20 * value as u16
}
#[arduino_uno::entry]
fn main() -> ! {
let pd = arduino_uno::Peripherals::take().unwrap();
let mut pins = arduino_uno::Pins::new(pd.PORTB, pd.PORTC, pd.PORTD);
let mut serial = arduino_uno::Serial::new(
pd.USART0, pins.d0, pins.d1.into_output(&mut pins.ddr),
9600.into_baudrate(),
);
let mut timer1 = arduino_uno::pwm::Timer1Pwm::new(
pd.TC1, arduino_uno::pwm::Prescaler::Prescale8);
// FIXME: can't export `avr_device::interrrupt::free`
{
let timer = unsafe { &*arduino_uno::pac::TC1::ptr() };
timer.tccr1a.modify(|_, w| w.wgm1().bits(0b10));
timer.tccr1b.modify(|_, w| w.wgm1().bits(0b11));
timer.icr1.modify(|_, w| unsafe { w.bits(MAX_CYCLES) });
}
let mut servo = pins.d9.into_output(&mut pins.ddr).into_pwm(&mut timer1);
servo.set_duty(duty(150));
servo.enable();
ufmt::uwriteln!(&mut serial, "Hello from Arduino!\r").void_unwrap();
loop {
let b: u8 = (serial.read_byte() - '0' as u8) * 100 +
(serial.read_byte() - '0' as u8) * 10 +
(serial.read_byte() - '0' as u8);
servo.set_duty(duty(b));
ufmt::uwriteln!(&mut serial, "duty set to {}\r", b).void_unwrap();
}
}
Moving discussion to appropriate issues.