[inky] Add Inky Impression 5.7" and 4" support #55
Conversation
It is a 7 colour ePaper/eInk HAT that comes with 5.7" (600 x 448 pixel) or 4" )640 x 400 pixel). Refactors the existing 3 color Inky and adds additional fields to auto-detect via EEPROM. Tested with 5.7" version.
|
Thanks! Will try to review tomorrow. |
Codecov Report
@@ Coverage Diff @@
## main #55 +/- ##
=======================================
- Coverage 64.9% 62.5% -2.4%
=======================================
Files 60 64 +4
Lines 7066 7337 +271
=======================================
+ Hits 4583 4585 +2
- Misses 2329 2598 +269
Partials 154 154
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
inky/types.go
Outdated
| // Use of this source code is governed under the Apache License, Version 2.0 | ||
| // that can be found in the LICENSE file. | ||
|
|
||
| //go:generate stringer -type=Model,Color,ImpressionColor -output types_string.go |
There was a problem hiding this comment.
You want an empty line after so it's not considered part of the package documentation.
|
Thanks for the review! Will address your comments (probably over the weekend) and ping you once it is ready for another pass. |
|
@maruel I believe this is now ready for another round of your review. Thanks. Also I'm planning to squash the commits into one before merging, please let me know if that is not what you prefer. |
There was a problem hiding this comment.
Looks good, the only blocker is to not pull the copyright into package doc. Please refer to https://go.dev/doc/comment. Thanks!
inky/opts.go
Outdated
| 17: "Black wHAT (SSD1683)", | ||
| 18: "Red wHAT (SSD1683)", | ||
| 19: "Yellow wHAT (SSD1683)", | ||
| displayVariantMap = []string{ |
There was a problem hiding this comment.
Fun fact: you can make the slice an array with using [...] instead of [].
I don't think the () are useful but it's not a big deal.
There was a problem hiding this comment.
Changed it to an array.
The () might be handy to distinguish boards with different IC drivers tbh. I haven't checked but there might be even material differences between them and the existing code may not work for new ones (they are 2 different implementations at https://github.com/pimoroni/inky ATM)
|
I squash and rebase anyway, so you can squash if you want but it doesn't matter with what gets merged in. |
Great, will leave it as it is then. Some folks just merge without squash/rebase and I'm a not big fan of that :) Thanks |
|
Please fix the err shadow. I know it's overly zealous but it's easier to enforce it wholesale than comment on the relevant cases in code review. |
|
Thanks a lot! |
Inky Impression is a 7 color ePaper/eInk HAT that comes as 5.7" (600 x 448 pixel) or 4" (640 x 400 pixel) variants.
This commit also refactors the existing 3 color Inky code a bit and adds additional auto-detection improvements.
Tested with 5.7" version.