Use an extension trait to implement commands

[rfuest]

By using an extension trait the write_command calls could be replaced by more readable code. Here is a partial implementation of such a trait:

pub trait InstructionExt {
    fn enter_invert_mode(&mut self) -> Result<(), Error>;
    fn set_pixel_format(&mut self, dpi: PixelFormat, dbi: PixelFormat) -> Result<(), Error>;
}

impl<T: WriteOnlyDataCommand> InstructionExt for T {
    fn enter_invert_mode(&mut self) -> Result<(), Error> {
        write_command(self, Instruction::INVON, &[])
    }

    fn set_pixel_format(&mut self, dpi: PixelFormat, dbi: PixelFormat) -> Result<(), Error> {
        let value = (dpi as u8) << 4 | (dbi as u8);
        write_command(self, Instruction::COLMOD, &[value])
    }
}

pub enum PixelFormat {
    Bpp3 = 0b001,
    Bpp8 = 0b010,
    Bpp12 = 0b011,
    Bpp16 = 0b101,
    Bpp18 = 0b110,
    Bpp24 = 0b111,
}

This would clean up the code in the init methods:

// Before:
write_command(di, Instruction::COLMOD, &[0b0101_0101])?; // 16bit 65k colors
write_command(di, Instruction::INVON, &[])?;

// After:
di.set_pixel_format(PixelFormat::Bpp16, PixelFormat::Bpp16)?;
di.enter_invert_mode()?;

[almindor]

I like the idea in principle. I think we need to "group up" the init a bit more and make the common things nicer. I'm not sure about extending the display interface though. Seems like the wrong target.

What if we extend the Instruction instead so it's something like Instruction::set_pixel_format(di, PixelFormat::Bpp16)

[rfuest]

I'm not sure about extending the display interface though. Seems like the wrong target.

It isn't perfect, but as long as the trait isn't added to the API I think it would be OK. Another solution would be a newtype wrapper around the display interface.

[almindor]

I'm not sure about extending the display interface though. Seems like the wrong target.

It isn't perfect, but as long as the trait isn't added to the API I think it would be OK. Another solution would be a newtype wrapper around the display interface.

Thinking about this a bit more, it seems maybe the DI is actually the perfect place for this. Once we get read interface traits we can even differentiate Builder and Model::init so that things that might needs reads can be added on top.

[rfuest]

I think that a wrapper around the DI would also work good and each function would only need to be defined in one place (not in the trait and in the impl):

pub struct CommandWriter<DI> {
    interface: DI,
};

impl<DI: WriteOnlyDataCommand> CommandWriter<DI> {
    fn write_command(&mut self) -> Result<(), Error> { ... }

    pub fn enter_invert_mode(&mut self) -> Result<(), Error> {
        self.write_command(Instruction::INVON, &[])
    }

    pub fn set_pixel_format(&mut self, dpi: PixelFormat, dbi: PixelFormat) -> Result<(), Error> {
        let value = (dpi as u8) << 4 | (dbi as u8);
        self.write_command(Instruction::COLMOD, &[value])
    }
}

[Dominaezzz]

Is this issue still relevant after https://github.com/almindor/mipidsi/pull/44?

[rfuest]

Is this issue still relevant after #44?

I don't think it is.