Error handling for DMA max transfer size set to 0

[lonesometraveler]

SpiDriver with a DMA max transfer size of 0 leads to panic.

let spi_driver =
            SpiDriver::new::<SPI2>(spi, sclk, serial_out, Some(serial_in), Dma::Auto(0))?;

The creation of the driver succeeds. But the panic occurs when the driver tries to process data in chunks by calling chunks or chunks_mut.

thread 'main' panicked at 'assertion failed: `(left != right)`
	 left: `0`,
	 right: `0`: chunks cannot have a size of zero', 

Setting the transfer size to 0 doesn't make sense. But mistakes happen. Wouldn't it be better if it panics earlier in max_tansfer_size with a message?
Maybe something like this?

impl Dma {
    pub const fn max_transfer_size(&self) -> usize {
        let max_transfer_size = match self {
            Dma::Disabled => TRANS_LEN,
            Dma::Channel1(size) | Dma::Channel2(size) | Dma::Auto(size) => *size,
        };
        match max_transfer_size {
            x if x % 4 != 0 => panic!("The max transfer size must be a multiple of 4"),
            x if x == 0 => panic!("The max transfer size must be greater than 0"),
            x if x > 4096 => 4096,
            _ => max_transfer_size,
        }
    }
}

Or assign a default value?

impl Dma {
    pub const fn max_transfer_size(&self) -> usize {
        let max_transfer_size = match self {
            Dma::Disabled => TRANS_LEN,
            Dma::Channel1(size) | Dma::Channel2(size) | Dma::Auto(size) => *size,
        };
        match max_transfer_size {
            x if x % 4 != 0 => panic!("The max transfer size must be a multiple of 4"),
            x if x == 0 => 4096,
            x if x > 4096 => 4096,
            _ => max_transfer_size,
        }
    }
}

[MabezDev]

I believe this should be fixed by #168? Could you take a look and confirm?

[lonesometraveler]

The same panic occurs with #168. My understanding is that #168 makes it possible to configure chunk size when DMA is disabled.

I think the case I report is an overlooked item from #76. Making sure val > 0 was mentioned in a comment. But it looks like it somehow did not get implemented.

[MabezDev]

Ah I understand, apologies for the noise.

I think this could be a good candidate for NonZeroUsize in the dma channel enum. Instead of a default value (which might not be expected), this uses the type system to enforce that the size > 0. If you don't like that suggestion, or find it makes the API too awkward to use then I would go with your first suggestion to panic.

[lonesometraveler]

Thank you. I like NonZeroUsize idea.

pub enum Dma {
    Disabled,
    Channel1(NonZeroUsize),
    Channel2(NonZeroUsize),
    Auto(NonZeroUsize),
}

Then we change max_transfer_size like this:

impl Dma {
    pub const fn max_transfer_size(&self) -> usize {
        let max_transfer_size: usize = match self {
            Dma::Disabled => TRANS_LEN,
-           Dma::Channel1(size) | Dma::Channel2(size) | Dma::Auto(size) => *size,
+           Dma::Channel1(size) | Dma::Channel2(size) | Dma::Auto(size) => (*size).get(),
        };
        if max_transfer_size % 4 != 0 {
            panic!("The max transfer size must be a multiple of 4")
        } else if max_transfer_size > 4096 {
            4096
        } else {
            max_transfer_size
        }
    }
}

Looks good? I will create a PR.

[MabezDev]

Looks good! Please create the PR :)