-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
display: Create Interfaces for text displays #42
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
=======================================
- Coverage 97.9% 93.7% -4.2%
=======================================
Files 33 34 +1
Lines 3760 3930 +170
=======================================
+ Hits 3680 3681 +1
- Misses 75 244 +169
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One proposal for the argument interactive -> pauseTime but I don't have a strong opinion.
// routines. This doesn't test brightness or contrast to avoid EEPROM wear | ||
// issues. | ||
func TestTextDisplay(dev display.TextDisplay, interactive bool) []error { | ||
result := make([]error, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var result []error
will save an unnecessary memory allocation
result := make([]error, 0) | ||
var err error | ||
|
||
pauseTime := time.Millisecond |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for not sleep at all when not interactive. These 1ms can add up quickly over time. Making it an argument would probably be nice, then: interfactive := pauseTime != 0
WriteString(text string) (n int, err error) | ||
} | ||
|
||
type Intensity int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the expected valid range for the value? Is it worth standardizing or is it better to leave the range to be device-specific? I don't have an opinion.
This creates a set of interfaces for use by text oriented displays like LCDs. Ultimately, I'm planning on adding multiple backpacks for hd44780, the Sparkfun SerLCD, MatrixOrbital LK204-7T, Adafruit USB LCD backpack (compatible w/ MatrixOrbital), and an SSH1106 OLED.
The arch for the backpacks will be a backpack that accepts a gpio.Group(). Then, I'll have implementations (MCP23008, 8574T, gpioioctl.LineSet, and sets of GPIO pins.