Added option for sparkline #7
Conversation
| @@ -0,0 +1,150 @@ | |||
| // Adaped version of ESParklines to sidestep random() issue in FixedPointsArduino library | |||
There was a problem hiding this comment.
I strongly prefer not including forked libraries in the Operame repository, if it can be avoided at all. I've submitted a PR to fix the underlying library instead, at Pharap/FixedPointsArduino#71
| @@ -41,6 +42,8 @@ bool add_units; | |||
| bool wifi_enabled; | |||
| bool mqtt_enabled; | |||
| int max_failures; | |||
| bool sparkline_enable; | |||
There was a problem hiding this comment.
Nitpick: the other variables are called _enabled. For consistency, _enabled would be preferred to _enable.
| @@ -41,6 +42,8 @@ bool add_units; | |||
| bool wifi_enabled; | |||
| bool mqtt_enabled; | |||
| int max_failures; | |||
| bool sparkline_enable; | |||
| int sparkline_bufferlenght; | |||
| @@ -54,6 +57,10 @@ void clear_sprite(int bg = TFT_BLACK) { | |||
| } | |||
| } | |||
|
|
|||
| SparkLine<uint16_t> display_sparkline(766, [&](const uint16_t x0, const uint16_t y0, const uint16_t x1, const uint16_t y1) { | |||
There was a problem hiding this comment.
I'm guessing the 766 was supposed to be sparkline_bufferlength? That variable is declared and assigned but not used elsewhere.
| @@ -68,6 +75,10 @@ void display_big(const String& text, int fg = TFT_WHITE, int bg = TFT_BLACK) { | |||
| sprite.setTextColor(fg, bg); | |||
| sprite.drawString(text, display.width()/2, display.height()/2); | |||
|
|
|||
| if (sparkline_enable) { | |||
| display_sparkline.draw( 10, display.height()/4*3, display.width()-20, display.height()/2-5); | |||
There was a problem hiding this comment.
I'm curious what the -5 is for.
| @@ -128,6 +140,7 @@ void ppm_demo() { | |||
| delay(30); | |||
| } | |||
| display_logo(); | |||
| if (sparkline_enable ) display_sparkline.reset(); | |||
There was a problem hiding this comment.
I'm wondering if having the sparkline in the demo is worth resetting the main history for. OTOH, this makes it consistent with accessing the portal (resetting the microcontroller is the only way out) and does make the demo include the sparkline.
| @@ -337,6 +350,10 @@ void setup() { | |||
| mqtt_template = WiFiSettings.string("operame_mqtt_template", "{} PPM", T.config_mqtt_template); | |||
| WiFiSettings.info(T.config_template_info); | |||
|
|
|||
| WiFiSettings.heading("sparkline"); | |||
| @@ -337,6 +350,10 @@ void setup() { | |||
| mqtt_template = WiFiSettings.string("operame_mqtt_template", "{} PPM", T.config_mqtt_template); | |||
| WiFiSettings.info(T.config_template_info); | |||
|
|
|||
| WiFiSettings.heading("sparkline"); | |||
| sparkline_enable = WiFiSettings.checkbox("operame_sparkline", false, T.config_sparkline); | |||
| sparkline_bufferlenght = WiFiSettings.integer("operame_sparkline_buffer", 0, 16383, 512, T.config_sparkline_buffer); ; | |||
There was a problem hiding this comment.
Maybe the buffer length should not be user configurable, at least not without explaining what it does. If it is configurable, I think it would be slightly more user friendly to let them specify the buffer size in minutes.
Why 16383 and 512?
Added option for a sparkline.