The name of the set_interrupt_line_config method is very confusing

[ajxville]

I have setup a new can instance in loopback mode for FDCAN1 on my stm32h723zgt6. It compiles fine. The transmit in the main loop, however, never seems to trigger the rx interrupt to fire.

I'm including a main.rs for a stm32h7xx-hal build as an example below:

#![no_main]
#![no_std]

use core::{

num::{ NonZeroU8, NonZeroU16}

};

use stm32h7xx_hal::{

gpio::Speed,
interrupt,
pac,
prelude::*,
pwr::PowerConfiguration,
rcc::{Ccdr, PllConfigStrategy, rec}

};

use stm32h7xx_hal::{

rcc::{Rcc, RccExt},
stm32::SYSCFG

};

use cortex_m_rt::entry;

use fdcan::{

config::{DataBitTiming, NominalBitTiming},
id::StandardId,
filter::{StandardFilter, StandardFilterSlot},
frame::{FrameFormat, TxFrameHeader},
interrupt::{Interrupts,  InterruptLine},

};

#[entry]
fn main() -> ! {

let mut cortex_peripherals = cortex_m::Peripherals::take().unwrap();
let device_peripherals = pac::Peripherals::take().unwrap();

// Constrain and Freeze power
let pwr = device_peripherals.PWR.constrain();
let pwr_cfg: PowerConfiguration = pwr.ldo().freeze();

// Constrain and Freeze clock
let rcc: Rcc = device_peripherals.RCC.constrain();
let ccdr: Ccdr = setup_clocks(rcc, pwr_cfg, device_peripherals.SYSCFG);

//Setup GPIO
let gpio_d = device_peripherals.GPIOD.split(ccdr.peripheral.GPIOD);
let gpio_can_tx = gpio_d.pd1.into_alternate().speed(Speed::VeryHigh);
let gpio_can_rx = gpio_d.pd0.into_alternate().speed(Speed::VeryHigh);

//Setup CANbus

unsafe {

    cortex_peripherals.NVIC.set_priority(interrupt::FDCAN1_IT0, 1);
    cortex_m::peripheral::NVIC::unmask(interrupt::FDCAN1_IT0);
}

assert_eq!(ccdr.clocks.pll2_q_ck().unwrap().raw(), 32_000_000);

let fdcan_prec = ccdr
    .peripheral
    .FDCAN
    .kernel_clk_mux(rec::FdcanClkSel::Pll2Q);

let mut can = device_peripherals.FDCAN1.fdcan(gpio_can_tx, gpio_can_rx, fdcan_prec);

// Kernel Clock 32MHz, Bit rate: 500kBit/s, Sample Point 87.5%
// Value was calculated with http://www.bittiming.can-wiki.info/
// TODO: use the can_bit_timings crate
let nominal_bit_timing = NominalBitTiming {
    prescaler: NonZeroU16::new(4).unwrap(),
    seg1: NonZeroU8::new(13).unwrap(),
    seg2: NonZeroU8::new(2).unwrap(),
    sync_jump_width: NonZeroU8::new(1).unwrap(),
};

// Kernel Clock 32MHz, Bit rate: 1MBit/s, Sample Point 87.5%
// Value was calculated with http://www.bittiming.can-wiki.info/
// TODO: use the can_bit_timings crate
let _data_bit_timing = DataBitTiming {
    prescaler: NonZeroU8::new(2).unwrap(),
    seg1: NonZeroU8::new(13).unwrap(),
    seg2: NonZeroU8::new(2).unwrap(),
    sync_jump_width: NonZeroU8::new(1).unwrap(),
    transceiver_delay_compensation: true,
};

//Configure timing
can.set_nominal_bit_timing(nominal_bit_timing);

//Configure Filters");
can.set_standard_filter(
    StandardFilterSlot::_0,
    StandardFilter::accept_all_into_fifo0(),
);

can.enable_interrupt_line(InterruptLine::_0, true);
can.set_interrupt_line_config(Interrupts::all());
can.enable_interrupts(Interrupts::all());

let mut can = can.into_external_loopback();

loop {

    let buffer = [
        0xA1, 0xA2, 0xAA, 0xAA, 0xFF, 0xFF, 0xFF, 0xFF, 0x0, 0x0, 0x0, 0x0,
        0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
    ];

    let header = TxFrameHeader {
        len: 8,
        id: StandardId::new(0x1).unwrap().into(),
        frame_format: FrameFormat::Standard,
        bit_rate_switching: false,
        marker: None,
    };

    let result = can.transmit(header, &buffer);
}

}

fn setup_clocks(rcc: Rcc, pwrcfg: PowerConfiguration, dp_sys_cfg: SYSCFG) -> Ccdr{

rcc
.bypass_hse()
.use_hse(24.MHz())
.sys_ck(384.MHz())

.pll1_strategy(PllConfigStrategy::Iterative)
.pll1_p_ck(384.MHz())
.pll1_q_ck(48.MHz())        //USB
.pll1_r_ck(192.MHz())

.pll2_strategy(PllConfigStrategy::Iterative)
//.pll2_p_ck(1_500_000.Hz())
.pll2_p_ck(32.MHz())
.pll2_q_ck(32.MHz())        //FDCAN
.pll2_r_ck(96.MHz())

.freeze(pwrcfg, &dp_sys_cfg)

}

#[panic_handler]
fn panic_handler(_info: &core::panic::PanicInfo) -> ! {

loop {
}

}

#[interrupt]
fn FDCAN1_IT1()
{
let _never_fired = true;
}

#[interrupt]
fn FDCAN1_IT0()
{
let _never_fired = true;
}

[richardeoin]

Hi, thank you for making this bug report! You're not seeing any interrupts because of a very confusing API has lead to them being disabled. I've added some comments to explain

can.enable_interrupt_line(InterruptLine::_0, true); // Enable line 0. Line 1 remains disabled.
can.set_interrupt_line_config(Interrupts::all()); // Set all interrupts to line 1
can.enable_interrupts(Interrupts::all()); // Does nothing, since all interrupt are on line 1 which is disabled.

I think the best way to fix this API is to depreciate the set_interrupt_line_config method. I would instead name it set_interrupt_line_1, and perhaps make a counterpart set_interrupt_line_0 that clears the corresponding bits.