Menagerie of changes - Stocks effect, AMOLED support, etc. #626

davepl · 2024-05-13T03:30:48Z

Description

Adds a new Stock effect and the required API, as well as AMOLED S3 support

Contributing requirements

I read the contribution guidelines in CONTRIBUTING.md.
I understand the BlinkenPerBit metric, and maximized it in this PR.
I selected main as the target branch.
All code herein is subjected to the license terms in COPYING.txt.

Amoled

robertlipe

Looks good! Nice use of modern C++ for time. Minor style bits for consistency.

(Oh, and use "changes" in title.)

robertlipe · 2024-05-13T12:31:20Z

include/effects/matrix/PatternStocks.h

+    {
+        // Lambda to split the symbols string by comma, because somehow this is cooler than strtok() was in 1989
+        // and I want to flex, but also because strtok() is not thread-safe and we're using threads here.  So there.
+        // Also, I'm using std::string here because std::getline() is easier to use with std::string than with String.


Fwiw, I have very similar code sitting in a branch for the same reasons. Expect to see something like this in utils some day.

robertlipe · 2024-05-13T12:31:59Z

include/effects/matrix/PatternStocks.h

+
+    // GetAllQuotes
+    //
+    // Retrieves all quotes for the given list of symbols, provided as a comma-separated string


Please end sentences in periods throughout.

robertlipe · 2024-05-13T12:33:49Z

include/effects/matrix/PatternStocks.h

+    system_clock::time_point lastUpdate;    // Time of last update
+    system_clock::time_point nextFetch = system_clock::now();  // Time of last quote pull
+
+    std::map<String, StockData> stockData;  // map of stock symbols to quotes


Bonus points for modern time handling. SO much easier to read than struct timeval.

robertlipe · 2024-05-13T12:36:33Z

include/effects/matrix/PatternStocks.h

+            }
+            else
+            {
+                Serial.printf("Failed to parse JSON: %s\n", error.c_str());


Consider DebugE so it shows in red.

robertlipe · 2024-05-13T12:36:54Z

include/effects/matrix/PatternStocks.h

+        }
+        else
+        {
+            Serial.printf("[HTTP] GET failed, error: %s\n", http.errorToString(httpCode).c_str());


robertlipe · 2024-05-13T12:56:39Z

include/effects/matrix/PatternStocks.h

+        g()->fillScreen(BLACK16);
+        g()->fillRect(0, 0, MATRIX_WIDTH, 9, g()->to16bit(CRGB(0,0,128)));
+
+        // Periodically refecth the stock data from the server


robertlipe · 2024-05-13T12:59:48Z

include/gfxbase.h

    }

    void drawPixel(int16_t x, int16_t y, uint16_t color) override
    {
        if (isValidPixel(x, y))
            leds[XY(x, y)] = from16Bit(color);
-        else


Should these just become asserts as they indicate a programming error?

Actually, they're not always. The error messages were removed because a dependency that can be used to move text across the panel doesn't do boundary checking. With the validity check already ignoring out of bounds pixels, it seemed unnecessary to also create a lot of debugging noise.

robertlipe · 2024-05-13T13:03:00Z

include/globals.h

@@ -1326,7 +1312,7 @@ extern RemoteDebug Debug;           // Let everyone in the project know about it

        #define USE_M5DISPLAY 1                               // enable the M5's LCD screen

-    #elif ESP32FEATHERTFT || PANLEE || LILYGOTDISPLAYS3
+    #elif USE_TFTSPI || ESP32FEATHERTFT || PANLEE || LILYGOTDISPLAYS3


Consider just moving use-tftspi into those three build definitions?

We have a constant tension of the build and the config trying to outsmart each other...

robertlipe · 2024-05-13T13:06:41Z

platformio.ini

-upload_port     =
-monitor_port    =
+upload_port     = 
+monitor_port    = 


No trailing spaces, please. This causes diff thrash.

robertlipe · 2024-05-13T13:07:59Z

platformio.ini

+                 -DLV_CONF_PATH="../../../../include/amoled/lv_conf.h"
+                 -DLV_CONF_INCLUDE_SIMPLE
+                 ${psram_flags.build_flags}
+                 ${unity_double_flags.build_flags}


What does this have to do with Unity?

davepl · 2024-05-13T13:52:50Z

Good feedback! The UNITY flag forces doubles to be the same size on the ESP32 and the x86 so that they can share data. There’s a Unity project that can render video and graphics and send them to the mesmerizer, but it only works if you spec the float size. As to the specifics of size, they are lost to the ages now. Which is to say “I forget”! - Dave

…

On May 13, 2024, at 6:14 AM, Robert Lipe ***@***.***> wrote: @robertlipe approved this pull request. Looks good! Nice use of modern C++ for time. Minor style bits for consistency. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + callback(StockData()); // HTTP request error + } + + http.end(); // Close the connection properly + } + + // GetAllQuotes + // + // Retrieves all quotes for the given list of symbols, provided as a comma-separated string + // Calls getQuote for each symbol in the list, and saves it in the stockData map + + void GetAllQuotes(const String& symbols, StockDataCallback callback = nullptr) + { + // Lambda to split the symbols string by comma, because somehow this is cooler than strtok() was in 1989 + // and I want to flex, but also because strtok() is not thread-safe and we're using threads here. So there. + // Also, I'm using std::string here because std::getline() is easier to use with std::string than with String. Fwiw, I have very similar code sitting in a branch for the same reasons. Expect to see something like this in utils some day. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + callback(StockData()); // Parsing error + } + } + else + { + Serial.printf("[HTTP] GET failed, error: %s\n", http.errorToString(httpCode).c_str()); + if (callback) + callback(StockData()); // HTTP request error + } + + http.end(); // Close the connection properly + } + + // GetAllQuotes + // + // Retrieves all quotes for the given list of symbols, provided as a comma-separated string Please end sentences in periods throughout. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > +class PatternStocks : public LEDStripEffect +{ + AnimatedText textSymbol = AnimatedText("STOCK", CRGB::White, &Apple5x7, 1.0f, MATRIX_WIDTH, 0, 0, 0); + AnimatedText textPrice = AnimatedText("PRICE", CRGB::Grey, &Apple5x7, 1.0f, MATRIX_WIDTH, 8, 0, 8); + AnimatedText textChange = AnimatedText("CHANGE", CRGB::White, &Apple5x7, 1.0f, MATRIX_WIDTH, 16, 0, 16); + AnimatedText textVolume = AnimatedText("VOLUME", CRGB::Grey, &Apple5x7, 1.0f, MATRIX_WIDTH, 24, 0, 24); + +private: + + int iCurrentStock = 0; + + bool isUpdating = false; // Flag to check if update is in progress + system_clock::time_point lastUpdate; // Time of last update + system_clock::time_point nextFetch = system_clock::now(); // Time of last quote pull + + std::map<String, StockData> stockData; // map of stock symbols to quotes Bonus points for modern time handling. SO much easier to read than struct timeval. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + stockData.close = doc["close"].as<float>(); + stockData.volume = doc["volume"].as<float>(); + + for (JsonVariant point : doc["points"].as<JsonArray>()) + { + StockPoint stockPoint; + stockPoint.dt = system_clock::from_time_t(point["dt"].as<time_t>()); + stockPoint.val = point["val"].as<float>(); + stockData.points.push_back(stockPoint); + } + if (callback) + callback(stockData); // Successful retrieval and parsing + } + else + { + Serial.printf("Failed to parse JSON: %s\n", error.c_str()); Consider DebugE so it shows in red. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + stockPoint.val = point["val"].as<float>(); + stockData.points.push_back(stockPoint); + } + if (callback) + callback(stockData); // Successful retrieval and parsing + } + else + { + Serial.printf("Failed to parse JSON: %s\n", error.c_str()); + if (callback) + callback(StockData()); // Parsing error + } + } + else + { + Serial.printf("[HTTP] GET failed, error: %s\n", http.errorToString(httpCode).c_str()); Likewise. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + + for (const String& symbol : symbolList) + { + GetQuote(symbol, [this, callback](const StockData& stockDataReceived) + { + if (!stockDataReceived.symbol.isEmpty()) // Check if the stock data is not empty + this->stockData[stockDataReceived.symbol] = stockDataReceived; + if (callback) + callback(stockDataReceived); // Optionally, call the callback for each symbol's data + }); + } + } + + + // Should this effect show its title. + // The stocks effect does not show a title so it doesn't obscure our text display ? And stock's. Also, inconsistent period use. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + + PatternStocks() : LEDStripEffect(EFFECT_MATRIX_STOCKS, "Stocks") + { + } + + PatternStocks(const JsonObjectConst& jsonObject) : LEDStripEffect(jsonObject) + { + } + + ~PatternStocks() + { + } + + static void FetchQuotesTask(void *pvParameters) + { + auto *instance = static_cast<PatternStocks*>(pvParameters); // Cast the void pointer back to the correct type Musing: why can't auto deduce the right type here? Is the type really a Patternstocks**? In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + int w = MATRIX_WIDTH; + int h = MATRIX_HEIGHT - y; + int n = std::min((uint)MATRIX_WIDTH, currentStock.points.size()); + + if (n > 0) + { + float max = 0.0f; + float min = numeric_limits<float>::max(); + + // We have the high and low data in the stock, but let's not trust it and calculate it ourselves + + for (int i = 0; i < n; i++) + { + max = std::max(max, currentStock.points[i].val); + min = std::min(min, currentStock.points[i].val); + } Is this (min, max) = std::minmax_element(points.begin(), points.end()); That allows simd use and possible unrolling if size is known. In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + g()->drawLine(x0, y0, x1, breakevenY, CRGB::Green); + } + } + } + } + + // Draw + // + // Draws the stock display made up of four animated text flyers (price, symbol, change, volume) + // and the stock history graph + + void Draw() override + { + static uint lastCount = 0; + + //static StockData stockData; Stray? In include/effects/matrix/PatternStocks.h <#626 (comment)>: > + + // Draw + // + // Draws the stock display made up of four animated text flyers (price, symbol, change, volume) + // and the stock history graph + + void Draw() override + { + static uint lastCount = 0; + + //static StockData stockData; + + g()->fillScreen(BLACK16); + g()->fillRect(0, 0, MATRIX_WIDTH, 9, g()->to16bit(CRGB(0,0,128))); + + // Periodically refecth the stock data from the server Typos. In include/gfxbase.h <#626 (comment)>: > } void drawPixel(int16_t x, int16_t y, uint16_t color) override { if (isValidPixel(x, y)) leds[XY(x, y)] = from16Bit(color); - else Should these just become asserts as they indicate a programming error? In include/globals.h <#626 (comment)>: > @@ -1326,7 +1312,7 @@ extern RemoteDebug Debug; // Let everyone in the project know about it #define USE_M5DISPLAY 1 // enable the M5's LCD screen - #elif ESP32FEATHERTFT || PANLEE || LILYGOTDISPLAYS3 + #elif USE_TFTSPI || ESP32FEATHERTFT || PANLEE || LILYGOTDISPLAYS3 Consider just moving use-tftspi into those three build definitions? We have a constant tension of the build and the config trying to outsmart each other... In platformio.ini <#626 (comment)>: > @@ -30,8 +30,8 @@ extra_configs = ; Options that are used (or extended) in all device sections (and hence environments) are defined here [base] -upload_port = -monitor_port = +upload_port = +monitor_port = No trailing spaces, please. This causes diff thrash. In platformio.ini <#626 (comment)>: > +monitor_speed = 115200 +upload_speed = 1500000 +board_build.partitions = config/partitions_custom_noota.csv +build_flags = ${base.build_flags} + -DLILYGO_TDISPLAY_AMOLED_SERIES=1 + -DARDUINO_USB_CDC_ON_BOOT=1 + -DAMOLED_S3=1 + -DUSE_SCREEN=1 + -DTOGGLE_BUTTON_1=21 + -DTOGGLE_BUTTON_2=0 + -DNUM_INFO_PAGES=2 + -DLED_PIN0=2 + -DLV_CONF_PATH="../../../../include/amoled/lv_conf.h" + -DLV_CONF_INCLUDE_SIMPLE + ${psram_flags.build_flags} + ${unity_double_flags.build_flags} What does this have to do with Unity? — Reply to this email directly, view it on GitHub <#626 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCFYOVIQ4OJ4T5LXS3KLZCC4D7AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJSGU3DONBRGA>. You are receiving this because you authored the thread.

davepl · 2024-05-13T16:56:16Z

Please end sentences in periods throughout.

Never! I’m a fast-coding rebel who plays by own rules, fast and loose.

Consider DebugE so it shows in red.

Fixed!

Likewise.

Also fixed.

Also, inconsistent period use.

I’m not going back to prison… they won’t take me alive!

Musing: why can't auto deduce the right type here? Is the type really a Patternstocks**?

Fixed

Is this (min, max) = std::minmax_element(points.begin(), points.end()); That allows simd use and possible unrolling if size is known.

SimD likely doesn’t exist on the ESP32, but I’ve been wrong before.

Stray?

Fixed!

Typos.

Fixed!!

Should these just become asserts as they indicate a programming error?

No, because DrawText calls it with out of bounds indices, and we don’t control DrawText, so we have to allow out of range. It’s the first case I couldn’t avoid.

[base] -upload_port = -monitor_port = +upload_port = +monitor_port = No trailing spaces, please. This causes diff thrash.

Ignore whitespace is your friend, but point taken. Cheers, Dave

robertlipe

Esp32-s3 has simd.

Std:algorithm is almost always clearer to a reader and is usually computationally a win.

davepl · 2024-05-13T20:28:06Z

You’ve said that three times without providing code. Show me the money.

…

On May 13, 2024, at 12:03 PM, Robert Lipe ***@***.***> wrote: @robertlipe commented on this pull request. Esp32-s3 has simd. Std:algorithm is almost always clearer to a reader and is usually computationally a win. — Reply to this email directly, view it on GitHub <#626 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCF2TYGJBPRDMAO5LC5LZCEFB3AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJTGUYTSOBUGE>. You are receiving this because you authored the thread.

Thermotronica · 2024-05-13T23:54:17Z

Sent from my iPhoneMessage ID: ***@***.***>

robertlipe · 2024-05-14T00:21:30Z

Sorry I repeated my second message. I got distracted and your comment broke the threading, so I didn't see it when I got back to the thread so I wasn't sure I'd sent it.. (Though you're somewhat right - the compiler will not generate SIMD in this case. The person that ports the code to a more modern CPU can work that out.) Given what a pain min() and max() are when mixing types (did you remember this when you coded the two casts or did you catch them later upon a compiler error?) it seems self-evident that letting STL handle these things is better ust by counting the lines in min_max1 and min_max2 below. minmax_element() will generate the right returns whether they're floats, doubles, ints, longs, strings, or anything that it knows how to compare intrinsically, and it accepts a comparator if you want to sort by something else, like sorting colors by brightness. std::pair<float, float> min_max1(std::vector<float>&v) { int n = std::min((int)MATRIX_WIDTH, (int)v.size()); float max = 0.0f; float min = std::numeric_limits<float>::max(); if (n > 0) { // We have the high and low data in the stock, but let's not trust it and calculate it ourselves for (int i = 0; i < n; i++) { max = std::max(max, v[i]); min = std::min(min, v[i]); } } return std::pair{min, max}; } #include <algorithm> // returns <float*, float*> auto min_max2(std::vector<float>&v) { return std::minmax_element(begin(v), end(v)); } // Test harness. std::vector<float> fill_vec() { std::vector<float> v(50); //srand(/*(unsigned)*/time(NULL)); // ick. std::generate(v.begin(), v.end(), std::rand); return v; } #include <cstdio> int main() { auto v = fill_vec(); const auto [f1,f2] = min_max1(v); printf("1: %f, %f\n", f1, f2); const auto [f3,f4] = min_max2(v); printf("2: %f, %f\n", *f3, *f4); return 0; } Duly noted that minmax() returns pointers to the two elements in the tuple. Since they might be large (see also: strings) new copies are not allocated. You can unpack with first() and last() if that tickles your fancy. If we wanted to mimic sorting only fhe first MATRIX_WIDTH, we could set the end element to (begin(v) + MATRIX_SIZE). Efficiency was one of my points, and I'll concede that on modern Espressif, this example's a bust. (That's not always the case. Intel's demonstrated an <algorithm> that will sometimes transparently spawn threads to walk large collections.) Hopefully you'll agree that std::algorithm versions of walking over loops are more readable. Now that i know we can apply "fast and loose rules", writing code and reviews should be more fun! On Mon, May 13, 2024 at 3:28 PM David W Plummer ***@***.***> wrote:

