search

yuri91 / ili9341-rs

A WIP, no_std, generic driver for the ILI9341 (and ILI9340C) TFT LCD display
Apache License 2.0
57 stars 50 forks source link

Do not hardcode display size #21

Closed almusil closed 3 years ago

almusil commented 3 years ago

ili driver family is pretty generic with support of multiple display sizes. By having width and size as parameter this driver can support broader range of ili displays.

almusil commented 3 years ago

@yuri91 Hello, can you please take a look?

jamwaffles commented 3 years ago

The SSD1306 crate uses an enum to define different display sizes in a safe way. Would that be a consideration for the ILI9341 driver or is the resolution completely variable?

almusil commented 3 years ago

I was able to find several drivers and their sizes: ili9341: 240x320 ili9486: 320x480 ili9486l: 320x480 ili9488: 320x480

So it seems that it should be possible to have it via enum. I'll rework this patch, thanks.

yuri91 commented 3 years ago

Hi!

This PR seems good to me.

Just a few considerations:

Let me know what you think

almusil commented 3 years ago

Hi!

This PR seems good to me.

Hi, thank for the review.

Just a few considerations:

* Since the driver will technically not be about only ili9341 anymore, should we change the crate name? (maybe I will open an issue about this)

* This is a breaking change, which is fine. But maybe it could make sense to leave the current size as default? (Especially if we keep the current crate name).

The breaking change with rename in single release sounds fine to me, 0.4.1 on crates will work as before. So moving to 0.5 after the crate is renamed and doc updated is fine as well IMO. Maybe having branch 0.4 that will have 0.4.2 release with note that the crate has moved to different name?

Let me know what you think