avr-hal icon indicating copy to clipboard operation
avr-hal copied to clipboard

Adds parametrized duty for `impl_pwm` macro

Open koutoftimer opened this issue 4 years ago • 2 comments

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

koutoftimer avatar Feb 10 '21 05:02 koutoftimer

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 avatar Feb 10 '21 09:02 Rahix

@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.

koutoftimer avatar Feb 10 '21 16:02 koutoftimer