search

almindor / st7789

Rust library for displays using the ST7735 driver
https://docs.rs/st7735-lcd
MIT License
47 stars 24 forks source link

Simplify interface by not munging data and commands into one #5

Closed therealprof closed 4 years ago

therealprof commented 4 years ago

Signed-off-by: Daniel Egger daniel@eggers-club.de

almindor commented 4 years ago

Not sure if I agree with this. It makes it less readable when it comes to understanding the parametrized commands.

Maybe doing a fn write_cmd(cmd) and fn write_cmd_args(cmd, Option<args>) kind of thing and using them would be better?

Seeing write_command(SOME_COMMAND); followed by a write_data(0b0000_0001) just doesn't make it more readable to me.

The other option could be making each parametrized command specific function call, such as set_madctl with an enum of values or such, but it seems a bit overkill. (think register sets in embedded-pac crates)

therealprof commented 4 years ago

Well, currently it is very inconsistently applied, e.g. in set_address_window you first have the write_command without extra parameter and then two write_word. This change only harmonizes that to be the same everywhere.

The background being that I'm actually trying to make ST7789 fit for non-8bit interfaces. There's also 9-bit SPI and 16-bit parallel supported by the driver chip (as well as other color formats) but at the moment everything is hardcoded for 8bit commands and RGB565 on 8bit interfaces.

therealprof commented 4 years ago

(and in theory it might be possible to bunch 2 8bit SPI transfers into 1 16bit SPI transfer as well)

almindor commented 4 years ago

Well sure, that's an artefact of the original design. I suppose what it should be is a write_command(cmd, params: Option<&[u8]>) making it universal for any sized command with parameters. This should also allow to pipe the bytes as needed.

therealprof commented 4 years ago

That might work. It would be a lot better though to have the commands include the appropriate number of arguments themselves instead of having the arguments dangling around as free 8bit data...

almindor commented 4 years ago

I'm going to check out expanding the instruction into a valued enum so it contains specifics for known finite param sets.

almindor commented 4 years ago

I've decided to take this in after experimenting with Instruction "rustification". It just made things too complex and added loads of boilerplate code, sometimes KISS-ing it is the way to go so I'll just take this in for v0.3.1