…

You’ve said that three times without providing code. Show me the money. > On May 13, 2024, at 12:03 PM, Robert Lipe ***@***.***> wrote: > > > @robertlipe commented on this pull request. > > Esp32-s3 has simd. > > Std:algorithm is almost always clearer to a reader and is usually computationally a win. > > — > Reply to this email directly, view it on GitHub < #626 (review)>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AA4HCF2TYGJBPRDMAO5LC5LZCEFB3AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJTGUYTSOBUGE>. > You are receiving this because you authored the thread. > — Reply to this email directly, view it on GitHub <#626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD36YG4FX6N4ZP3FM7J3ZCEO6XAVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBYG42DEOBYGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

davepl · 2024-05-14T00:31:22Z

It’s more concise and likely more efficient, but no… not more readable to me! But I have an aversion to STL iterators, as much as I love the std algorithms. But without looking it up, what does minmax even return? The min? The max? A tuple of minmax? Now I’ve got to go look it up, whereas the pedantic code was at least clear. Totally a personal thing, but: for (auto p : set) // Yay! for (auto p = set.begin(); p!= set.end(); p++) // Less yay Now if one could just write: auto m = set.max(); // Super Yay! …I’d be happy with that. - Dave

…

On May 13, 2024, at 5:21 PM, Robert Lipe ***@***.***> wrote: auto min_max2(std::vector<float>&v) { return std::minmax_element(begin(v), end(v));

davepl · 2024-05-14T00:44:22Z

This is about as tidy as I can get it, because you get an intereator back, not a value. auto [min_it, max_it] = std::minmax_element(points.begin(), points.end()); // Dereference the iterators to get the actual min and max values int min = *min_it; int max = *max_it; If there was a way to write: auto [min, max] = std::minmax_elements(points); Then I’d be sold! - Dave

…

On May 13, 2024, at 5:21 PM, Robert Lipe ***@***.***> wrote: Sorry I repeated my second message. I got distracted and your comment broke the threading, so I didn't see it when I got back to the thread so I wasn't sure I'd sent it.. (Though you're somewhat right - the compiler will not generate SIMD in this case. The person that ports the code to a more modern CPU can work that out.) Given what a pain min() and max() are when mixing types (did you remember this when you coded the two casts or did you catch them later upon a compiler error?) it seems self-evident that letting STL handle these things is better ust by counting the lines in min_max1 and min_max2 below. minmax_element() will generate the right returns whether they're floats, doubles, ints, longs, strings, or anything that it knows how to compare intrinsically, and it accepts a comparator if you want to sort by something else, like sorting colors by brightness. std::pair<float, float> min_max1(std::vector<float>&v) { int n = std::min((int)MATRIX_WIDTH, (int)v.size()); float max = 0.0f; float min = std::numeric_limits<float>::max(); if (n > 0) { // We have the high and low data in the stock, but let's not trust it and calculate it ourselves for (int i = 0; i < n; i++) { max = std::max(max, v[i]); min = std::min(min, v[i]); } } return std::pair{min, max}; } #include <algorithm> // returns <float*, float*> auto min_max2(std::vector<float>&v) { return std::minmax_element(begin(v), end(v)); } // Test harness. std::vector<float> fill_vec() { std::vector<float> v(50); //srand(/*(unsigned)*/time(NULL)); // ick. std::generate(v.begin(), v.end(), std::rand); return v; } #include <cstdio> int main() { auto v = fill_vec(); const auto [f1,f2] = min_max1(v); printf("1: %f, %f\n", f1, f2); const auto [f3,f4] = min_max2(v); printf("2: %f, %f\n", *f3, *f4); return 0; } Duly noted that minmax() returns pointers to the two elements in the tuple. Since they might be large (see also: strings) new copies are not allocated. You can unpack with first() and last() if that tickles your fancy. If we wanted to mimic sorting only fhe first MATRIX_WIDTH, we could set the end element to (begin(v) + MATRIX_SIZE). Efficiency was one of my points, and I'll concede that on modern Espressif, this example's a bust. (That's not always the case. Intel's demonstrated an <algorithm> that will sometimes transparently spawn threads to walk large collections.) Hopefully you'll agree that std::algorithm versions of walking over loops are more readable. Now that i know we can apply "fast and loose rules", writing code and reviews should be more fun! On Mon, May 13, 2024 at 3:28 PM David W Plummer ***@***.***> wrote: > You’ve said that three times without providing code. Show me the money. > > > On May 13, 2024, at 12:03 PM, Robert Lipe ***@***.***> wrote: > > > > > > @robertlipe commented on this pull request. > > > > Esp32-s3 has simd. > > > > Std:algorithm is almost always clearer to a reader and is usually > computationally a win. > > > > — > > Reply to this email directly, view it on GitHub < > #626 (review)>, > or unsubscribe < > https://github.com/notifications/unsubscribe-auth/AA4HCF2TYGJBPRDMAO5LC5LZCEFB3AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJTGUYTSOBUGE>. > > > You are receiving this because you authored the thread. > > > > — > Reply to this email directly, view it on GitHub > <#626 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACCSD36YG4FX6N4ZP3FM7J3ZCEO6XAVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBYG42DEOBYGM> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4HCF3TSHSVL3PU6A2AGF3ZCFKJ7AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBZGA2DIOBZHE>. You are receiving this because you authored the thread.

