Skip to content
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

Add builder method with_reliable_init() #23

Merged
merged 2 commits into from
Mar 1, 2023
Merged

Conversation

vcrn
Copy link
Contributor

@vcrn vcrn commented Feb 28, 2023

This PR should somewhat fix issue #4, at least make a workaround available.

This PR adds a new builder method, with_reliable_init(), which toggles the screen off and on 3 times, with a user-set delay in between each toggle. I tried creating a workaround that only used a pause, without toggling the screen, but was unable to improve the system's behavior.

I see this as more of a workaround than as a solution, but a workaround that has been very useful for me. Because of that, I suggest we leave issue #4 open.

@mjhouse
Copy link
Owner

mjhouse commented Feb 28, 2023

Also, how does this interact with the with_display(mut self, value: Display)? If you call this before you call with_display, that seems ok (this function will turn the screen on and then with_display will turn it back off if they pass Display::Off), but if you call it after with_display, this function will turn the display on, regardless of whether they passed Display::Off or not.

@vcrn
Copy link
Contributor Author

vcrn commented Mar 1, 2023

Thank you for the feedback! I've fixed both of your points. Regarding whether the example runs: with the correct set up (setting the pins, etc, like the examples in examples/) it does run.

Regarding the method always leaving the LCD::On, you are correct. I tried to be clear about it in the doc, but was unhappy with this behavior. I've added an if-state to check whether it's On or Off, and then toggle it accordingly to leave it in the same state. Do you think it's enough to check whether it's On and then do an else or should an else if be used to check whether it's off? It seems like to only two states possible, I just want to be sure that I'm not missing something.

@mjhouse
Copy link
Owner

mjhouse commented Mar 1, 2023

That should work. It's a little verbose, but I can't think of a way to simplify it off the top of my head. I'll merge it.

@mjhouse mjhouse merged commit 311e122 into mjhouse:master Mar 1, 2023
@vcrn
Copy link
Contributor Author

vcrn commented Mar 2, 2023

Yeah, it's quite unelegant. I'll probably open a PR for something nicer later on, unless the issue is fixed before then without using this builder method.

It would probably be nicer to save the state of the display to a variable, then run the loop that was present in my first commit (without taking into consideration the state of the display), and then set the display's state to that of the variable's, and then returning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants