One example:
class ModulinoLight : public Module {
public:
String getColorApproximate() {
String color = "UNKNOWN";
} else if (l > 80.0) {
color = "VERY LIGHT " + color; <--- multiple new/delete on heap to create a new string ram buffer?
My review helper: I need this helper, because my technical English is limited.
Source: Technical analysis provided by AI on Google Search, powered by the Gemini model family:
Technical Analysis: Memory Risks of getColorApproximate()
The implementation of getColorApproximate() in the Modulino library is a classic example of Heap Fragmentation risks in embedded systems.
The Core Problem:
The function uses the String class and multiple concatenations (e.g., color = "DARK " + color;). On microcontrollers with limited RAM (like AVR or even ESP32), this leads to:
Multiple Dynamic Allocations: Every + operation creates a temporary String object, allocates new memory, copies the data, and deallocates the old block.
Heap Fragmentation: These repeated "alloc-and-free" cycles create small gaps in the RAM. Over time, the heap becomes like "Swiss cheese," eventually leading to crashes when a large enough contiguous block cannot be found.
Unnecessary Copying: Instead of returning a simple pointer (const char*) to a static string in Flash, the function forces the creation of a full object and a deep copy of the text.
Recommendation:
For long-term stability, avoid using this function in the main loop(). Instead, use getColor() for raw data or rewrite the logic to return const char* literals to keep the heap clean.
I do try to limit or not even use the String class (I get old habits die hard) but for the sake of discussion I would say that I find that AI have been largely influenced by the numerous mentions that String class is super dangerous and they don’t dig further.
Yes it uses more RAM than a fixed c-strings and in ways that’s not so visible especially if you don’t understand what’s happening when you concatenate or return dynamic strings or pass by values etc and yes It could in specific cases lead to fragmentation and poke holes in the heap.
But true heap fragmentation requires a genuinely chaotic allocation pattern where many live allocations of varying sizes are interleaved in memory simultaneously, preventing the allocator from coalescing adjacent free blocks. This is rare in practice because it demands multiple concurrent owners all allocating and holding blocks of different sizes at the same time — a scenario that simply doesn’t arise in typical Arduino or ESP32 code, where allocation patterns tend to be predictable and short-lived.
For this library used normally, the single String in getColorApproximate() is a limited issue even on a small AVR unless you are dangerously close to RAM limit.
The alternative would be returning a const char* pointing to a static C-string inside the function which eliminates all heap involvement entirely — no allocation, no free, no coalescing needed. The string lives in a fixed memory location for the lifetime of the program, and the return is just a pointer copy.
The downside is that the function is no longer reentrant. Every call returns a pointer to the same memory location, so if two parts of your code call getColorApproximate() and both hold the returned pointer, the second call silently overwrites what the first pointer is pointing at. On a single-threaded Arduino sketch this is usually harmless, but on an ESP32 running FreeRTOS with multiple tasks, this becomes a genuine race condition that is difficult to debug.
The current String approach, despite its overhead, at least gives each caller their own independent copy of the data. The trade-off is essentially heap safety versus reentrancy safety, and which matters more depends entirely on your use case.
The sole use of the function in the examples (exactly twice) is to print a description of the color. In that case, heap allocation can be avoided entirely by implementing Printable, so that the color can "print itself"
class ColorApproximate : public Printable {
float h, s, l;
public:
ColorApproximate(uint8_t r, uint8_t g, uint8_t b) {
// void LTR381RGBClass::getHSL(int r, int g, int b, float& h, float& s, float& l) {
float red = r / 255.0;
float green = g / 255.0;
float blue = b / 255.0;
float cmax = max(red, max(blue, green));
float cmin = min(red, min(blue, green));
float delta = cmax - cmin;
l = (cmax + cmin) / 2;
if (delta == 0) {
h = 0;
} else if (cmax == red) {
h = 60 * fmod((((green - blue) / delta)), 6);
} else if (cmax == green) {
h = 60 * (((blue - red) / delta) + 2);
} else {
h = 60 * (((red - green) / delta) + 4);
}
if (h < 0) { // Adjust hue here because printTo is const
h += 360;
}
if (delta == 0) {
s = 0.0;
} else {
s = (delta / (1 - abs(2 * l - 1)));
}
l = l * 100;
s = s * 100;
}
virtual size_t printTo(Print& p) const override {
size_t ret = 0;
if (l > 90.0) {
return p.print("WHITE");
}
if (l <= 0.20) {
return p.print("BLACK");
}
if (s < 10.0) {
if (l < 50.0) {
return p.print("DARK GRAY");
} else {
return p.print("LIGHT GRAY");
}
}
// Adjust color based on saturation
if (s < 20.0) {
ret += p.print("VERY PALE ");
} else if (s < 40.0) {
ret += p.print("PALE ");
} else if (s > 80.0) {
ret += p.print("VERY VIVID ");
} else if (s > 60.0) {
ret += p.print("VIVID ");
}
// Adjust color based on lightness
if (l < 20.0) {
ret += p.print("VERY DARK ");
} else if (l < 40.0) {
ret += p.print("DARK ");
} else if (l > 80.0) {
ret += p.print("VERY LIGHT ");
} else if (l > 60.0) {
ret += p.print("LIGHT ");
}
if (h < 15 || h >= 345) {
ret += p.print("RED");
} else if (h < 45) {
ret += p.print("ORANGE");
} else if (h < 75) {
ret += p.print("YELLOW");
} else if (h < 105) {
ret += p.print("LIME");
} else if (h < 135) {
ret += p.print("GREEN");
} else if (h < 165) {
ret += p.print("SPRING GREEN");
} else if (h < 195) {
ret += p.print("CYAN");
} else if (h < 225) {
ret += p.print("AZURE");
} else if (h < 255) {
ret += p.print("BLUE");
} else if (h < 285) {
ret += p.print("VIOLET");
} else if (h < 315) {
ret += p.print("MAGENTA");
} else {
ret += p.print("ROSE");
}
return ret;
}
};
void setup() {
Serial.begin(115200);
Serial.println(ColorApproximate{ 255, 0, 0 });
Serial.println(ColorApproximate{ 0, 255, 0 });
Serial.println(ColorApproximate{ 0, 0, 255 });
}
void loop() {
uint8_t r = random(0, 256);
uint8_t g = random(0, 256);
uint8_t b = random(0, 256);
auto c = ColorApproximate{ r, g, b };
Serial.print(r);
Serial.print('\t');
Serial.print(g);
Serial.print('\t');
Serial.print(b);
Serial.print('\t');
Serial.println(c);
delay(1500);
}
(Note that the original function starts with String color = "UNKNOWN" but it never returns that value; it either returns early or sets the base color to something else.)
That’s an elegant solution. The one trade-off is that the caller can no longer hold the color description as a value to compare or manipulate programmatically. You get something you can print, not something you can store or branch on. For the stated use case — printing a human-readable description — that is not a limitation at all, but it would matter if a caller ever wanted to do something like if (getColorApproximate() == "RED") …
➜ That pattern, while poor practice, is at least possible with the original String return and likely used by newbies.
My personal meaning is that the ModulinoLight class should not implement the 'color-string'
method to minimize any risk. The class should not be responsible to do this. In existing example programs the color mapping to text usage would be great to understood the light class output values.
Having something like this is one clean solution ...
const char* green = "GREEN"; // e.g. in memory 0x10001000
const char* red = "RED"; // e.g. in memory 0x10002000
... because only a pointer reference is getting copied. Regardless how many application are using the pointer to one color text, only the pointer value (0x10001000) is getting used or replaced.
But nevertheless many color text names are possible and should be done by the user and not by the ModulinoLight class. This keeps the Modulino library slim, no hidden issue, no discussion and reduces the maintenance effort
The idea of a library is to provide common services. Getting the name of the color seems something many people would want to do and if they don’t the unused function and Strings will get deleted from te code at the optimization phase.
So I do think it’s a meaningful addition to the library.