robertlipe · 2024-05-14T00:58:06Z

Indeed, there is a requirement to be proficient in the language one professes to program in. Does memmove(a, b, n) move from a to to a? I still look it up and that's been around a lot longer. std::min_element returns the minimum. std::max_element returns the maximum. std::minmax_element returns a tuple with pointers to the first and last, as I described and for the reason I described. In RISC V or MIPS, for anything that fits in a register, they go in %R0 and %R1. Since it's a personal thing, I can see that nobody's ever going to drag you toward modern C++, so I'll surrender. Iterators weren't awesome, but there are data collections that aren't contiguous arrays and sometimes p->begin(); p->end(); p = p->next() is just what it takes. The nicer syntax (actually, the nicer nicer syntax that doesn't make a copy like yours does) for (const auto& p : set) is indeed nicer still. Sometimes new tricks are cool to learn. The denser syntax leaves less ambiguity what the programmer expressed to the compiler - whether that is what they MEANT or not, remains a problem, of course. :-) I'm happy to use library functions because someone else has debugged them and we know at an instant what they're expected to do instead of sussing out that your loop looks at the first PIXELS width of the array for max and min. I'm done with this one. On Mon, May 13, 2024 at 7:31 PM David W Plummer ***@***.***> wrote:

…

It’s more concise and likely more efficient, but no… not more readable to me! But I have an aversion to STL iterators, as much as I love the std algorithms. But without looking it up, what does minmax even return? The min? The max? A tuple of minmax? Now I’ve got to go look it up, whereas the pedantic code was at least clear. Totally a personal thing, but: for (auto p : set) // Yay! for (auto p = set.begin(); p!= set.end(); p++) // Less yay Now if one could just write: auto m = set.max(); // Super Yay! …I’d be happy with that. - Dave > On May 13, 2024, at 5:21 PM, Robert Lipe ***@***.***> wrote: > > auto min_max2(std::vector<float>&v) { > return std::minmax_element(begin(v), end(v)); — Reply to this email directly, view it on GitHub <#626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCSD36Z4Q3LHJ4Z7HSXCXTZCFLO7AVCNFSM6AAAAABHTMRH76VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBZGA2TGOBWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

rbergen · 2024-05-14T14:44:18Z

I've just pushed an update to this PR, which includes the following changes:

I made the stock data fetch thread a NetworkReader entry, saving us a thread
I trigger a stock data fetch on Init(), to have stock data ready before the stock effect is shown - unless it's the first up, which I don't think we can really address?
I made the stock server and stock symbol list stock effect settings. They're accessible in the UI via the cogwheel on the Stocks effect. Notes about this one:
- I was able to get the HTTPClient to work with the port number added to the location string
- If the list of ticker symbols is changed, an immediate reload of stock data is triggered
- I removed the defaults for server and ticker symbols from secrets(.example).h, as we did before with the identifiers for the Subscriber effect - those are effect settings too.
- I refactored the way settings specs for effect settings are provided by the effects in question. I think the new way is much easier as it hides a lot of the under-the-hood stuff that's in place in LEDStripEffect to save memory.
I modified the logic that decides when PatternStocks::Draw() cycles to the next stock. The old way made the effect "flip through" the configured stocks at high speed when the stock data was first loaded (or more accurately, if the stock data count increased while the effect was running).
I now only move to the next stock (index) if the update interval passes or the current stock index may be invalid because we have less stock data available than before.
I fixed a bug in the NetworkReader implementation that meant it would try to retrieve data when WiFi wasn't up (yet).
I bumped some actions in the GitHub workflow to a higher version as the previous action versions were using Node.JS 16, which is now deprecated.

I note that the CI build of the heltecv3demo project started failing sometime during today - this is unrelated to the last bullet in the list of changes I just mentioned. The error message is that pins_arduino.h cannot be found for that project, and Google seems to say that this is always caused by an underlying problem (board or platformio.ini configuration).

This could be a temporary issue that is located and is to be fixed in a dependency. If it persists until the time this PR is ready to merge otherwise, I will exclude the heltecv3demo project from CI builds for the time being.

rbergen

Looks good to me, with the suggestion to either finish or remove the UPC block at the end of README.md.

Also, I think we should wait a day or two to see if heltecv3demo will start building again before we proceed with merging this.

rbergen · 2024-05-14T17:59:44Z

README.md

+
+UPC Codes
+-----------------------------------------------------------------------------
+1	646680745383	MESMERKIT     Mesmerizer Kit w/64x32 Matrix, Remote, Receiver
+2	646680745390	MESMERPCB     Assembled PCB Only
+3	646680745406	
+4	646680745413	
+5	646680745420	


Could this be work in progress, or an accidental paste? It seems a bit lost here...

rbergen · 2024-05-14T18:03:34Z

include/gfxbase.h

    }

    void drawPixel(int16_t x, int16_t y, uint16_t color) override
    {
        if (isValidPixel(x, y))
            leds[XY(x, y)] = from16Bit(color);
-        else


Actually, they're not always. The error messages were removed because a dependency that can be used to move text across the panel doesn't do boundary checking. With the validity check already ignoring out of bounds pixels, it seemed unnecessary to also create a lot of debugging noise.

rbergen · 2024-05-19T07:37:35Z

I've removed the UPC block from the README as I suggested. As the heltecv3demo build still fails for a reason that must be rooted in a dependency, I've disabled CI for that env for the time being.

Barring any objections, I will merge this PR in its current state tomorrow (May 20, 2024) morning (GMT+2).

Dave Plummer and others added 10 commits January 24, 2024 16:37

Add AMOLED support

775204e

Update checkin

2baff67

Safekeeping checking

9376220

Fix unintended checkin

3f9711d

Sort of working text effects

0fb01c5

Merge pull request #50 from davepl/amoled

874532c

Amoled

Merge branch 'PlummersSoftwareLLC:main' into main

b69e83b

Checking build break.,..

76276d4

Merge branch 'main' of github.com:davepl/NightDriverStrip

f6270b7

Rotate visible stock, add stock graph

fdb799d

davepl requested a review from rbergen May 13, 2024 03:33

robertlipe approved these changes May 13, 2024

View reviewed changes

Review changes

99c66d5

robertlipe reviewed May 13, 2024

View reviewed changes

rbergen added 8 commits May 14, 2024 07:43

Update stocks effect

131cda3

Implement stock effect settings

5d45e48

Use AllocatedJsonDocument

dc782e8

Allow saved device config to be ignored

5b9f7be

Polishing

eb823a8

Fix capitalization of Heltec v3 board

11c002d

Unfix capitalization

598f64a

Remove variable - the expression is simple enough

68d4b7d

Bump github actions versions

40eddba

rbergen added 4 commits May 14, 2024 19:08

Add ability to exclude envs in CI

2bc577d

Generate warning for skipped envs

1cd6e0a

Improve message

13bcd4f

Clear exclusion list

b1fafa1

rbergen reviewed May 14, 2024

View reviewed changes

rbergen changed the title ~~Menagerie of changed - Stocks effect, AMOLED support, etc.~~ Menagerie of changes - Stocks effect, AMOLED support, etc. May 14, 2024

rbergen mentioned this pull request May 15, 2024

Panic when driving larger matrixes (one long strip) on 32MB 8MB ESP32-S3-DevKitC1 #580

Open

rbergen added 3 commits May 19, 2024 09:09

Don't use onboard pixel

e7ffc49

Remove UPC block at end of README

8c1772e

Disable heltecv3demo in CI

a1ded0f

rbergen merged commit 48ac73c into PlummersSoftwareLLC:main May 20, 2024
35 checks passed

This was referenced May 21, 2024

Add Initial Support for a Stock Ticker #564

Closed

Feature: Stock Ticker #341

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Menagerie of changes - Stocks effect, AMOLED support, etc. #626

Menagerie of changes - Stocks effect, AMOLED support, etc. #626

davepl commented May 13, 2024

robertlipe left a comment •

edited

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

rbergen May 14, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

robertlipe May 13, 2024

davepl commented May 13, 2024 via email

davepl commented May 13, 2024 via email

robertlipe left a comment

davepl commented May 13, 2024 via email

Thermotronica commented May 13, 2024 via email

robertlipe commented May 14, 2024 via email

davepl commented May 14, 2024 via email

davepl commented May 14, 2024 via email

robertlipe commented May 14, 2024 via email

rbergen commented May 14, 2024

rbergen left a comment

rbergen May 14, 2024

rbergen May 14, 2024

rbergen commented May 19, 2024

Menagerie of changes - Stocks effect, AMOLED support, etc. #626

Menagerie of changes - Stocks effect, AMOLED support, etc. #626

Conversation

davepl commented May 13, 2024

Description

Contributing requirements

robertlipe left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davepl commented May 13, 2024 via email

davepl commented May 13, 2024 via email

robertlipe left a comment

Choose a reason for hiding this comment

davepl commented May 13, 2024 via email

Thermotronica commented May 13, 2024 via email

robertlipe commented May 14, 2024 via email

davepl commented May 14, 2024 via email

davepl commented May 14, 2024 via email

robertlipe commented May 14, 2024 via email

rbergen commented May 14, 2024

rbergen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbergen commented May 19, 2024

robertlipe left a comment •

edited