-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
List support #22
List support #22
Conversation
Hello, can you take a quick picture of how the lists are displayed and upload it here, I'm currently in a position where I can't test. Thanks |
Sounds good. Might take a day or two. It'll be a video though ;) |
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.
Thanks for supporting 😊
@@ -49,6 +52,9 @@ class MenuItem { | |||
char* text; | |||
char* textOn; | |||
char* textOff; | |||
std::vector<String> items; |
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.
Some microprocessor architectures don't support the std::vector
, whereas pointers are supported everywhere, that's why I used pointers. If you could find a way to use pointers instead of vectors, that'd be great, otherwise, you could just duplicate these files and make it specific for architectures that support std::vector
, this way, users with lower arch are not left out (like me 😁). Using pointers will be the universal solution...
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 use vectors because I am from a Python/JS/AS3 background, not well versed in this language, but I hope you could help 'make it sane'.
MenuItem* menu) { | ||
this->lcd = new LiquidCrystal(rs, en, d0, d1, d2, d3); | ||
this->lcd = new LiquidCrystal(rs, en, d0, d1, d2, d3, backlight, POSITIVE); |
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 have no statistic as to how many users still use the original LiquidCrystal lib by Arduino (I still use it tho), if the number is minimal, I could divert to your proposal. Do you have any stats?
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.
my platformio.ini
contains this entry in lib_deps
: fmalpartida/LiquidCrystal
Is that the 'original LiquidCrystal lib by Arduino'? I use what worked..
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.
This is the one by Arduino arduino-libraries/LiquidCrystal@^1.0.7
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.
Opened ticket #23 for cursor change.
Opened #26 for the |
ps. I think you should release 2.0.0 as it is (it's beta now?), and use this pr as ideas for newer version, e.g. 3.0. I like how this library kept things minimal and it always should. |
I have updated the CONTRIBUTING.md, please update this PR to reflect the guideline. |
this sounds quite hostile hehe. this pr is a sketch or work in progress i was hoping we could discuss. i will review these guidelines |
Since I don't have time to test, let GitHub do the testing, the most important part is including an example project, for simpler PRs, I will just merge if all checks pass, that's what I wanted you to see 😄. |
@thijstriemstra You don't need to do 👆 anymore 😁, those c++11 warnings will not show in the new release 🤞 |
I needed to pick strings and ints from a list so I hacked this together. I'm sure it can be improved so any suggestions are welcome.
Usage:
I also needed to redraw the menu, like this:
The PR also includes a fix/workaround for #21