Update esp8266-weather-station-color.ino #150
Conversation
- Allow user to force a screen re-calibration - Break up the screen into 4 sections for navigation
|
I just tested this myself and I still like your idea. Thanks! For whatever reason my test device here has got top/bottom mixed up for the touch screen. Don't understand this yet as the whole point of the calibration procedure is to take care of this. Sigh... I'll leave review comments right with the code. |
esp8266-weather-station-color.ino
Outdated
| * - Right forward one page | ||
| * - Bottom jump to page 0 | ||
| */ | ||
| const static uint8_t top = gfx.getHeight() / 4; |
There was a problem hiding this comment.
Kind-of unrelated to this PR but...
Is it worth the effort fetching width & height dynamically? TouchControllerWS.cpp has 240/320 embedded in the code 🙈 Maybe extract that into settings.h, WDYT?
There was a problem hiding this comment.
I don't think that hard coding the screen size is good. The screen is defiantly something that should be runtime calibrated if it is a fixed size why would touch calibration be needed?
esp8266-weather-station-color.ino
Outdated
| if (!asleep) { // no need to change screens; | ||
| if (p.y < 80) { | ||
| if (!asleep) { // no need to update or change screens; | ||
| if (p.y <= top) Serial.print(" TOP "); |
There was a problem hiding this comment.
Since these are not if-else-if you might see "Bottom Right" on the console even though no action is associated with that specific section (only "Bottom" is relevant in that context). That made me wonder...
Ideally
- all of that logic should go into a "ScreenSection"-like class or function
- the evaluation of which section was clicked should accept a
TS_Pointand return a section enum: TOP, BOTTOM, MIDDLE_LEFT, MIDDLE_RIGHT
There was a problem hiding this comment.
- That is by design for the debugging output the extra space in the string gives output like " Top Left " or " Right Bottom " the screen selection part is using the nested if then else.
- Agree it would be better to do the screen selection and draw screen in a function call outside of loop().
- using enum would allow using case instead of if/then/else.
I'll work on making these changes, thanks.
There was a problem hiding this comment.
Done. Any thing else I need to do?
Moved touch screen point selection to new changeScreen function. Commented out confusing screen selection debug output. changed unsigned 16 bit member variable screen to unsigned 8 bit.
|
Moved touch screen point selection to new changeScreen function. |
Documenting screen changes
Corrected path to image
|
marcelstoer: I see the size I used was for the display object not the touch screen your changes with the fixed size looks good. |
|
Thanks again for the inspiration and the first implementation! https://twitter.com/thingpulse/status/1495371662270906369 |
2 participants
Fixes #148.
<Description of and rationale behind this PR>